Title: [187064] branches/safari-601.1-branch
Revision
187064
Author
matthew_han...@apple.com
Date
2015-07-20 21:37:05 -0700 (Mon, 20 Jul 2015)

Log Message

Merge r186957. rdar://problem/21867831

Modified Paths

Added Paths

Diff

Modified: branches/safari-601.1-branch/LayoutTests/ChangeLog (187063 => 187064)


--- branches/safari-601.1-branch/LayoutTests/ChangeLog	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/LayoutTests/ChangeLog	2015-07-21 04:37:05 UTC (rev 187064)
@@ -1,5 +1,26 @@
 2015-07-20  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r186957. rdar://problem/21867831
+
+    2015-07-17  Benjamin Poulain  <bpoul...@apple.com>
+
+            [Content Extensions] CSS-display-none rules are not working properly
+            https://bugs.webkit.org/show_bug.cgi?id=147024
+
+            Reviewed by Sam Weinig.
+
+            * http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged-expected.txt: Added.
+            * http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html: Added.
+            * http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html.json: Added.
+            * http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged-expected.txt: Added.
+            * http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html: Added.
+            * http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html.json: Added.
+            * http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource-expected.txt: Added.
+            * http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html: Added.
+            * http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html.json: Added.
+
+2015-07-20  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r186858. rdar://problem/21643094
 
     2015-07-15  Tim Horton  <timothy_hor...@apple.com>

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged-expected.txt (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged-expected.txt	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged-expected.txt	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,5 @@
+This text should be visible
+
+This text should be visible
+
+This text should be visible

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,12 @@
+<head>
+<meta charset="UTF-8"></meta>
+<script>
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p class="hidden1">This text should be visible</p>
+<p class="hidden2">This text should not be visible!!!</p>
+<p class="hidden3">This text should be visible</p>
+<p class="hidden4">This text should be visible</p>
+</body>

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html.json (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html.json	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html.json	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,56 @@
+[
+    {
+        "action": {
+            "type": "block"
+        },
+        "trigger": {
+            "url-filter": "this-should-not-block"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden1"
+        },
+        "trigger": {
+            "url-filter": "this-should-not-match"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden2"
+        },
+        "trigger": {
+            "url-filter": "css-display-none-with-different-case-sensitivity-are-not-merged.html$"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden3"
+        },
+        "trigger": {
+            "url-filter-is-case-sensitive": true,
+            "url-filter": "css-display-none-with-different-Case-sensitivity-are-not-merged.html$"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden4"
+        },
+        "trigger": {
+            "url-filter": "this-does-not-match-the-url-either"
+        }
+    },
+    {
+        "action": {
+            "type": "block"
+        },
+        "trigger": {
+            "url-filter-is-case-sensitive": true,
+            "url-filter": "css-display-none-with-different-Case-sensitivity-are-not-merged.html$"
+        }
+    }
+]

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged-expected.txt (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged-expected.txt	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged-expected.txt	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,3 @@
+This text should be visible
+
+This text should be visible

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,11 @@
+<head>
+<meta charset="UTF-8"></meta>
+<script>
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p class="hidden1">This text should be visible</p>
+<p class="hidden2">This text should not be visible!!!</p>
+<p class="hidden3">This text should be visible</p>
+</body>

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html.json (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html.json	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html.json	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,29 @@
+[
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden1"
+        },
+        "trigger": {
+            "url-filter": "this-does-not-match-the-url"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden2"
+        },
+        "trigger": {
+            "url-filter": "css-display-none-with-different-triggers-are-not-merged.html"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden3"
+        },
+        "trigger": {
+            "url-filter": "this-does-not-match-the-url-either"
+        }
+    }
+]

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource-expected.txt (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource-expected.txt	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource-expected.txt	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,3 @@
+This text should be visible.
+
+This text should be visible.

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,13 @@
+<head>
+<meta charset="UTF-8"></meta>
+<script>
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p id="hidden">This text should be visible.</p>
+<p class="hidden1">This text should not be visible!!! 1</p>
+<p class="hidden2">This text should not be visible!!! 2</p>
+<p class="hidden1 hidden2">This text should not be visible!!! 3</p>
+<p id="hidden3">This text should be visible.</p>
+</body>

Added: branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html.json (0 => 187064)


--- branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html.json	                        (rev 0)
+++ branches/safari-601.1-branch/LayoutTests/http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html.json	2015-07-21 04:37:05 UTC (rev 187064)
@@ -0,0 +1,21 @@
+[
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden1"
+        },
+        "trigger": {
+            "url-filter": "two-distinguishable-css-display-none-rules-on-main-resource.html"
+        }
+    },
+    {
+        "action": {
+            "type": "css-display-none",
+            "selector": ".hidden2"
+        },
+        "trigger": {
+            "resource-type": ["document"],
+            "url-filter": "distinguishable-css-display-none-rules-on-main-resource.html$"
+        }
+    }
+]

Modified: branches/safari-601.1-branch/Source/WebCore/ChangeLog (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/ChangeLog	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/ChangeLog	2015-07-21 04:37:05 UTC (rev 187064)
@@ -1,5 +1,55 @@
 2015-07-20  Matthew Hanson  <matthew_han...@apple.com>
 
+        Merge r186957. rdar://problem/21867831
+
+    2015-07-17  Benjamin Poulain  <bpoul...@apple.com>
+
+            [Content Extensions] CSS-display-none rules are not working properly
+            https://bugs.webkit.org/show_bug.cgi?id=147024
+
+            Reviewed by Sam Weinig.
+
+            There were 2 bugs prevening rules with css-display-none and a url-filter from working
+            correctly.
+
+            First, ContentExtensions::serializeActions() was merging selectors regardless of their
+            trigger. All the CSS Selectors would be grouped together and applied regardless of which
+            rule apply.
+
+            That problem was fixed by grouping CSS rules by trigger. We want all the undistinguishable
+            CSS rules to be merged. The trigger makes 2 rules dinstinguishable as one rule can apply
+            on a page while the next rule does not. The simplest approach is to group by trigger.
+
+            The second problem had to do with rules added before the document is created.
+            When accumulating those rules, we were only keeping the last one. The reason was that
+            DocumentLoader::addPendingContentExtensionDisplayNoneSelector() would only keep a single
+            selector list by extension.
+
+            This is fixed by keeping a vector of all the rules that apply.
+
+            Tests: http/tests/contentextensions/css-display-none-with-different-case-sensitivity-are-not-merged.html
+                   http/tests/contentextensions/css-display-none-with-different-triggers-are-not-merged.html
+                   http/tests/contentextensions/two-distinguishable-css-display-none-rules-on-main-resource.html
+
+            * contentextensions/ContentExtensionCompiler.cpp:
+            (WebCore::ContentExtensions::serializeActions):
+            * contentextensions/ContentExtensionRule.h:
+            (WebCore::ContentExtensions::Trigger::isEmpty):
+            (WebCore::ContentExtensions::TriggerHash::hash):
+            (WebCore::ContentExtensions::TriggerHash::equal):
+            (WebCore::ContentExtensions::TriggerHashTraits::constructDeletedValue):
+            (WebCore::ContentExtensions::TriggerHashTraits::isDeletedValue):
+            (WebCore::ContentExtensions::TriggerHashTraits::emptyValue):
+            (WebCore::ContentExtensions::TriggerHashTraits::isEmptyValue):
+            * contentextensions/ContentExtensionsBackend.cpp:
+            (WebCore::ContentExtensions::ContentExtensionsBackend::processContentExtensionRulesForLoad): Deleted.
+            * loader/DocumentLoader.cpp:
+            (WebCore::DocumentLoader::commitData):
+            (WebCore::DocumentLoader::addPendingContentExtensionDisplayNoneSelector):
+            * loader/DocumentLoader.h:
+
+2015-07-20  Matthew Hanson  <matthew_han...@apple.com>
+
         Merge r186956. rdar://problem/21643094
 
     2015-07-17  Tim Horton  <timothy_hor...@apple.com>

Modified: branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionCompiler.cpp	2015-07-21 04:37:05 UTC (rev 187064)
@@ -74,7 +74,7 @@
     Vector<String> selectors;
     Vector<unsigned> clientLocations;
 };
-typedef HashMap<uint32_t, PendingDisplayNoneActions, DefaultHash<uint32_t>::Hash, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> PendingDisplayNoneActionsMap;
+typedef HashMap<Trigger, PendingDisplayNoneActions, TriggerHash, TriggerHashTraits> PendingDisplayNoneActionsMap;
 
 static void resolvePendingDisplayNoneActions(Vector<SerializedActionByte>& actions, Vector<unsigned>& actionLocations, PendingDisplayNoneActionsMap& pendingDisplayNoneActionsMap)
 {
@@ -167,7 +167,7 @@
             break;
         }
         case ActionType::CSSDisplayNoneSelector: {
-            const auto addResult = cssDisplayNoneActionsMap.add(flags, PendingDisplayNoneActions());
+            const auto addResult = cssDisplayNoneActionsMap.add(rule.trigger(), PendingDisplayNoneActions());
             PendingDisplayNoneActions& pendingDisplayNoneActions = addResult.iterator->value;
             pendingDisplayNoneActions.selectors.append(rule.action().stringArgument());
             pendingDisplayNoneActions.clientLocations.append(actionLocations.size());

Modified: branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionRule.h (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionRule.h	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionRule.h	2015-07-21 04:37:05 UTC (rev 187064)
@@ -51,12 +51,21 @@
         IfDomain,
         UnlessDomain,
     } domainCondition { DomainCondition::None };
-    
+
     ~Trigger()
     {
         ASSERT(domains.isEmpty() == (domainCondition == DomainCondition::None));
     }
-    
+
+    bool isEmpty() const
+    {
+        return urlFilter.isEmpty()
+            && !urlFilterIsCaseSensitive
+            && !flags
+            && domains.isEmpty()
+            && domainCondition == DomainCondition::None;
+    }
+
     bool operator==(const Trigger& other) const
     {
         return urlFilter == other.urlFilter
@@ -66,7 +75,58 @@
             && domainCondition == other.domainCondition;
     }
 };
-    
+
+struct TriggerHash {
+    static unsigned hash(const Trigger& trigger)
+    {
+        unsigned hash = trigger.urlFilterIsCaseSensitive ? 10619863 : 40960001;
+        if (!trigger.urlFilter.isNull())
+            hash ^= StringHash::hash(trigger.urlFilter);
+        hash = WTF::pairIntHash(hash, DefaultHash<ResourceFlags>::Hash::hash(trigger.flags));
+
+        for (const String& domain : trigger.domains)
+            hash ^= StringHash::hash(domain);
+
+        if (trigger.domainCondition == Trigger::DomainCondition::IfDomain)
+            hash |= 1 << 16;
+        else if (trigger.domainCondition == Trigger::DomainCondition::IfDomain)
+            hash |= 1 << 31;
+        return hash;
+    }
+
+    static bool equal(const Trigger& a, const Trigger& b)
+    {
+        return a == b;
+    }
+
+    static const bool safeToCompareToEmptyOrDeleted = false;
+};
+
+struct TriggerHashTraits : public WTF::CustomHashTraits<Trigger> {
+    static const bool emptyValueIsZero = false;
+    static const bool hasIsEmptyValueFunction = true;
+
+    static void constructDeletedValue(Trigger& trigger)
+    {
+        new (NotNull, std::addressof(trigger.urlFilter)) String(WTF::HashTableDeletedValue);
+    }
+
+    static bool isDeletedValue(const Trigger& trigger)
+    {
+        return trigger.urlFilter.isHashTableDeletedValue();
+    }
+
+    static Trigger emptyValue()
+    {
+        return Trigger();
+    }
+
+    static bool isEmptyValue(const Trigger& trigger)
+    {
+        return trigger.isEmpty();
+    }
+};
+
 struct Action {
     Action()
         : m_type(ActionType::InvalidAction)

Modified: branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionsBackend.cpp (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionsBackend.cpp	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/contentextensions/ContentExtensionsBackend.cpp	2015-07-21 04:37:05 UTC (rev 187064)
@@ -156,7 +156,6 @@
     ResourceLoadInfo resourceLoadInfo = { request.url(), mainDocumentURL, resourceType };
     Vector<ContentExtensions::Action> actions = actionsForResourceLoad(resourceLoadInfo);
 
-    StringBuilder css;
     bool willBlockLoad = false;
     for (const auto& action : actions) {
         switch (action.type()) {

Modified: branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.cpp (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.cpp	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.cpp	2015-07-21 04:37:05 UTC (rev 187064)
@@ -836,8 +836,10 @@
 
     for (auto& pendingStyleSheet : m_pendingNamedContentExtensionStyleSheets)
         styleSheetCollection.maybeAddContentExtensionSheet(pendingStyleSheet.key, *pendingStyleSheet.value);
-    for (auto& pendingSelector : m_pendingContentExtensionDisplayNoneSelectors)
-        styleSheetCollection.addDisplayNoneSelector(pendingSelector.key, pendingSelector.value.first, pendingSelector.value.second);
+    for (auto& pendingSelectorEntry : m_pendingContentExtensionDisplayNoneSelectors) {
+        for (const auto& pendingSelector : pendingSelectorEntry.value)
+            styleSheetCollection.addDisplayNoneSelector(pendingSelectorEntry.key, pendingSelector.first, pendingSelector.second);
+    }
 
     m_pendingNamedContentExtensionStyleSheets.clear();
     m_pendingContentExtensionDisplayNoneSelectors.clear();
@@ -1585,7 +1587,8 @@
 void DocumentLoader::addPendingContentExtensionDisplayNoneSelector(const String& identifier, const String& selector, uint32_t selectorID)
 {
     ASSERT(!m_gotFirstByte);
-    m_pendingContentExtensionDisplayNoneSelectors.set(identifier, std::make_pair(selector, selectorID));
+    auto addResult = m_pendingContentExtensionDisplayNoneSelectors.add(identifier, Vector<std::pair<String, uint32_t>>());
+    addResult.iterator->value.append(std::make_pair(selector, selectorID));
 }
 #endif
 

Modified: branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.h (187063 => 187064)


--- branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.h	2015-07-21 04:37:00 UTC (rev 187063)
+++ branches/safari-601.1-branch/Source/WebCore/loader/DocumentLoader.h	2015-07-21 04:37:05 UTC (rev 187064)
@@ -445,7 +445,7 @@
 
 #if ENABLE(CONTENT_EXTENSIONS)
         HashMap<String, RefPtr<StyleSheetContents>> m_pendingNamedContentExtensionStyleSheets;
-        HashMap<String, std::pair<String, uint32_t>> m_pendingContentExtensionDisplayNoneSelectors;
+        HashMap<String, Vector<std::pair<String, uint32_t>>> m_pendingContentExtensionDisplayNoneSelectors;
 #endif
 
 #ifndef NDEBUG
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to