Changeset 240325 in webkit


Ignore:
Timestamp:
Jan 22, 2019 9:30:09 PM (5 years ago)
Author:
Chris Dumez
Message:

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:
Location:
trunk/Source/WebKit
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r240317 r240325  
     12019-01-22  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r240178) Some API tests are crashing
     4        https://bugs.webkit.org/show_bug.cgi?id=193680
     5
     6        Reviewed by Alex Christensen.
     7
     8        r240178 made sure that userScripts / scriptMessageHandlers / contentExtensions are always
     9        properly populated in the WebPageCreationParameters. This was needed in case we need to
     10        reconstruct the WebUserContentController on the WebProcess side. However, this caused a
     11        regression in the case we reuse a process where the WebUserContentController still exists
     12        (because it was kept alive, e.g. by the WebPageGroup). In that case, we would add duplicate
     13        entries to the existing WebUserContentController instance because its "add" methods did not
     14        have duplicate checks. To address the issue, this patch adds duplicate checks to the
     15        WebUserContentController "add" methods.
     16
     17        * WebProcess/UserContent/WebUserContentController.cpp:
     18        (WebKit::WebUserContentController::addUserScriptMessageHandlerInternal):
     19        (WebKit::WebUserContentController::removeUserScriptMessageHandlerInternal):
     20        (WebKit::WebUserContentController::addUserScriptInternal):
     21        (WebKit::WebUserContentController::removeUserScriptInternal):
     22        (WebKit::WebUserContentController::addUserStyleSheetInternal):
     23        (WebKit::WebUserContentController::removeUserStyleSheetInternal):
     24        (WebKit::WebUserContentController::forEachUserMessageHandler const):
     25        * WebProcess/UserContent/WebUserContentController.h:
     26
    1272019-01-22  Michael Catanzaro  <mcatanzaro@igalia.com>
    228

  • trunk/Source/WebKit/WebProcess/UserContent/WebUserContentController.cpp

    r235282 r240325  
    319319void WebUserContentController::addUserScriptMessageHandlerInternal(InjectedBundleScriptWorld& world, uint64_t userScriptMessageHandlerIdentifier, const String& name)
    320320{
    321     auto& messageHandlersInWorld = m_userMessageHandlers.ensure(&world, [] { return Vector<RefPtr<WebUserMessageHandlerDescriptorProxy>>(); }).iterator->value;
    322     messageHandlersInWorld.append(WebUserMessageHandlerDescriptorProxy::create(this, name, world, userScriptMessageHandlerIdentifier));
     321    auto& messageHandlersInWorld = m_userMessageHandlers.ensure(&world, [] { return Vector<std::pair<uint64_t, RefPtr<WebUserMessageHandlerDescriptorProxy>>> { }; }).iterator->value;
     322    if (messageHandlersInWorld.findMatching([&](auto& pair) { return pair.first ==  userScriptMessageHandlerIdentifier; }) != notFound)
     323        return;
     324    messageHandlersInWorld.append(std::make_pair(userScriptMessageHandlerIdentifier, WebUserMessageHandlerDescriptorProxy::create(this, name, world, userScriptMessageHandlerIdentifier)));
    323325}
    324326
    … …  
    330332
    331333    auto& userMessageHandlers = it->value;
    332 
    333     bool userMessageHandlersChanged = false;
    334     for (int i = userMessageHandlers.size() - 1; i >= 0; --i) {
    335         if (userMessageHandlers[i]->identifier() == userScriptMessageHandlerIdentifier) {
    336             userMessageHandlers.remove(i);
    337             userMessageHandlersChanged = true;
    338         }
    339     }
     334    bool userMessageHandlersChanged = userMessageHandlers.removeFirstMatching([userScriptMessageHandlerIdentifier](auto& pair) {
     335        return pair.first ==  userScriptMessageHandlerIdentifier;
     336    });
    340337
    341338    if (!userMessageHandlersChanged)
    … …  
    371368#endif
    372369
    373 void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld& world, uint64_t userScriptIdentifier, UserScript&& userScript, InjectUserScriptImmediately immediately)
     370void WebUserContentController::addUserScriptInternal(InjectedBundleScriptWorld& world, const Optional<uint64_t>& userScriptIdentifier, UserScript&& userScript, InjectUserScriptImmediately immediately)
    374371{
    375372    if (immediately == InjectUserScriptImmediately::Yes) {
    … …  
    389386    }
    390387
    391     auto& scriptsInWorld = m_userScripts.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserScript>>(); }).iterator->value;
     388    auto& scriptsInWorld = m_userScripts.ensure(&world, [] { return Vector<std::pair<Optional<uint64_t>, WebCore::UserScript>>(); }).iterator->value;
     389    if (userScriptIdentifier && scriptsInWorld.findMatching([&](auto& pair) { return pair.first == userScriptIdentifier; }) != notFound)
     390        return;
     391
    392392    scriptsInWorld.append(std::make_pair(userScriptIdentifier, WTFMove(userScript)));
    393393}
    … …  
    395395void WebUserContentController::addUserScript(InjectedBundleScriptWorld& world, UserScript&& userScript)
    396396{
    397     addUserScriptInternal(world, 0, WTFMove(userScript), InjectUserScriptImmediately::No);
     397    addUserScriptInternal(world, WTF::nullopt, WTFMove(userScript), InjectUserScriptImmediately::No);
    398398}
    399399
    … …  
    405405
    406406    auto& scripts = it->value;
    407     for (int i = scripts.size() - 1; i >= 0; --i) {
    408         if (scripts[i].second.url() == url)
    409             scripts.remove(i);
    410     }
     407    scripts.removeAllMatching([&](auto& pair) {
     408        return pair.second.url() == url;
     409    });
    411410
    412411    if (scripts.isEmpty())
    … …  
    421420
    422421    auto& scripts = it->value;
    423     for (int i = scripts.size() - 1; i >= 0; --i) {
    424         if (scripts[i].first == userScriptIdentifier)
    425             scripts.remove(i);
    426     }
     422    scripts.removeFirstMatching([userScriptIdentifier](auto& pair) {
     423        return pair.first == userScriptIdentifier;
     424    });
    427425
    428426    if (scripts.isEmpty())
    … …  
    435433}
    436434
    437 void WebUserContentController::addUserStyleSheetInternal(InjectedBundleScriptWorld& world, uint64_t userStyleSheetIdentifier, UserStyleSheet&& userStyleSheet)
    438 {
    439     auto& styleSheetsInWorld = m_userStyleSheets.ensure(&world, [] { return Vector<std::pair<uint64_t, WebCore::UserStyleSheet>>(); }).iterator->value;
     435void WebUserContentController::addUserStyleSheetInternal(InjectedBundleScriptWorld& world, const Optional<uint64_t>& userStyleSheetIdentifier, UserStyleSheet&& userStyleSheet)
     436{
     437    auto& styleSheetsInWorld = m_userStyleSheets.ensure(&world, [] { return Vector<std::pair<Optional<uint64_t>, WebCore::UserStyleSheet>>(); }).iterator->value;
     438    if (userStyleSheetIdentifier && styleSheetsInWorld.findMatching([&](auto& pair) { return pair.first == userStyleSheetIdentifier; }) != notFound)
     439        return;
     440
    440441    styleSheetsInWorld.append(std::make_pair(userStyleSheetIdentifier, WTFMove(userStyleSheet)));
    441442}
    … …  
    443444void WebUserContentController::addUserStyleSheet(InjectedBundleScriptWorld& world, UserStyleSheet&& userStyleSheet)
    444445{
    445     addUserStyleSheetInternal(world, 0, WTFMove(userStyleSheet));
     446    addUserStyleSheetInternal(world, WTF::nullopt, WTFMove(userStyleSheet));
    446447    invalidateInjectedStyleSheetCacheInAllFramesInAllPages();
    447448}
    … …  
    454455
    455456    auto& stylesheets = it->value;
    456 
    457     bool sheetsChanged = false;
    458     for (int i = stylesheets.size() - 1; i >= 0; --i) {
    459         if (stylesheets[i].second.url() == url) {
    460             stylesheets.remove(i);
    461             sheetsChanged = true;
    462         }
    463     }
     457    bool sheetsChanged = stylesheets.removeAllMatching([&](auto& pair) {
     458        return pair.second.url() == url;
     459    });
    464460
    465461    if (!sheetsChanged)
    … …  
    479475
    480476    auto& stylesheets = it->value;
    481 
    482     bool sheetsChanged = false;
    483     for (int i = stylesheets.size() - 1; i >= 0; --i) {
    484         if (stylesheets[i].first == userStyleSheetIdentifier) {
    485             stylesheets.remove(i);
    486             sheetsChanged = true;
    487         }
    488     }
     477    bool sheetsChanged = stylesheets.removeFirstMatching([userStyleSheetIdentifier](auto& pair) {
     478        return pair.first == userStyleSheetIdentifier;
     479    });
    489480
    490481    if (!sheetsChanged)
    … …  
    535526void WebUserContentController::forEachUserMessageHandler(Function<void(const WebCore::UserMessageHandlerDescriptor&)>&& functor) const
    536527{
    537     for (const auto& userMessageHandlerVector : m_userMessageHandlers.values()) {
    538         for (const auto& userMessageHandler : userMessageHandlerVector)
    539             functor(*userMessageHandler.get());
     528    for (auto& userMessageHandlerVector : m_userMessageHandlers.values()) {
     529        for (auto& pair : userMessageHandlerVector)
     530            functor(*pair.second.get());
    540531    }
    541532}

  • trunk/Source/WebKit/WebProcess/UserContent/WebUserContentController.h

    r238771 r240325  
    105105#endif
    106106
    107     void addUserScriptInternal(InjectedBundleScriptWorld&, uint64_t userScriptIdentifier, WebCore::UserScript&&, InjectUserScriptImmediately);
     107    void addUserScriptInternal(InjectedBundleScriptWorld&, const Optional<uint64_t>& userScriptIdentifier, WebCore::UserScript&&, InjectUserScriptImmediately);
    108108    void removeUserScriptInternal(InjectedBundleScriptWorld&, uint64_t userScriptIdentifier);
    109     void addUserStyleSheetInternal(InjectedBundleScriptWorld&, uint64_t userStyleSheetIdentifier, WebCore::UserStyleSheet&&);
     109    void addUserStyleSheetInternal(InjectedBundleScriptWorld&, const Optional<uint64_t>& userStyleSheetIdentifier, WebCore::UserStyleSheet&&);
    110110    void removeUserStyleSheetInternal(InjectedBundleScriptWorld&, uint64_t userStyleSheetIdentifier);
    111111#if ENABLE(USER_MESSAGE_HANDLERS)
    … …  
    116116    UserContentControllerIdentifier m_identifier;
    117117
    118     typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, WebCore::UserScript>>> WorldToUserScriptMap;
     118    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<Optional<uint64_t>, WebCore::UserScript>>> WorldToUserScriptMap;
    119119    WorldToUserScriptMap m_userScripts;
    120120
    121     typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, WebCore::UserStyleSheet>>> WorldToUserStyleSheetMap;
     121    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<Optional<uint64_t>, WebCore::UserStyleSheet>>> WorldToUserStyleSheetMap;
    122122    WorldToUserStyleSheetMap m_userStyleSheets;
    123123
    124124#if ENABLE(USER_MESSAGE_HANDLERS)
    125     typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<RefPtr<WebUserMessageHandlerDescriptorProxy>>> WorldToUserMessageHandlerVectorMap;
     125    typedef HashMap<RefPtr<InjectedBundleScriptWorld>, Vector<std::pair<uint64_t, RefPtr<WebUserMessageHandlerDescriptorProxy>>>> WorldToUserMessageHandlerVectorMap;
    126126    WorldToUserMessageHandlerVectorMap m_userMessageHandlers;
    127127#endif
Note: See TracChangeset for help on using the changeset viewer.