Attachment 359793 Details for Bug 193680 – Patch

[patch] Patch

bug-193680-20190122155236.patch (text/plain), 12.54 KB, created by Chris Dumez on 2019-01-22 15:52:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-22 15:52:37 PST

Size: 12.54 KB

patch

obsolete

>Subversion Revision: 240299
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 1a3bfa204c18adf316cc5a8e2cdcb45e6ece671a..ed3cc4a90d5227ddffc76c81c2128c596ff0b3be 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-01-22  Chris Dumez  <cdumez@apple.com>
>+
>+        Regression(r240178) Some API tests are crashing
>+        https://bugs.webkit.org/show_bug.cgi?id=193680
>+
>+        Reviewed by Alex Christensen.
>+
>+        r240178 made sure that userScripts / scriptMessageHandlers / contentExtensions are always
>+        properly populated in the WebPageCreationParameters. This was needed in case we need to
>+        reconstruct the WebUserContentController on the WebProcess side. However, this caused a
>+        regression in the case we reuse a process where the WebUserContentController still exists
>+        (because it was kept alive, e.g. by the WebPageGroup). In that case, we would add duplicate
>+        entries to the existing WebUserContentController instance because its "add" methods did not
>+        have duplicate checks. To address the issue, this patch adds duplicate checks to the
>+        WebUserContentController "add" methods.
>+
>+        * WebProcess/UserContent/WebUserContentController.cpp:
>+        (WebKit::WebUserContentController::addUserScriptMessageHandlerInternal):
>+        (WebKit::WebUserContentController::removeUserScriptMessageHandlerInternal):
>+        (WebKit::WebUserContentController::addUserScriptInternal):
>+        (WebKit::WebUserContentController::removeUserScriptInternal):
>+        (WebKit::WebUserContentController::addUserStyleSheetInternal):
>+        (WebKit::WebUserContentController::removeUserStyleSheetInternal):
>+        (WebKit::WebUserContentController::forEachUserMessageHandler const):
>+        * WebProcess/UserContent/WebUserContentController.h:
>+
> 2019-01-22  Antti Koivisto  <antti@apple.com>
> 
>         [iOS] Flash when swiping back to Google search result page
>diff --git a/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp b/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp
>index a2732678505e0260572cffb3f1b14f6ae70a0e4f..0fa27252ffab3ce5145b97fae0dc7f0253199562 100644
>--- a/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp
>+++ b/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp
>@@ -318,8 +318,10 @@ void WebUserContentController::removeAllUserScriptMessageHandlers(const Vector<u
> #if ENABLE(USER_MESSAGE_HANDLERS)
> void WebUserContentController::addUserScriptMessageHandlerInternal(InjectedBundleScriptWorld& world, uint64_t userScriptMessageHandlerIdentifier, const String& name)
> {
>-    auto& messageHandlersInWorld = m_userMessageHandlers.ensure(&world, [] { return Vector<RefPtr<WebUserMessageHandlerDescriptorProxy>>(); }).iterator->value;
>-    messageHandlersInWorld.append(WebUserMessageHandlerDescriptorProxy::create(this, name, world, userScriptMessageHandlerIdentifier));
>+    auto& messageHandlersInWorld = m_userMessageHandlers.ensure(&world, [] { return Vector<std::pair<uint64_t, RefPtr<WebUserMessageHandlerDescriptorProxy>>> { }; }).iterator->value;
>+    if (messageHandlersInWorld.findMatching([&](auto& pair) { return pair.first ==  userScriptMessageHandlerIdentifier; }) != notFound)
>+        return;
>+    messageHandlersInWorld.append(std::make_pair(userScriptMessageHandlerIdentifier, WebUserMessageHandlerDescriptorProxy::create(this, name, world, userScriptMessageHandlerIdentifier)));
> }
> 
> void WebUserContentController::removeUserScriptMessageHandlerInternal(InjectedBundleScriptWorld& world, uint64_t userScriptMessageHandlerIdentifier)
>@@ -329,14 +331,9 @@ void WebUserContentController::removeUserScriptMessageHandlerInternal(InjectedBu
>         return;
> 
>     auto& userMessageHandlers = it->value;
>-
>-    bool userMessageHandlersChanged = false;
>-    for (int i = userMessageHandlers.size() - 1; i >= 0; --i) {
>-        if (userMessageHandlers[i]->identifier() == userScriptMessageHandlerIdentifier) {
>-            userMessageHandlers.remove(i);
>-            userMessageHandlersChanged = true;
>-        }
>-    }
>+    bool userMessageHandlersChanged = userMessageHandlers.removeFirstMatching([userScriptMessageHandlerIdentifier](auto& pair) {
>+        return pair.first ==  userScriptMessageHandlerIdentifier;
>+    });
> 
>     if (!userMessageHandlersChanged)
>         return;
>@@ -370,7 +367,7 @@ void WebUserContentController::removeAllContentRuleLists()
> }
> #endif
> 
>-void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld& world, uint64_t userScriptIdentifier, UserScript&& userScript, InjectUserScriptImmediately immediately)
>+void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld& world, const Optional<uint64_t>& userScriptIdentifier, UserScript&& userScript, InjectUserScriptImmediately immediately)
> {
>     if (immediately == InjectUserScriptImmediately::Yes) {
>         Page::forEachPage([&] (auto& page) {
>@@ -388,13 +385,16 @@ void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld&
>         });
>     }
> 
>-    auto& scriptsInWorld = m_userScripts.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserScript>>(); }).iterator->value;
>+    auto& scriptsInWorld = m_userScripts.ensure(&world, [] { return Vector<std::pair<Optional<uint64_t>, WebCore::UserScript>>(); }).iterator->value;
>+    if (userScriptIdentifier && scriptsInWorld.findMatching([&](auto& pair) { return pair.first == userScriptIdentifier; }) != notFound)
>+        return;
>+
>     scriptsInWorld.append(std::make_pair(userScriptIdentifier, WTFMove(userScript)));
> }
> 
> void WebUserContentController::addUserScript(InjectedBundleScriptWorld& world, UserScript&& userScript)
> {
>-    addUserScriptInternal(world, 0, WTFMove(userScript), InjectUserScriptImmediately::No);
>+    addUserScriptInternal(world, WTF::nullopt, WTFMove(userScript), InjectUserScriptImmediately::No);
> }
> 
> void WebUserContentController::removeUserScriptWithURL(InjectedBundleScriptWorld& world, const URL& url)
>@@ -404,10 +404,9 @@ void WebUserContentController::removeUserScriptWithURL(InjectedBundleScriptWorld
>         return;
> 
>     auto& scripts = it->value;
>-    for (int i = scripts.size() - 1; i >= 0; --i) {
>-        if (scripts[i].second.url() == url)
>-            scripts.remove(i);
>-    }
>+    scripts.removeAllMatching([&](auto& pair) {
>+        return pair.second.url() == url;
>+    });
> 
>     if (scripts.isEmpty())
>         m_userScripts.remove(it);
>@@ -420,10 +419,9 @@ void WebUserContentController::removeUserScriptInternal(InjectedBundleScriptWorl
>         return;
> 
>     auto& scripts = it->value;
>-    for (int i = scripts.size() - 1; i >= 0; --i) {
>-        if (scripts[i].first == userScriptIdentifier)
>-            scripts.remove(i);
>-    }
>+    scripts.removeFirstMatching([userScriptIdentifier](auto& pair) {
>+        return pair.first == userScriptIdentifier;
>+    });
> 
>     if (scripts.isEmpty())
>         m_userScripts.remove(it);
>@@ -434,15 +432,18 @@ void WebUserContentController::removeUserScripts(InjectedBundleScriptWorld& worl
>     m_userScripts.remove(&world);
> }
> 
>-void WebUserContentController::addUserStyleSheetInternal(InjectedBundleScriptWorld& world, uint64_t userStyleSheetIdentifier, UserStyleSheet&& userStyleSheet)
>+void WebUserContentController::addUserStyleSheetInternal(InjectedBundleScriptWorld& world, const Optional<uint64_t>& userStyleSheetIdentifier, UserStyleSheet&& userStyleSheet)
> {
>-    auto& styleSheetsInWorld = m_userStyleSheets.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserStyleSheet>>(); }).iterator->value;
>+    auto& styleSheetsInWorld = m_userStyleSheets.ensure(&world, [] { return Vector<std::pair<Optional<uint64_t>, WebCore::UserStyleSheet>>(); }).iterator->value;
>+    if (userStyleSheetIdentifier && styleSheetsInWorld.findMatching([&](auto& pair) { return pair.first == userStyleSheetIdentifier; }) != notFound)
>+        return;
>+
>     styleSheetsInWorld.append(std::make_pair(userStyleSheetIdentifier, WTFMove(userStyleSheet)));
> }
> 
> void WebUserContentController::addUserStyleSheet(InjectedBundleScriptWorld& world, UserStyleSheet&& userStyleSheet)
> {
>-    addUserStyleSheetInternal(world, 0, WTFMove(userStyleSheet));
>+    addUserStyleSheetInternal(world, WTF::nullopt, WTFMove(userStyleSheet));
>     invalidateInjectedStyleSheetCacheInAllFramesInAllPages();
> }
> 
>@@ -453,14 +454,9 @@ void WebUserContentController::removeUserStyleSheetWithURL(InjectedBundleScriptW
>         return;
> 
>     auto& stylesheets = it->value;
>-
>-    bool sheetsChanged = false;
>-    for (int i = stylesheets.size() - 1; i >= 0; --i) {
>-        if (stylesheets[i].second.url() == url) {
>-            stylesheets.remove(i);
>-            sheetsChanged = true;
>-        }
>-    }
>+    bool sheetsChanged = stylesheets.removeAllMatching([&](auto& pair) {
>+        return pair.second.url() == url;
>+    });
> 
>     if (!sheetsChanged)
>         return;
>@@ -478,14 +474,9 @@ void WebUserContentController::removeUserStyleSheetInternal(InjectedBundleScript
>         return;
> 
>     auto& stylesheets = it->value;
>-
>-    bool sheetsChanged = false;
>-    for (int i = stylesheets.size() - 1; i >= 0; --i) {
>-        if (stylesheets[i].first == userStyleSheetIdentifier) {
>-            stylesheets.remove(i);
>-            sheetsChanged = true;
>-        }
>-    }
>+    bool sheetsChanged = stylesheets.removeFirstMatching([userStyleSheetIdentifier](auto& pair) {
>+        return pair.first == userStyleSheetIdentifier;
>+    });
> 
>     if (!sheetsChanged)
>         return;
>@@ -534,9 +525,9 @@ void WebUserContentController::forEachUserStyleSheet(Function<void(const WebCore
> #if ENABLE(USER_MESSAGE_HANDLERS)
> void WebUserContentController::forEachUserMessageHandler(Function<void(const WebCore::UserMessageHandlerDescriptor&)>&& functor) const
> {
>-    for (const auto& userMessageHandlerVector : m_userMessageHandlers.values()) {
>-        for (const auto& userMessageHandler : userMessageHandlerVector)
>-            functor(*userMessageHandler.get());
>+    for (auto& userMessageHandlerVector : m_userMessageHandlers.values()) {
>+        for (auto& pair : userMessageHandlerVector)
>+            functor(*pair.second.get());
>     }
> }
> #endif
>diff --git a/Source/WebKit/WebProcess/UserContent/WebUserContentController.h b/Source/WebKit/WebProcess/UserContent/WebUserContentController.h
>index 164f5782edc1f8e34b77debc9ed8b029d9008847..5f1afb4220dd9d06a7f07ebe1f7e8aa802c1692f 100644
>--- a/Source/WebKit/WebProcess/UserContent/WebUserContentController.h
>+++ b/Source/WebKit/WebProcess/UserContent/WebUserContentController.h
>@@ -104,9 +104,9 @@ private:
>     void removeAllContentRuleLists();
> #endif
> 
>-    void addUserScriptInternal(InjectedBundleScriptWorld&, uint64_t userScriptIdentifier, WebCore::UserScript&&, InjectUserScriptImmediately);
>+    void addUserScriptInternal(InjectedBundleScriptWorld&, const Optional<uint64_t>& userScriptIdentifier, WebCore::UserScript&&, InjectUserScriptImmediately);
>     void removeUserScriptInternal(InjectedBundleScriptWorld&, uint64_t userScriptIdentifier);
>-    void addUserStyleSheetInternal(InjectedBundleScriptWorld&, uint64_t userStyleSheetIdentifier, WebCore::UserStyleSheet&&);
>+    void addUserStyleSheetInternal(InjectedBundleScriptWorld&, const Optional<uint64_t>& userStyleSheetIdentifier, WebCore::UserStyleSheet&&);
>     void removeUserStyleSheetInternal(InjectedBundleScriptWorld&, uint64_t userStyleSheetIdentifier);
> #if ENABLE(USER_MESSAGE_HANDLERS)
>     void addUserScriptMessageHandlerInternal(InjectedBundleScriptWorld&, uint64_t userScriptMessageHandlerIdentifier, const String& name);
>@@ -115,14 +115,14 @@ private:
> 
>     UserContentControllerIdentifier m_identifier;
> 
>-    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, WebCore::UserScript>>> WorldToUserScriptMap;
>+    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<Optional<uint64_t>, WebCore::UserScript>>> WorldToUserScriptMap;
>     WorldToUserScriptMap m_userScripts;
> 
>-    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, WebCore::UserStyleSheet>>> WorldToUserStyleSheetMap;
>+    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<Optional<uint64_t>, WebCore::UserStyleSheet>>> WorldToUserStyleSheetMap;
>     WorldToUserStyleSheetMap m_userStyleSheets;
> 
> #if ENABLE(USER_MESSAGE_HANDLERS)
>-    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<RefPtr<WebUserMessageHandlerDescriptorProxy>>> WorldToUserMessageHandlerVectorMap;
>+    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, RefPtr<WebUserMessageHandlerDescriptorProxy>>>> WorldToUserMessageHandlerVectorMap;
>     WorldToUserMessageHandlerVectorMap m_userMessageHandlers;
> #endif
> #if ENABLE(CONTENT_EXTENSIONS)

Actions: View | Formatted Diff | Diff

Attachments on bug 193680: 359769 | 359776 | 359782 | 359793 | 359818