- Revision
- 218956
- Author
- [email protected]
- Date
- 2017-06-29 12:31:36 -0700 (Thu, 29 Jun 2017)
Log Message
WKContentRuleLists with if-top-url or unless-top-url should run regex against entire top URL
https://bugs.webkit.org/show_bug.cgi?id=173980
Reviewed by Tim Horton.
Source/WebKit2:
if-top-url and unless-top-url were broken inside WebKit2 apps. This fixes them and adds a test.
ContentExtension::populateConditionCacheIfNeeded was calling WebCompiledContentRuleList::conditionsApplyOnlyToDomain
but m_data.conditionsApplyOnlyToDomainOffset was always 0 instead of ConditionsApplyOnlyToDomainOffset because
it wasn't being encoded and decoded when telling the WebProcess about the content rule list. This was causing us
to use whatever was at offset 0 in the file instead of the correct value stored in the file to determine
whether to run regular expressions against the entire top URL for if-top-url or unless-top-url or against
just the domain for if-top-domain or unless-top-domain. Luckily, offset 0 in the file is always
ContentRuleListStore::CurrentContentRuleListFileVersion, so it was deterministic and easy to debug.
I should have added a LayoutTest with r213669 to verify correct behavior in an actual WKWebView,
but I didn't because it wouldn't have caught regressions since the contentextension tests are
marked as flaky since r206914, but once that is fixed the new test http/tests/contentextensions/top-url.html
will verify that this feature is behaving correctly. It failed before this change and passes after.
* Shared/WebCompiledContentRuleListData.cpp:
(WebKit::WebCompiledContentRuleListData::encode):
(WebKit::WebCompiledContentRuleListData::decode):
LayoutTests:
* http/tests/contentextensions/top-url-expected.txt: Added.
* http/tests/contentextensions/top-url.html: Added.
* http/tests/contentextensions/top-url.html.json: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (218955 => 218956)
--- trunk/LayoutTests/ChangeLog 2017-06-29 19:24:52 UTC (rev 218955)
+++ trunk/LayoutTests/ChangeLog 2017-06-29 19:31:36 UTC (rev 218956)
@@ -1,3 +1,14 @@
+2017-06-29 Alex Christensen <[email protected]>
+
+ WKContentRuleLists with if-top-url or unless-top-url should run regex against entire top URL
+ https://bugs.webkit.org/show_bug.cgi?id=173980
+
+ Reviewed by Tim Horton.
+
+ * http/tests/contentextensions/top-url-expected.txt: Added.
+ * http/tests/contentextensions/top-url.html: Added.
+ * http/tests/contentextensions/top-url.html.json: Added.
+
2017-06-29 JF Bastien <[email protected]>
WebAssembly: disable some APIs under CSP
Added: trunk/LayoutTests/http/tests/contentextensions/top-url-expected.txt (0 => 218956)
--- trunk/LayoutTests/http/tests/contentextensions/top-url-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/contentextensions/top-url-expected.txt 2017-06-29 19:31:36 UTC (rev 218956)
@@ -0,0 +1,24 @@
+CONSOLE MESSAGE: line 6: Content blocker prevented frame displaying http://127.0.0.1:8000/contentextensions/top-url.html from loading a resource from http://127.0.0.1:8000/resources/square100.png?should_be_blocked_1
+CONSOLE MESSAGE: line 7: Content blocker prevented frame displaying http://127.0.0.1:8000/contentextensions/top-url.html from loading a resource from http://127.0.0.1:8000/resources/square100.png?should_be_blocked_2
+CONSOLE MESSAGE: line 8: Content blocker prevented frame displaying http://127.0.0.1:8000/contentextensions/top-url.html from loading a resource from http://127.0.0.1:8000/resources/square100.png?should_be_blocked_3
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderText {#text} at (0,0) size 200x18
+ text run at (0,0) width 200: "The images below should load."
+ RenderBR {BR} at (199,14) size 1x0
+ RenderImage {IMG} at (0,18) size 100x100
+ RenderBR {BR} at (100,118) size 0x0
+ RenderImage {IMG} at (0,118) size 100x100
+ RenderBR {BR} at (100,218) size 0x0
+ RenderText {#text} at (0,218) size 242x18
+ text run at (0,218) width 242: "The images below should be blocked."
+ RenderBR {BR} at (241,232) size 1x0
+ RenderImage {IMG} at (0,236) size 0x0
+ RenderBR {BR} at (0,236) size 0x0
+ RenderImage {IMG} at (0,236) size 0x0
+ RenderBR {BR} at (0,236) size 0x0
+ RenderImage {IMG} at (0,236) size 0x0
+ RenderBR {BR} at (0,236) size 0x0
Added: trunk/LayoutTests/http/tests/contentextensions/top-url.html (0 => 218956)
--- trunk/LayoutTests/http/tests/contentextensions/top-url.html (rev 0)
+++ trunk/LayoutTests/http/tests/contentextensions/top-url.html 2017-06-29 19:31:36 UTC (rev 218956)
@@ -0,0 +1,9 @@
+<body>
+The images below should load.<br>
+<img src=""
+<img src=""
+The images below should be blocked.<br>
+<img src=""
+<img src=""
+<img src=""
+</body>
Added: trunk/LayoutTests/http/tests/contentextensions/top-url.html.json (0 => 218956)
--- trunk/LayoutTests/http/tests/contentextensions/top-url.html.json (rev 0)
+++ trunk/LayoutTests/http/tests/contentextensions/top-url.html.json 2017-06-29 19:31:36 UTC (rev 218956)
@@ -0,0 +1,46 @@
+[
+ {
+ "action": {
+ "type": "block"
+ },
+ "trigger": {
+ "url-filter": "should_be_blocked_1"
+ }
+ },
+ {
+ "action": {
+ "type": "block"
+ },
+ "trigger": {
+ "url-filter": "should_be_blocked_2",
+ "if-top-url": ["^https?://127.0.0.1.*/contentex", "^should.not*match"]
+ }
+ },
+ {
+ "action": {
+ "type": "block"
+ },
+ "trigger": {
+ "url-filter": "should_be_blocked_3",
+ "unless-top-url": ["should?not.match"]
+ }
+ },
+ {
+ "action": {
+ "type": "block"
+ },
+ "trigger": {
+ "url-filter": "should_not_be_blocked_1",
+ "if-top-url": ["also.should?not+match"]
+ }
+ },
+ {
+ "action": {
+ "type": "block"
+ },
+ "trigger": {
+ "url-filter": "should_not_be_blocked_2",
+ "unless-top-url": ["127.0.0.1.*/top-url\\.html"]
+ }
+ }
+]
Modified: trunk/Source/WebKit2/ChangeLog (218955 => 218956)
--- trunk/Source/WebKit2/ChangeLog 2017-06-29 19:24:52 UTC (rev 218955)
+++ trunk/Source/WebKit2/ChangeLog 2017-06-29 19:31:36 UTC (rev 218956)
@@ -1,3 +1,28 @@
+2017-06-29 Alex Christensen <[email protected]>
+
+ WKContentRuleLists with if-top-url or unless-top-url should run regex against entire top URL
+ https://bugs.webkit.org/show_bug.cgi?id=173980
+
+ Reviewed by Tim Horton.
+
+ if-top-url and unless-top-url were broken inside WebKit2 apps. This fixes them and adds a test.
+ ContentExtension::populateConditionCacheIfNeeded was calling WebCompiledContentRuleList::conditionsApplyOnlyToDomain
+ but m_data.conditionsApplyOnlyToDomainOffset was always 0 instead of ConditionsApplyOnlyToDomainOffset because
+ it wasn't being encoded and decoded when telling the WebProcess about the content rule list. This was causing us
+ to use whatever was at offset 0 in the file instead of the correct value stored in the file to determine
+ whether to run regular expressions against the entire top URL for if-top-url or unless-top-url or against
+ just the domain for if-top-domain or unless-top-domain. Luckily, offset 0 in the file is always
+ ContentRuleListStore::CurrentContentRuleListFileVersion, so it was deterministic and easy to debug.
+
+ I should have added a LayoutTest with r213669 to verify correct behavior in an actual WKWebView,
+ but I didn't because it wouldn't have caught regressions since the contentextension tests are
+ marked as flaky since r206914, but once that is fixed the new test http/tests/contentextensions/top-url.html
+ will verify that this feature is behaving correctly. It failed before this change and passes after.
+
+ * Shared/WebCompiledContentRuleListData.cpp:
+ (WebKit::WebCompiledContentRuleListData::encode):
+ (WebKit::WebCompiledContentRuleListData::decode):
+
2017-06-29 Chris Dumez <[email protected]>
statistics.mostRecentUserInteraction should be of type WallTime
Modified: trunk/Source/WebKit2/Shared/WebCompiledContentRuleListData.cpp (218955 => 218956)
--- trunk/Source/WebKit2/Shared/WebCompiledContentRuleListData.cpp 2017-06-29 19:24:52 UTC (rev 218955)
+++ trunk/Source/WebKit2/Shared/WebCompiledContentRuleListData.cpp 2017-06-29 19:31:36 UTC (rev 218956)
@@ -38,6 +38,7 @@
data->createHandle(handle, SharedMemory::Protection::ReadOnly);
encoder << handle;
+ encoder << conditionsApplyOnlyToDomainOffset;
encoder << actionsOffset;
encoder << actionsSize;
encoder << filtersWithoutConditionsBytecodeOffset;
@@ -55,6 +56,8 @@
return false;
compiledContentRuleListData.data = "" SharedMemory::Protection::ReadOnly);
+ if (!decoder.decode(compiledContentRuleListData.conditionsApplyOnlyToDomainOffset))
+ return false;
if (!decoder.decode(compiledContentRuleListData.actionsOffset))
return false;
if (!decoder.decode(compiledContentRuleListData.actionsSize))
