Changeset 239210 in webkit


Ignore:
Timestamp:
Dec 14, 2018 9:40:53 AM (5 years ago)
Author:
Chris Dumez
Message:

[PSON] WebsitePolicies are lost on process-swap
https://bugs.webkit.org/show_bug.cgi?id=192694
<rdar://problem/46715748>

Reviewed by Brady Eidson.

Source/WebKit:

In case of process-swap on navigation, instead of sending the websitePolicies to the old
process, send them to the new process as we trigger the navigation. We tell the new process
that it is continuing a load and it will therefore not re-trigger a decidePolicyForNavigationAction.

  • Shared/LoadParameters.cpp:

(WebKit::LoadParameters::encode const):
(WebKit::LoadParameters::decode):

  • Shared/LoadParameters.h:
  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::reattachToWebProcessForReload):
(WebKit::WebPageProxy::reattachToWebProcessWithItem):
(WebKit::WebPageProxy::loadRequestWithNavigation):
(WebKit::WebPageProxy::loadDataWithNavigation):
(WebKit::WebPageProxy::goToBackForwardItem):
(WebKit::WebPageProxy::receivedNavigationPolicyDecision):
(WebKit::WebPageProxy::continueNavigationInNewProcess):

  • UIProcess/WebPageProxy.h:
  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::loadRequest):
(WebKit::WebPage::loadDataImpl):
(WebKit::WebPage::loadData):
(WebKit::WebPage::loadAlternateHTML):
(WebKit::WebPage::goToBackForwardItem):
(WebKit::WebPage::createDocumentLoader):

  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/WebPage.messages.in:

Tools:

