Attachment 373681 Details for Bug 199591 – Patch

[patch] Patch

bug-199591-20190708170957.patch (text/plain), 19.40 KB, created by Chris Dumez on 2019-07-08 17:09:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-07-08 17:09:58 PDT

Size: 19.40 KB

patch

obsolete

>Subversion Revision: 247239
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index ca4666271303b6fcaca58521eddd4d07fe4881bf..911926796b3dba72ab8273a51f46579fef522bf3 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,16 @@
>+2019-07-08  Chris Dumez  <cdumez@apple.com>
>+
>+        Use WeakHashSet for WebUserContentControllerProxy::m_processes
>+        https://bugs.webkit.org/show_bug.cgi?id=199591
>+        <rdar://problem/52798721>
>+
>+        Reviewed by Youenn Fablet.
>+
>+        Update WeakHashSet::add() to return an AddResult type, similarly to our other containers.
>+
>+        * wtf/WeakHashSet.h:
>+        (WTF::WeakHashSet::add):
>+
> 2019-07-08  Christopher Reid  <chris.reid@sony.com>
> 
>         Implement MappedFileData for Windows
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 1ca00081312ed2cb2d053924ee661e3f920842ee..3d2cbb9c5908b22f8a7a0bfb6c152b1e22bf0523 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-07-08  Chris Dumez  <cdumez@apple.com>
>+
>+        Use WeakHashSet for WebUserContentControllerProxy::m_processes
>+        https://bugs.webkit.org/show_bug.cgi?id=199591
>+        <rdar://problem/52798721>
>+
>+        Reviewed by Youenn Fablet.
>+
>+        Use WeakHashSet for WebUserContentControllerProxy::m_processses for safety. In theory, a WebProcessProxy could
>+        stay in the map if we failed to call WebProcessProxy::shutDown() before destroying it.
>+
>+        * UIProcess/UserContent/WebUserContentControllerProxy.cpp:
>+        (WebKit::WebUserContentControllerProxy::~WebUserContentControllerProxy):
>+        (WebKit::WebUserContentControllerProxy::addProcess):
>+        (WebKit::WebUserContentControllerProxy::removeProcess):
>+        (WebKit::WebUserContentControllerProxy::addUserContentWorldUse):
>+        (WebKit::WebUserContentControllerProxy::removeUserContentWorldUses):
>+        (WebKit::WebUserContentControllerProxy::addUserScript):
>+        (WebKit::WebUserContentControllerProxy::removeUserScript):
>+        (WebKit::WebUserContentControllerProxy::removeAllUserScripts):
>+        (WebKit::WebUserContentControllerProxy::addUserStyleSheet):
>+        (WebKit::WebUserContentControllerProxy::removeUserStyleSheet):
>+        (WebKit::WebUserContentControllerProxy::removeAllUserStyleSheets):
>+        (WebKit::WebUserContentControllerProxy::addUserScriptMessageHandler):
>+        (WebKit::WebUserContentControllerProxy::removeUserMessageHandlerForName):
>+        (WebKit::WebUserContentControllerProxy::removeAllUserMessageHandlers):
>+        (WebKit::WebUserContentControllerProxy::addContentRuleList):
>+        (WebKit::WebUserContentControllerProxy::removeContentRuleList):
>+        (WebKit::WebUserContentControllerProxy::removeAllContentRuleLists):
>+        * UIProcess/UserContent/WebUserContentControllerProxy.h:
>+        (WebKit::WebUserContentControllerProxy::addNetworkProcess):
>+        (WebKit::WebUserContentControllerProxy::removeNetworkProcess):
>+
> 2019-07-08  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Support select all in non-editable element
>diff --git a/Source/WTF/wtf/WeakHashSet.h b/Source/WTF/wtf/WeakHashSet.h
>index 0728cf38aa6f16a9c080cac049e44eef7a9dfa79..1bc3690bb1149ef47dd752e0e65674a4af64ae50 100644
>--- a/Source/WTF/wtf/WeakHashSet.h
>+++ b/Source/WTF/wtf/WeakHashSet.h
>@@ -44,6 +44,7 @@ template <typename T>
> class WeakHashSet {
> public:
>     typedef HashSet<Ref<WeakPtrImpl>> WeakPtrImplSet;
>+    typedef typename WeakPtrImplSet::AddResult AddResult;
> 
>     class WeakHashSetConstIterator : public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, const T*, const T&> {
>     private:
>@@ -96,9 +97,9 @@ public:
>     const_iterator end() const { return WeakHashSetConstIterator(m_set, m_set.end()); }
> 
>     template <typename U>
>-    void add(const U& value)
>+    AddResult add(const U& value)
>     {
>-        m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_impl);
>+        return m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_impl);
>     }
> 
>     template <typename U>
>diff --git a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>index 834db7ae0792acb54593800806c92b2ffa5cc311..107bb227086c3cc40100b3e270d967a4bb1a7ffb 100644
>--- a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>+++ b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp
>@@ -74,19 +74,21 @@ WebUserContentControllerProxy::WebUserContentControllerProxy()
> WebUserContentControllerProxy::~WebUserContentControllerProxy()
> {
>     webUserContentControllerProxies().remove(m_identifier);
>-    for (auto* process : m_processes) {
>-        process->removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
>-        process->didDestroyWebUserContentControllerProxy(*this);
>+    for (auto& process : m_processes) {
>+        process.removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
>+        process.didDestroyWebUserContentControllerProxy(*this);
>     }
> #if ENABLE(CONTENT_EXTENSIONS)
>-    for (auto* process : m_networkProcesses)
>-        process->didDestroyWebUserContentControllerProxy(*this);
>+    for (auto& process : m_networkProcesses)
>+        process.didDestroyWebUserContentControllerProxy(*this);
> #endif
> }
> 
> void WebUserContentControllerProxy::addProcess(WebProcessProxy& webProcessProxy, WebPageCreationParameters& parameters)
> {
>-    if (m_processes.add(&webProcessProxy).isNewEntry)
>+    ASSERT(!m_processes.hasNullReferences());
>+
>+    if (m_processes.add(webProcessProxy).isNewEntry)
>         webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier(), *this);
> 
>     ASSERT(parameters.userContentWorlds.isEmpty());
>@@ -114,9 +116,10 @@ void WebUserContentControllerProxy::addProcess(WebProcessProxy& webProcessProxy,
> 
> void WebUserContentControllerProxy::removeProcess(WebProcessProxy& webProcessProxy)
> {
>-    ASSERT(m_processes.contains(&webProcessProxy));
>+    ASSERT(m_processes.contains(webProcessProxy));
>+    ASSERT(!m_processes.hasNullReferences());
> 
>-    m_processes.remove(&webProcessProxy);
>+    m_processes.remove(webProcessProxy);
>     webProcessProxy.removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
> }
> 
>@@ -127,8 +130,8 @@ void WebUserContentControllerProxy::addUserContentWorldUse(API::UserContentWorld
> 
>     auto addResult = m_userContentWorlds.add(&world);
>     if (addResult.isNewEntry) {
>-        for (WebProcessProxy* process : m_processes)
>-            process->send(Messages::WebUserContentController::AddUserContentWorlds({ std::make_pair(world.identifier(), world.name()) }), identifier());
>+        for (auto& process : m_processes)
>+            process.send(Messages::WebUserContentController::AddUserContentWorlds({ std::make_pair(world.identifier(), world.name()) }), identifier());
>     }
> }
> 
>@@ -151,8 +154,8 @@ bool WebUserContentControllerProxy::shouldSendRemoveUserContentWorldsMessage(API
> void WebUserContentControllerProxy::removeUserContentWorldUses(API::UserContentWorld& world, unsigned numberOfUsesToRemove)
> {
>     if (shouldSendRemoveUserContentWorldsMessage(world, numberOfUsesToRemove)) {
>-        for (WebProcessProxy* process : m_processes)
>-            process->send(Messages::WebUserContentController::RemoveUserContentWorlds({ world.identifier() }), identifier());
>+        for (auto& process : m_processes)
>+            process.send(Messages::WebUserContentController::RemoveUserContentWorlds({ world.identifier() }), identifier());
>     }
> }
> 
>@@ -164,8 +167,8 @@ void WebUserContentControllerProxy::removeUserContentWorldUses(HashCountedSet<Re
>             worldsToRemove.append(worldUsePair.key->identifier());
>     }
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveUserContentWorlds(worldsToRemove), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveUserContentWorlds(worldsToRemove), identifier());
> }
> 
> void WebUserContentControllerProxy::addUserScript(API::UserScript& userScript, InjectUserScriptImmediately immediately)
>@@ -176,16 +179,16 @@ void WebUserContentControllerProxy::addUserScript(API::UserScript& userScript, I
> 
>     m_userScripts->elements().append(&userScript);
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::AddUserScripts({ { userScript.identifier(), world->identifier(), userScript.userScript() } }, immediately), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::AddUserScripts({ { userScript.identifier(), world->identifier(), userScript.userScript() } }, immediately), identifier());
> }
> 
> void WebUserContentControllerProxy::removeUserScript(API::UserScript& userScript)
> {
>     Ref<API::UserContentWorld> world = userScript.userContentWorld();
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveUserScript(world->identifier(), userScript.identifier()), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveUserScript(world->identifier(), userScript.identifier()), identifier());
> 
>     m_userScripts->elements().removeAll(&userScript);
> 
>@@ -194,8 +197,8 @@ void WebUserContentControllerProxy::removeUserScript(API::UserScript& userScript
> 
> void WebUserContentControllerProxy::removeAllUserScripts(API::UserContentWorld& world)
> {
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllUserScripts({ world.identifier() }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllUserScripts({ world.identifier() }), identifier());
> 
>     unsigned userScriptsRemoved = m_userScripts->removeAllOfTypeMatching<API::UserScript>([&](const auto& userScript) {
>         return &userScript->userContentWorld() == &world;
>@@ -215,8 +218,8 @@ void WebUserContentControllerProxy::removeAllUserScripts()
>     for (const auto& worldCountPair : worlds)
>         worldIdentifiers.uncheckedAppend(worldCountPair.key->identifier());
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllUserScripts(worldIdentifiers), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllUserScripts(worldIdentifiers), identifier());
> 
>     m_userScripts->elements().clear();
> 
>@@ -231,16 +234,16 @@ void WebUserContentControllerProxy::addUserStyleSheet(API::UserStyleSheet& userS
> 
>     m_userStyleSheets->elements().append(&userStyleSheet);
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::AddUserStyleSheets({ { userStyleSheet.identifier(), world->identifier(), userStyleSheet.userStyleSheet() } }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::AddUserStyleSheets({ { userStyleSheet.identifier(), world->identifier(), userStyleSheet.userStyleSheet() } }), identifier());
> }
> 
> void WebUserContentControllerProxy::removeUserStyleSheet(API::UserStyleSheet& userStyleSheet)
> {
>     Ref<API::UserContentWorld> world = userStyleSheet.userContentWorld();
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveUserStyleSheet(world->identifier(), userStyleSheet.identifier()), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveUserStyleSheet(world->identifier(), userStyleSheet.identifier()), identifier());
> 
>     m_userStyleSheets->elements().removeAll(&userStyleSheet);
> 
>@@ -249,8 +252,8 @@ void WebUserContentControllerProxy::removeUserStyleSheet(API::UserStyleSheet& us
> 
> void WebUserContentControllerProxy::removeAllUserStyleSheets(API::UserContentWorld& world)
> {
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllUserStyleSheets({ world.identifier() }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllUserStyleSheets({ world.identifier() }), identifier());
> 
>     unsigned userStyleSheetsRemoved = m_userStyleSheets->removeAllOfTypeMatching<API::UserStyleSheet>([&](const auto& userStyleSheet) {
>         return &userStyleSheet->userContentWorld() == &world;
>@@ -270,8 +273,8 @@ void WebUserContentControllerProxy::removeAllUserStyleSheets()
>     for (const auto& worldCountPair : worlds)
>         worldIdentifiers.uncheckedAppend(worldCountPair.key->identifier());
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllUserStyleSheets(worldIdentifiers), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllUserStyleSheets(worldIdentifiers), identifier());
> 
>     m_userStyleSheets->elements().clear();
> 
>@@ -291,8 +294,8 @@ bool WebUserContentControllerProxy::addUserScriptMessageHandler(WebScriptMessage
> 
>     m_scriptMessageHandlers.add(handler.identifier(), &handler);
> 
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::AddUserScriptMessageHandlers({ { handler.identifier(), world->identifier(), handler.name() } }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::AddUserScriptMessageHandlers({ { handler.identifier(), world->identifier(), handler.name() } }), identifier());
>     
>     return true;
> }
>@@ -301,8 +304,8 @@ void WebUserContentControllerProxy::removeUserMessageHandlerForName(const String
> {
>     for (auto it = m_scriptMessageHandlers.begin(), end = m_scriptMessageHandlers.end(); it != end; ++it) {
>         if (it->value->name() == name && &it->value->userContentWorld() == &world) {
>-            for (WebProcessProxy* process : m_processes)
>-                process->send(Messages::WebUserContentController::RemoveUserScriptMessageHandler(world.identifier(), it->value->identifier()), identifier());
>+            for (auto& process : m_processes)
>+                process.send(Messages::WebUserContentController::RemoveUserScriptMessageHandler(world.identifier(), it->value->identifier()), identifier());
> 
>             m_scriptMessageHandlers.remove(it);
> 
>@@ -314,8 +317,8 @@ void WebUserContentControllerProxy::removeUserMessageHandlerForName(const String
> 
> void WebUserContentControllerProxy::removeAllUserMessageHandlers(API::UserContentWorld& world)
> {
>-    for (WebProcessProxy* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllUserScriptMessageHandlers({ world.identifier() }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllUserScriptMessageHandlers({ world.identifier() }), identifier());
> 
>     unsigned numberRemoved = 0;
>     m_scriptMessageHandlers.removeIf([&](auto& entry) {
>@@ -352,33 +355,33 @@ void WebUserContentControllerProxy::addContentRuleList(API::ContentRuleList& con
> 
>     auto pair = std::make_pair(contentRuleList.name(), contentRuleList.compiledRuleList().data());
> 
>-    for (auto* process : m_processes)
>-        process->send(Messages::WebUserContentController::AddContentRuleLists({ pair }), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::AddContentRuleLists({ pair }), identifier());
> 
>-    for (auto* process : m_networkProcesses)
>-        process->send(Messages::NetworkContentRuleListManager::AddContentRuleLists { identifier(), { pair } }, 0);
>+    for (auto& process : m_networkProcesses)
>+        process.send(Messages::NetworkContentRuleListManager::AddContentRuleLists { identifier(), { pair } }, 0);
> }
> 
> void WebUserContentControllerProxy::removeContentRuleList(const String& name)
> {
>     m_contentRuleLists.remove(name);
> 
>-    for (auto* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveContentRuleList(name), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveContentRuleList(name), identifier());
> 
>-    for (auto* process : m_networkProcesses)
>-        process->send(Messages::NetworkContentRuleListManager::RemoveContentRuleList { identifier(), name }, 0);
>+    for (auto& process : m_networkProcesses)
>+        process.send(Messages::NetworkContentRuleListManager::RemoveContentRuleList { identifier(), name }, 0);
> }
> 
> void WebUserContentControllerProxy::removeAllContentRuleLists()
> {
>     m_contentRuleLists.clear();
> 
>-    for (auto* process : m_processes)
>-        process->send(Messages::WebUserContentController::RemoveAllContentRuleLists(), identifier());
>+    for (auto& process : m_processes)
>+        process.send(Messages::WebUserContentController::RemoveAllContentRuleLists(), identifier());
> 
>-    for (auto* process : m_networkProcesses)
>-        process->send(Messages::NetworkContentRuleListManager::RemoveAllContentRuleLists { identifier() }, 0);
>+    for (auto& process : m_networkProcesses)
>+        process.send(Messages::NetworkContentRuleListManager::RemoveAllContentRuleLists { identifier() }, 0);
> }
> #endif
> 
>diff --git a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.h b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.h
>index 0ed36a59cc3a71d9f219e1948429c8a791d0c628..e6444b2f418c14552a13d88eec7678ae672d0aa8 100644
>--- a/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.h
>+++ b/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.h
>@@ -27,14 +27,15 @@
> 
> #include "APIObject.h"
> #include "MessageReceiver.h"
>+#include "NetworkProcessProxy.h"
> #include "UserContentControllerIdentifier.h"
> #include <WebCore/PageIdentifier.h>
> #include <wtf/Forward.h>
> #include <wtf/HashCountedSet.h>
> #include <wtf/HashMap.h>
>-#include <wtf/HashSet.h>
> #include <wtf/Ref.h>
> #include <wtf/RefCounted.h>
>+#include <wtf/WeakHashSet.h>
> #include <wtf/text/StringHash.h>
> 
> namespace API {
>@@ -96,8 +97,8 @@ public:
>     void removeAllUserMessageHandlers(API::UserContentWorld&);
> 
> #if ENABLE(CONTENT_EXTENSIONS)
>-    void addNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.add(&proxy); }
>-    void removeNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.remove(&proxy); }
>+    void addNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.add(proxy); }
>+    void removeNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.remove(proxy); }
> 
>     void addContentRuleList(API::ContentRuleList&);
>     void removeContentRuleList(const String&);
>@@ -119,14 +120,14 @@ private:
>     bool shouldSendRemoveUserContentWorldsMessage(API::UserContentWorld&, unsigned numberOfUsesToRemove);
> 
>     UserContentControllerIdentifier m_identifier;
>-    HashSet<WebProcessProxy*> m_processes;
>+    WeakHashSet<WebProcessProxy> m_processes;
>     Ref<API::Array> m_userScripts;
>     Ref<API::Array> m_userStyleSheets;
>     HashMap<uint64_t, RefPtr<WebScriptMessageHandler>> m_scriptMessageHandlers;
>     HashCountedSet<RefPtr<API::UserContentWorld>> m_userContentWorlds;
> 
> #if ENABLE(CONTENT_EXTENSIONS)
>-    HashSet<NetworkProcessProxy*> m_networkProcesses;
>+    WeakHashSet<NetworkProcessProxy> m_networkProcesses;
>     HashMap<String, RefPtr<API::ContentRuleList>> m_contentRuleLists;
> #endif
> };

Actions: View | Formatted Diff | Diff

Attachments on bug 199591: 373672 | 373680 | 373681