Changeset 247243 in webkit


Ignore:
Timestamp:
Jul 8, 2019 5:53:26 PM (5 years ago)
Author:
Chris Dumez
Message:

Use WeakHashSet for WebUserContentControllerProxy::m_processes
​https://bugs.webkit.org/show_bug.cgi?id=199591
<rdar://problem/52798721>

Reviewed by Youenn Fablet.

Source/WebKit:

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):

Source/WTF:

Update WeakHashSet::add() to return an AddResult type, similarly to our other containers.

  • wtf/WeakHashSet.h:

(WTF::WeakHashSet::add):

Location:
trunk/Source
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r247225 r247243  
     12019-07-08  Chris Dumez  <cdumez@apple.com>
     2
     3        Use WeakHashSet for WebUserContentControllerProxy::m_processes
     4        https://bugs.webkit.org/show_bug.cgi?id=199591
     5        <rdar://problem/52798721>
     6
     7        Reviewed by Youenn Fablet.
     8
     9        Update WeakHashSet::add() to return an AddResult type, similarly to our other containers.
     10
     11        * wtf/WeakHashSet.h:
     12        (WTF::WeakHashSet::add):
     13
    1142019-07-08  Christopher Reid  <chris.reid@sony.com>
    215

  • trunk/Source/WTF/wtf/WeakHashSet.h

    r245972 r247243  
    4545public:
    4646    typedef HashSet<Ref<WeakPtrImpl>> WeakPtrImplSet;
     47    typedef typename WeakPtrImplSet::AddResult AddResult;
    4748
    4849    class WeakHashSetConstIterator : public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, const T*, const T&> {
    … …  
    9798
    9899    template <typename U>
    99     void add(const U& value)
     100    AddResult add(const U& value)
    100101    {
    101         m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_impl);
     102        return m_set.add(*makeWeakPtr<T>(const_cast<U&>(value)).m_impl);
    102103    }
    103104

  • trunk/Source/WebKit/ChangeLog

    r247233 r247243  
     12019-07-08  Chris Dumez  <cdumez@apple.com>
     2
     3        Use WeakHashSet for WebUserContentControllerProxy::m_processes
     4        https://bugs.webkit.org/show_bug.cgi?id=199591
     5        <rdar://problem/52798721>
     6
     7        Reviewed by Youenn Fablet.
     8
     9        Use WeakHashSet for WebUserContentControllerProxy::m_processses for safety. In theory, a WebProcessProxy could
     10        stay in the map if we failed to call WebProcessProxy::shutDown() before destroying it.
     11
     12        * UIProcess/UserContent/WebUserContentControllerProxy.cpp:
     13        (WebKit::WebUserContentControllerProxy::~WebUserContentControllerProxy):
     14        (WebKit::WebUserContentControllerProxy::addProcess):
     15        (WebKit::WebUserContentControllerProxy::removeProcess):
     16        (WebKit::WebUserContentControllerProxy::addUserContentWorldUse):
     17        (WebKit::WebUserContentControllerProxy::removeUserContentWorldUses):
     18        (WebKit::WebUserContentControllerProxy::addUserScript):
     19        (WebKit::WebUserContentControllerProxy::removeUserScript):
     20        (WebKit::WebUserContentControllerProxy::removeAllUserScripts):
     21        (WebKit::WebUserContentControllerProxy::addUserStyleSheet):
     22        (WebKit::WebUserContentControllerProxy::removeUserStyleSheet):
     23        (WebKit::WebUserContentControllerProxy::removeAllUserStyleSheets):
     24        (WebKit::WebUserContentControllerProxy::addUserScriptMessageHandler):
     25        (WebKit::WebUserContentControllerProxy::removeUserMessageHandlerForName):
     26        (WebKit::WebUserContentControllerProxy::removeAllUserMessageHandlers):
     27        (WebKit::WebUserContentControllerProxy::addContentRuleList):
     28        (WebKit::WebUserContentControllerProxy::removeContentRuleList):
     29        (WebKit::WebUserContentControllerProxy::removeAllContentRuleLists):
     30        * UIProcess/UserContent/WebUserContentControllerProxy.h:
     31        (WebKit::WebUserContentControllerProxy::addNetworkProcess):
     32        (WebKit::WebUserContentControllerProxy::removeNetworkProcess):
     33
    1342019-07-08  Daniel Bates  <dabates@apple.com>
    235

  • trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.cpp

    r245796 r247243  
    7575{
    7676    webUserContentControllerProxies().remove(m_identifier);
    77     for (auto* process : m_processes) {
    78         process->removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
    79         process->didDestroyWebUserContentControllerProxy(*this);
     77    for (auto& process : m_processes) {
     78        process.removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
     79        process.didDestroyWebUserContentControllerProxy(*this);
    8080    }
    8181#if ENABLE(CONTENT_EXTENSIONS)
    82     for (auto* process : m_networkProcesses)
    83         process->didDestroyWebUserContentControllerProxy(*this);
     82    for (auto& process : m_networkProcesses)
     83        process.didDestroyWebUserContentControllerProxy(*this);
    8484#endif
    8585}
    … …  
    8787void WebUserContentControllerProxy::addProcess(WebProcessProxy& webProcessProxy, WebPageCreationParameters& parameters)
    8888{
    89     if (m_processes.add(&webProcessProxy).isNewEntry)
     89    ASSERT(!m_processes.hasNullReferences());
     90
     91    if (m_processes.add(webProcessProxy).isNewEntry)
    9092        webProcessProxy.addMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier(), *this);
    9193
    … …  
    115117void WebUserContentControllerProxy::removeProcess(WebProcessProxy& webProcessProxy)
    116118{
    117     ASSERT(m_processes.contains(&webProcessProxy));
    118 
    119     m_processes.remove(&webProcessProxy);
     119    ASSERT(m_processes.contains(webProcessProxy));
     120    ASSERT(!m_processes.hasNullReferences());
     121
     122    m_processes.remove(webProcessProxy);
    120123    webProcessProxy.removeMessageReceiver(Messages::WebUserContentControllerProxy::messageReceiverName(), identifier());
    121124}
    … …  
    128131    auto addResult = m_userContentWorlds.add(&world);
    129132    if (addResult.isNewEntry) {
    130         for (WebProcessProxy* process : m_processes)
    131             process->send(Messages::WebUserContentController::AddUserContentWorlds({ std::make_pair(world.identifier(), world.name()) }), identifier());
     133        for (auto& process : m_processes)
     134            process.send(Messages::WebUserContentController::AddUserContentWorlds({ std::make_pair(world.identifier(), world.name()) }), identifier());
    132135    }
    133136}
    … …  
    152155{
    153156    if (shouldSendRemoveUserContentWorldsMessage(world, numberOfUsesToRemove)) {
    154         for (WebProcessProxy* process : m_processes)
    155             process->send(Messages::WebUserContentController::RemoveUserContentWorlds({ world.identifier() }), identifier());
     157        for (auto& process : m_processes)
     158            process.send(Messages::WebUserContentController::RemoveUserContentWorlds({ world.identifier() }), identifier());
    156159    }
    157160}
    … …  
    165168    }
    166169
    167     for (WebProcessProxy* process : m_processes)
    168         process->send(Messages::WebUserContentController::RemoveUserContentWorlds(worldsToRemove), identifier());
     170    for (auto& process : m_processes)
     171        process.send(Messages::WebUserContentController::RemoveUserContentWorlds(worldsToRemove), identifier());
    169172}
    170173
    … …  
    177180    m_userScripts->elements().append(&userScript);
    178181
    179     for (WebProcessProxy* process : m_processes)
    180         process->send(Messages::WebUserContentController::AddUserScripts({ { userScript.identifier(), world->identifier(), userScript.userScript() } }, immediately), identifier());
     182    for (auto& process : m_processes)
     183        process.send(Messages::WebUserContentController::AddUserScripts({ { userScript.identifier(), world->identifier(), userScript.userScript() } }, immediately), identifier());
    181184}
    182185
    … …  
    185188    Ref<API::UserContentWorld> world = userScript.userContentWorld();
    186189
    187     for (WebProcessProxy* process : m_processes)
    188         process->send(Messages::WebUserContentController::RemoveUserScript(world->identifier(), userScript.identifier()), identifier());
     190    for (auto& process : m_processes)
     191        process.send(Messages::WebUserContentController::RemoveUserScript(world->identifier(), userScript.identifier()), identifier());
    189192
    190193    m_userScripts->elements().removeAll(&userScript);
    … …  
    195198void WebUserContentControllerProxy::removeAllUserScripts(API::UserContentWorld& world)
    196199{
    197     for (WebProcessProxy* process : m_processes)
    198         process->send(Messages::WebUserContentController::RemoveAllUserScripts({ world.identifier() }), identifier());
     200    for (auto& process : m_processes)
     201        process.send(Messages::WebUserContentController::RemoveAllUserScripts({ world.identifier() }), identifier());
    199202
    200203    unsigned userScriptsRemoved = m_userScripts->removeAllOfTypeMatching<API::UserScript>([&](const auto& userScript) {
    … …  
    216219        worldIdentifiers.uncheckedAppend(worldCountPair.key->identifier());
    217220
    218     for (WebProcessProxy* process : m_processes)
    219         process->send(Messages::WebUserContentController::RemoveAllUserScripts(worldIdentifiers), identifier());
     221    for (auto& process : m_processes)
     222        process.send(Messages::WebUserContentController::RemoveAllUserScripts(worldIdentifiers), identifier());
    220223
    221224    m_userScripts->elements().clear();
    … …  
    232235    m_userStyleSheets->elements().append(&userStyleSheet);
    233236
    234     for (WebProcessProxy* process : m_processes)
    235         process->send(Messages::WebUserContentController::AddUserStyleSheets({ { userStyleSheet.identifier(), world->identifier(), userStyleSheet.userStyleSheet() } }), identifier());
     237    for (auto& process : m_processes)
     238        process.send(Messages::WebUserContentController::AddUserStyleSheets({ { userStyleSheet.identifier(), world->identifier(), userStyleSheet.userStyleSheet() } }), identifier());
    236239}
    237240
    … …  
    240243    Ref<API::UserContentWorld> world = userStyleSheet.userContentWorld();
    241244
    242     for (WebProcessProxy* process : m_processes)
    243         process->send(Messages::WebUserContentController::RemoveUserStyleSheet(world->identifier(), userStyleSheet.identifier()), identifier());
     245    for (auto& process : m_processes)
     246        process.send(Messages::WebUserContentController::RemoveUserStyleSheet(world->identifier(), userStyleSheet.identifier()), identifier());
    244247
    245248    m_userStyleSheets->elements().removeAll(&userStyleSheet);
    … …  
    250253void WebUserContentControllerProxy::removeAllUserStyleSheets(API::UserContentWorld& world)
    251254{
    252     for (WebProcessProxy* process : m_processes)
    253         process->send(Messages::WebUserContentController::RemoveAllUserStyleSheets({ world.identifier() }), identifier());
     255    for (auto& process : m_processes)
     256        process.send(Messages::WebUserContentController::RemoveAllUserStyleSheets({ world.identifier() }), identifier());
    254257
    255258    unsigned userStyleSheetsRemoved = m_userStyleSheets->removeAllOfTypeMatching<API::UserStyleSheet>([&](const auto& userStyleSheet) {
    … …  
    271274        worldIdentifiers.uncheckedAppend(worldCountPair.key->identifier());
    272275
    273     for (WebProcessProxy* process : m_processes)
    274         process->send(Messages::WebUserContentController::RemoveAllUserStyleSheets(worldIdentifiers), identifier());
     276    for (auto& process : m_processes)
     277        process.send(Messages::WebUserContentController::RemoveAllUserStyleSheets(worldIdentifiers), identifier());
    275278
    276279    m_userStyleSheets->elements().clear();
    … …  
    292295    m_scriptMessageHandlers.add(handler.identifier(), &handler);
    293296
    294     for (WebProcessProxy* process : m_processes)
    295         process->send(Messages::WebUserContentController::AddUserScriptMessageHandlers({ { handler.identifier(), world->identifier(), handler.name() } }), identifier());
     297    for (auto& process : m_processes)
     298        process.send(Messages::WebUserContentController::AddUserScriptMessageHandlers({ { handler.identifier(), world->identifier(), handler.name() } }), identifier());
    296299   
    297300    return true;
    … …  
    302305    for (auto it = m_scriptMessageHandlers.begin(), end = m_scriptMessageHandlers.end(); it != end; ++it) {
    303306        if (it->value->name() == name && &it->value->userContentWorld() == &world) {
    304             for (WebProcessProxy* process : m_processes)
    305                 process->send(Messages::WebUserContentController::RemoveUserScriptMessageHandler(world.identifier(), it->value->identifier()), identifier());
     307            for (auto& process : m_processes)
     308                process.send(Messages::WebUserContentController::RemoveUserScriptMessageHandler(world.identifier(), it->value->identifier()), identifier());
    306309
    307310            m_scriptMessageHandlers.remove(it);
    … …  
    315318void WebUserContentControllerProxy::removeAllUserMessageHandlers(API::UserContentWorld& world)
    316319{
    317     for (WebProcessProxy* process : m_processes)
    318         process->send(Messages::WebUserContentController::RemoveAllUserScriptMessageHandlers({ world.identifier() }), identifier());
     320    for (auto& process : m_processes)
     321        process.send(Messages::WebUserContentController::RemoveAllUserScriptMessageHandlers({ world.identifier() }), identifier());
    319322
    320323    unsigned numberRemoved = 0;
    … …  
    353356    auto pair = std::make_pair(contentRuleList.name(), contentRuleList.compiledRuleList().data());
    354357
    355     for (auto* process : m_processes)
    356         process->send(Messages::WebUserContentController::AddContentRuleLists({ pair }), identifier());
    357 
    358     for (auto* process : m_networkProcesses)
    359         process->send(Messages::NetworkContentRuleListManager::AddContentRuleLists { identifier(), { pair } }, 0);
     358    for (auto& process : m_processes)
     359        process.send(Messages::WebUserContentController::AddContentRuleLists({ pair }), identifier());
     360
     361    for (auto& process : m_networkProcesses)
     362        process.send(Messages::NetworkContentRuleListManager::AddContentRuleLists { identifier(), { pair } }, 0);
    360363}
    361364
    … …  
    364367    m_contentRuleLists.remove(name);
    365368
    366     for (auto* process : m_processes)
    367         process->send(Messages::WebUserContentController::RemoveContentRuleList(name), identifier());
    368 
    369     for (auto* process : m_networkProcesses)
    370         process->send(Messages::NetworkContentRuleListManager::RemoveContentRuleList { identifier(), name }, 0);
     369    for (auto& process : m_processes)
     370        process.send(Messages::WebUserContentController::RemoveContentRuleList(name), identifier());
     371
     372    for (auto& process : m_networkProcesses)
     373        process.send(Messages::NetworkContentRuleListManager::RemoveContentRuleList { identifier(), name }, 0);
    371374}
    372375
    … …  
    375378    m_contentRuleLists.clear();
    376379
    377     for (auto* process : m_processes)
    378         process->send(Messages::WebUserContentController::RemoveAllContentRuleLists(), identifier());
    379 
    380     for (auto* process : m_networkProcesses)
    381         process->send(Messages::NetworkContentRuleListManager::RemoveAllContentRuleLists { identifier() }, 0);
     380    for (auto& process : m_processes)
     381        process.send(Messages::WebUserContentController::RemoveAllContentRuleLists(), identifier());
     382
     383    for (auto& process : m_networkProcesses)
     384        process.send(Messages::NetworkContentRuleListManager::RemoveAllContentRuleLists { identifier() }, 0);
    382385}
    383386#endif

  • trunk/Source/WebKit/UIProcess/UserContent/WebUserContentControllerProxy.h

    r245796 r247243  
    2828#include "APIObject.h"
    2929#include "MessageReceiver.h"
     30#include "NetworkProcessProxy.h"
    3031#include "UserContentControllerIdentifier.h"
    3132#include <WebCore/PageIdentifier.h>
    … …  
    3334#include <wtf/HashCountedSet.h>
    3435#include <wtf/HashMap.h>
    35 #include <wtf/HashSet.h>
    3636#include <wtf/Ref.h>
    3737#include <wtf/RefCounted.h>
     38#include <wtf/WeakHashSet.h>
    3839#include <wtf/text/StringHash.h>
    3940
    … …  
    9798
    9899#if ENABLE(CONTENT_EXTENSIONS)
    99     void addNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.add(&proxy); }
    100     void removeNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.remove(&proxy); }
     100    void addNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.add(proxy); }
     101    void removeNetworkProcess(NetworkProcessProxy& proxy) { m_networkProcesses.remove(proxy); }
    101102
    102103    void addContentRuleList(API::ContentRuleList&);
    … …  
    120121
    121122    UserContentControllerIdentifier m_identifier;
    122     HashSet<WebProcessProxy*> m_processes;
     123    WeakHashSet<WebProcessProxy> m_processes;
    123124    Ref<API::Array> m_userScripts;
    124125    Ref<API::Array> m_userStyleSheets;
    … …  
    127128
    128129#if ENABLE(CONTENT_EXTENSIONS)
    129     HashSet<NetworkProcessProxy*> m_networkProcesses;
     130    WeakHashSet<NetworkProcessProxy> m_networkProcesses;
    130131    HashMap<String, RefPtr<API::ContentRuleList>> m_contentRuleLists;
    131132#endif
Note: See TracChangeset for help on using the changeset viewer.