Extend existing API test to reproduce the issue.

  • TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r239204 r239210  
     12018-12-14  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] WebsitePolicies are lost on process-swap
     4        https://bugs.webkit.org/show_bug.cgi?id=192694
     5        <rdar://problem/46715748>
     6
     7        Reviewed by Brady Eidson.
     8
     9        In case of process-swap on navigation, instead of sending the websitePolicies to the old
     10        process, send them to the new process as we trigger the navigation. We tell the new process
     11        that it is continuing a load and it will therefore not re-trigger a decidePolicyForNavigationAction.
     12
     13        * Shared/LoadParameters.cpp:
     14        (WebKit::LoadParameters::encode const):
     15        (WebKit::LoadParameters::decode):
     16        * Shared/LoadParameters.h:
     17        * UIProcess/WebPageProxy.cpp:
     18        (WebKit::WebPageProxy::reattachToWebProcessForReload):
     19        (WebKit::WebPageProxy::reattachToWebProcessWithItem):
     20        (WebKit::WebPageProxy::loadRequestWithNavigation):
     21        (WebKit::WebPageProxy::loadDataWithNavigation):
     22        (WebKit::WebPageProxy::goToBackForwardItem):
     23        (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
     24        (WebKit::WebPageProxy::continueNavigationInNewProcess):
     25        * UIProcess/WebPageProxy.h:
     26        * WebProcess/WebPage/WebPage.cpp:
     27        (WebKit::WebPage::loadRequest):
     28        (WebKit::WebPage::loadDataImpl):
     29        (WebKit::WebPage::loadData):
     30        (WebKit::WebPage::loadAlternateHTML):
     31        (WebKit::WebPage::goToBackForwardItem):
     32        (WebKit::WebPage::createDocumentLoader):
     33        * WebProcess/WebPage/WebPage.h:
     34        * WebProcess/WebPage/WebPage.messages.in:
     35
    1362018-12-14  Patrick Griffis  <pgriffis@igalia.com>
    237
  • trunk/Source/WebKit/Shared/LoadParameters.cpp

    r238633 r239210  
    5050    encoder << unreachableURLString;
    5151    encoder << provisionalLoadErrorURLString;
     52    encoder << websitePolicies;
    5253    encoder << shouldOpenExternalURLsPolicy;
    5354    encoder << shouldTreatAsContinuingLoad;
     
    106107        return false;
    107108
     109    std::optional<std::optional<WebsitePoliciesData>> websitePolicies;
     110    decoder >> websitePolicies;
     111    if (!websitePolicies)
     112        return false;
     113    data.websitePolicies = WTFMove(*websitePolicies);
     114
    108115    if (!decoder.decode(data.shouldOpenExternalURLsPolicy))
    109116        return false;
  • trunk/Source/WebKit/Shared/LoadParameters.h

    r238633 r239210  
    2929#include "SandboxExtension.h"
    3030#include "UserData.h"
     31#include "WebsitePoliciesData.h"
    3132#include <WebCore/FrameLoaderTypes.h>
    3233#include <WebCore/ResourceRequest.h>
     
    6162    String provisionalLoadErrorURLString;
    6263
     64    std::optional<WebsitePoliciesData> websitePolicies;
     65
    6366    uint64_t shouldOpenExternalURLsPolicy;
    6467    bool shouldTreatAsContinuingLoad { false };
  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r239196 r239210  
    860860
    861861    // We allow stale content when reloading a WebProcess that's been killed or crashed.
    862     m_process->send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), m_backForwardList->currentItem()->itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No), m_pageID);
     862    m_process->send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), m_backForwardList->currentItem()->itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No, std::nullopt), m_pageID);
    863863    m_process->responsivenessTimer().start();
    864864
     
    879879    auto navigation = m_navigationState->createBackForwardNavigation(item, m_backForwardList->currentItem(), FrameLoadType::IndexedBackForward);
    880880
    881     m_process->send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), item.itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No), m_pageID);
     881    m_process->send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), item.itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No, std::nullopt), m_pageID);
    882882    m_process->responsivenessTimer().start();
    883883
     
    10501050}
    10511051
    1052 void WebPageProxy::loadRequestWithNavigation(API::Navigation& navigation, ResourceRequest&& request, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, API::Object* userData, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad)
     1052void WebPageProxy::loadRequestWithNavigation(API::Navigation& navigation, ResourceRequest&& request, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy, API::Object* userData, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& websitePolicies)
    10531053{
    10541054    ASSERT(!m_isClosed);
     
    10681068    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
    10691069    loadParameters.shouldTreatAsContinuingLoad = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::Yes;
     1070    loadParameters.websitePolicies = WTFMove(websitePolicies);
    10701071    loadParameters.lockHistory = navigation.lockHistory();
    10711072    loadParameters.lockBackForwardList = navigation.lockBackForwardList();
     
    11341135}
    11351136
    1136 void WebPageProxy::loadDataWithNavigation(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData)
     1137void WebPageProxy::loadDataWithNavigation(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, std::optional<WebsitePoliciesData>&& websitePolicies)
    11371138{
    11381139    ASSERT(!m_isClosed);
     
    11521153    loadParameters.baseURLString = baseURL;
    11531154    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
     1155    loadParameters.websitePolicies = WTFMove(websitePolicies);
    11541156    addPlatformLoadParameters(loadParameters);
    11551157
     
    13301332        navigation = m_navigationState->createBackForwardNavigation(item, m_backForwardList->currentItem(), frameLoadType);
    13311333
    1332     m_process->send(Messages::WebPage::GoToBackForwardItem(navigation ? navigation->navigationID() : 0, item.itemID(), frameLoadType, ShouldTreatAsContinuingLoad::No), m_pageID);
     1334    m_process->send(Messages::WebPage::GoToBackForwardItem(navigation ? navigation->navigationID() : 0, item.itemID(), frameLoadType, ShouldTreatAsContinuingLoad::No, std::nullopt), m_pageID);
    13331335    m_process->responsivenessTimer().start();
    13341336
     
    26392641            RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "%p - WebPageProxy::decidePolicyForNavigationAction, keep using process %i for navigation, reason: %{public}s", this, processIdentifier(), reason.utf8().data());
    26402642
    2641         receivedPolicyDecision(policyAction, navigation.ptr(), WTFMove(data), WTFMove(sender));
    2642 
    2643         if (processForNavigation.ptr() != &process())
    2644             continueNavigationInNewProcess(navigation, destinationSuspendedPage ? process().processPool().takeSuspendedPage(*destinationSuspendedPage) : nullptr, WTFMove(processForNavigation), processSwapRequestedByClient);
     2643        bool shouldProcessSwap = processForNavigation.ptr() != &process();
     2644        receivedPolicyDecision(policyAction, navigation.ptr(), shouldProcessSwap ? std::nullopt : WTFMove(data), WTFMove(sender));
     2645
     2646        if (shouldProcessSwap)
     2647            continueNavigationInNewProcess(navigation, destinationSuspendedPage ? process().processPool().takeSuspendedPage(*destinationSuspendedPage) : nullptr, WTFMove(processForNavigation), processSwapRequestedByClient, WTFMove(data));
    26452648    });
    26462649}
     
    26782681}
    26792682
    2680 void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPageProxy, Ref<WebProcessProxy>&& process, ProcessSwapRequestedByClient processSwapRequestedByClient)
     2683void WebPageProxy::continueNavigationInNewProcess(API::Navigation& navigation, std::unique_ptr<SuspendedPageProxy>&& suspendedPageProxy, Ref<WebProcessProxy>&& process, ProcessSwapRequestedByClient processSwapRequestedByClient, std::optional<WebsitePoliciesData>&& websitePolicies)
    26812684{
    26822685    LOG(Loading, "Continuing navigation %" PRIu64 " '%s' in a new web process", navigation.navigationID(), navigation.loggingString());
     
    27102713        });
    27112714        m_process->send(Messages::WebPage::UpdateBackForwardListForReattach(WTFMove(itemStates)), m_pageID);
    2712         m_process->send(Messages::WebPage::GoToBackForwardItem(navigation.navigationID(), item->itemID(), *navigation.backForwardFrameLoadType(), ShouldTreatAsContinuingLoad::Yes), m_pageID);
     2715        m_process->send(Messages::WebPage::GoToBackForwardItem(navigation.navigationID(), item->itemID(), *navigation.backForwardFrameLoadType(), ShouldTreatAsContinuingLoad::Yes, WTFMove(websitePolicies)), m_pageID);
    27132716        m_process->responsivenessTimer().start();
    27142717
     
    27252728    ASSERT(!navigation.currentRequest().isEmpty());
    27262729    if (auto& substituteData = navigation.substituteData())
    2727         loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get());
     2730        loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get(), WTFMove(websitePolicies));
    27282731    else
    2729         loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes);
     2732        loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes, WTFMove(websitePolicies));
    27302733
    27312734    ASSERT(!m_mainFrame);
  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r239196 r239210  
    15771577    RefPtr<API::Navigation> reattachToWebProcessWithItem(WebBackForwardListItem&);
    15781578
    1579     void loadDataWithNavigation(API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData = nullptr);
    1580     void loadRequestWithNavigation(API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad);
     1579    void loadDataWithNavigation(API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData = nullptr, std::optional<WebsitePoliciesData>&& = std::nullopt);
     1580    void loadRequestWithNavigation(API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& = std::nullopt);
    15811581
    15821582    void requestNotificationPermission(uint64_t notificationID, const String& originString);
     
    18941894    void reportPageLoadResult(const WebCore::ResourceError& = { });
    18951895
    1896     void continueNavigationInNewProcess(API::Navigation&, std::unique_ptr<SuspendedPageProxy>&&, Ref<WebProcessProxy>&&, ProcessSwapRequestedByClient);
     1896    void continueNavigationInNewProcess(API::Navigation&, std::unique_ptr<SuspendedPageProxy>&&, Ref<WebProcessProxy>&&, ProcessSwapRequestedByClient, std::optional<WebsitePoliciesData>&&);
    18971897
    18981898    void setNeedsFontAttributes(bool);
  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r239182 r239210  
    13541354
    13551355    m_pendingNavigationID = loadParameters.navigationID;
     1356    m_pendingWebsitePolicies = WTFMove(loadParameters.websitePolicies);
    13561357
    13571358    m_sandboxExtensionTracker.beginLoad(m_mainFrame.get(), WTFMove(loadParameters.sandboxExtensionHandle));
     
    13751376
    13761377    ASSERT(!m_pendingNavigationID);
    1377 }
    1378 
    1379 void WebPage::loadDataImpl(uint64_t navigationID, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
     1378    ASSERT(!m_pendingWebsitePolicies);
     1379}
     1380
     1381void WebPage::loadDataImpl(uint64_t navigationID, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
    13801382{
    13811383    SendStopResponsivenessTimer stopper;
    13821384
    13831385    m_pendingNavigationID = navigationID;
     1386    m_pendingWebsitePolicies = WTFMove(websitePolicies);
    13841387
    13851388    ResourceRequest request(baseURL);
     
    14011404    auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(loadParameters.data.data()), loadParameters.data.size());
    14021405    URL baseURL = loadParameters.baseURLString.isEmpty() ? WTF::blankURL() : URL(URL(), loadParameters.baseURLString);
    1403     loadDataImpl(loadParameters.navigationID, WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, URL(), loadParameters.userData);
    1404 }
    1405 
    1406 void WebPage::loadAlternateHTML(const LoadParameters& loadParameters)
     1406    loadDataImpl(loadParameters.navigationID, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, URL(), loadParameters.userData);
     1407}
     1408
     1409void WebPage::loadAlternateHTML(LoadParameters&& loadParameters)
    14071410{
    14081411    platformDidReceiveLoadParameters(loadParameters);
     
    14131416    auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(loadParameters.data.data()), loadParameters.data.size());
    14141417    m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL(provisionalLoadErrorURL);   
    1415     loadDataImpl(loadParameters.navigationID, WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, unreachableURL, loadParameters.userData);
     1418    loadDataImpl(loadParameters.navigationID, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, unreachableURL, loadParameters.userData);
    14161419    m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL({ });
    14171420}
     
    14761479}
    14771480
    1478 void WebPage::goToBackForwardItem(uint64_t navigationID, const BackForwardItemIdentifier& backForwardItemID, FrameLoadType backForwardType, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad)
     1481void WebPage::goToBackForwardItem(uint64_t navigationID, const BackForwardItemIdentifier& backForwardItemID, FrameLoadType backForwardType, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& websitePolicies)
    14791482{
    14801483    SendStopResponsivenessTimer stopper;
     
    14911494    ASSERT(!m_pendingNavigationID);
    14921495    m_pendingNavigationID = navigationID;
     1496    m_pendingWebsitePolicies = WTFMove(websitePolicies);
    14931497
    14941498    m_page->goToItem(*item, backForwardType, shouldTreatAsContinuingLoad);
     
    59805984            m_pendingNavigationID = 0;
    59815985        }
     5986
     5987        if (auto pendingWebsitePolicies = WTFMove(m_pendingWebsitePolicies))
     5988            WebsitePoliciesData::applyToDocumentLoader(WTFMove(*pendingWebsitePolicies), documentLoader);
    59825989    }
    59835990
  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r239182 r239210  
    5151#include "WebURLSchemeHandler.h"
    5252#include "WebUserContentController.h"
     53#include "WebsitePoliciesData.h"
    5354#include <JavaScriptCore/InspectorFrontendChannel.h>
    5455#include <WebCore/ActivityState.h>
     
    11951196    String sourceForFrame(WebFrame*);
    11961197
    1197     void loadDataImpl(uint64_t navigationID, Ref<WebCore::SharedBuffer>&&, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& failingURL, const UserData&);
     1198    void loadDataImpl(uint64_t navigationID, std::optional<WebsitePoliciesData>&&, Ref<WebCore::SharedBuffer>&&, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& failingURL, const UserData&);
    11981199
    11991200    // Actions
     
    12021203    void loadRequest(LoadParameters&&);
    12031204    void loadData(LoadParameters&&);
    1204     void loadAlternateHTML(const LoadParameters&);
     1205    void loadAlternateHTML(LoadParameters&&);
    12051206    void navigateToPDFLinkWithSimulatedClick(const String& url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint);
    12061207    void reload(uint64_t navigationID, uint32_t reloadOptions, SandboxExtension::Handle&&);
    1207     void goToBackForwardItem(uint64_t navigationID, const WebCore::BackForwardItemIdentifier&, WebCore::FrameLoadType, WebCore::ShouldTreatAsContinuingLoad);
     1208    void goToBackForwardItem(uint64_t navigationID, const WebCore::BackForwardItemIdentifier&, WebCore::FrameLoadType, WebCore::ShouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&&);
    12081209    void tryRestoreScrollPosition();
    12091210    void setInitialFocus(bool forward, bool isKeyboardEventValid, const WebKeyboardEvent&, CallbackID);
     
    17491750
    17501751    uint64_t m_pendingNavigationID { 0 };
     1752    std::optional<WebsitePoliciesData> m_pendingWebsitePolicies;
    17511753
    17521754    bool m_mainFrameProgressCompleted { false };
  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

    r239006 r239210  
    146146    CenterSelectionInVisibleArea()
    147147
    148     GoToBackForwardItem(uint64_t navigationID, struct WebCore::BackForwardItemIdentifier backForwardItemID, enum:uint8_t WebCore::FrameLoadType backForwardType, enum:bool WebCore::ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad)
     148    GoToBackForwardItem(uint64_t navigationID, struct WebCore::BackForwardItemIdentifier backForwardItemID, enum:uint8_t WebCore::FrameLoadType backForwardType, enum:bool WebCore::ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, std::optional<WebKit::WebsitePoliciesData> websitePolicies)
    149149    TryRestoreScrollPosition()
    150150
  • trunk/Tools/ChangeLog

    r239207 r239210  
     12018-12-14  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] WebsitePolicies are lost on process-swap
     4        https://bugs.webkit.org/show_bug.cgi?id=192694
     5        <rdar://problem/46715748>
     6
     7        Reviewed by Brady Eidson.
     8
     9        Extend existing API test to reproduce the issue.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
     12
    1132018-12-14  Michael Catanzaro  <mcatanzaro@igalia.com>
    214
  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm

    r239046 r239210  
    973973        TestWebKitAPI::Util::spinRunLoop();
    974974    loadCount = 0;
     975
     976    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"test://www.google.com/main.html"]];
     977    [webView loadRequest:request];
     978
     979    TestWebKitAPI::Util::run(&finishedNavigation);
     980    finishedNavigation = false;
     981
     982    EXPECT_EQ(1U, loadCount);
     983    loadCount = 0;
    975984}
    976985
Note: See TracChangeset for help on using the changeset viewer.