Attachment 359769 Details for Bug 193680 – Patch

[patch] Patch

bug-193680-20190122132528.patch (text/plain), 7.94 KB, created by Chris Dumez on 2019-01-22 13:25:29 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-01-22 13:25:29 PST

Size: 7.94 KB

patch

obsolete

>Subversion Revision: 240252
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index fdfa234e85ee9a26a6dc87473d0e08359405ee36..0ecdb7c500443ab6e6dc536780fc93eb5db22851 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 NOBODY (OOPS!).
>+
>+        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  Oriol Brufau  <obrufau@igalia.com>
> 
>         [css-logical] Implement flow-relative margin, padding and border shorthands
>diff --git a/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp b/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp
>index a2732678505e0260572cffb3f1b14f6ae70a0e4f..9f7baca85e8399f8418826696ca82648327370f3 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;
>@@ -389,6 +386,9 @@ void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld&
>     }
> 
>     auto& scriptsInWorld = m_userScripts.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserScript>>(); }).iterator->value;
>+    if (scriptsInWorld.findMatching([&](auto& pair) { return pair.first == userScriptIdentifier; }) != notFound)
>+        return;
>+
>     scriptsInWorld.append(std::make_pair(userScriptIdentifier, WTFMove(userScript)));
> }
> 
>@@ -420,10 +420,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);
>@@ -437,6 +436,9 @@ void WebUserContentController::removeUserScripts(InjectedBundleScriptWorld& worl
> void WebUserContentController::addUserStyleSheetInternal(InjectedBundleScriptWorld& world, uint64_t userStyleSheetIdentifier, UserStyleSheet&& userStyleSheet)
> {
>     auto& styleSheetsInWorld = m_userStyleSheets.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserStyleSheet>>(); }).iterator->value;
>+    if (styleSheetsInWorld.findMatching([&](auto& pair) { return pair.first == userStyleSheetIdentifier; }) != notFound)
>+        return;
>+
>     styleSheetsInWorld.append(std::make_pair(userStyleSheetIdentifier, WTFMove(userStyleSheet)));
> }
> 
>@@ -478,14 +480,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;
>@@ -535,8 +532,8 @@ void WebUserContentController::forEachUserStyleSheet(Function<void(const WebCore
> 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 (const 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..1ee2863d4a662b51fc2aa672e86198a9ddee3a5c 100644
>--- a/Source/WebKit/WebProcess/UserContent/WebUserContentController.h
>+++ b/Source/WebKit/WebProcess/UserContent/WebUserContentController.h
>@@ -122,7 +122,7 @@ private:
>     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