Changeset 239223 in webkit


Ignore:
Timestamp:
Dec 14, 2018 11:29:41 AM (5 years ago)
Author:
Chris Dumez
Message:

[PSON] Process-swapping on a loadHTMLString causes duplicate decidePolicyForNavigationAction delegate calls
​https://bugs.webkit.org/show_bug.cgi?id=192704

Reviewed by Geoffrey Garen.

Source/WebKit:

Process-swapping on a loadHTMLString causes duplicate decidePolicyForNavigationAction delegate calls. This
is because we were failing to pass the ShouldTreatAsContinuingLoad flag to the WebContent process when
doing a LoadData.

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::loadData):
(WebKit::WebPageProxy::loadDataWithNavigation):
(WebKit::WebPageProxy::continueNavigationInNewProcess):

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

(WebKit::WebPage::loadDataImpl):
(WebKit::WebPage::loadData):
(WebKit::WebPage::loadAlternateHTML):

  • WebProcess/WebPage/WebPage.h:

Tools:

Extend existing API test to reproduce the problem.

  • TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r239221 r239223  
     12018-12-14  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] Process-swapping on a loadHTMLString causes duplicate decidePolicyForNavigationAction delegate calls
     4        https://bugs.webkit.org/show_bug.cgi?id=192704
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Process-swapping on a loadHTMLString causes duplicate decidePolicyForNavigationAction delegate calls. This
     9        is because we were failing to pass the ShouldTreatAsContinuingLoad flag to the WebContent process when
     10        doing a LoadData.
     11
     12        * UIProcess/WebPageProxy.cpp:
     13        (WebKit::WebPageProxy::loadData):
     14        (WebKit::WebPageProxy::loadDataWithNavigation):
     15        (WebKit::WebPageProxy::continueNavigationInNewProcess):
     16        * UIProcess/WebPageProxy.h:
     17        * WebProcess/WebPage/WebPage.cpp:
     18        (WebKit::WebPage::loadDataImpl):
     19        (WebKit::WebPage::loadData):
     20        (WebKit::WebPage::loadAlternateHTML):
     21        * WebProcess/WebPage/WebPage.h:
     22
    1232018-12-14  Joseph Pecoraro  <pecoraro@apple.com>
    224

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r239219 r239223  
    11311131
    11321132    auto navigation = m_navigationState->createLoadDataNavigation(std::make_unique<API::SubstituteData>(data.vector(), MIMEType, encoding, baseURL, userData));
    1133     loadDataWithNavigation(navigation, data, MIMEType, encoding, baseURL, userData);
     1133    loadDataWithNavigation(navigation, data, MIMEType, encoding, baseURL, userData, ShouldTreatAsContinuingLoad::No);
    11341134    return WTFMove(navigation);
    11351135}
    11361136
    1137 void 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)
     1137void WebPageProxy::loadDataWithNavigation(API::Navigation& navigation, const IPC::DataReference& data, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, ShouldTreatAsContinuingLoad shouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& websitePolicies)
    11381138{
    11391139    ASSERT(!m_isClosed);
    … …  
    11521152    loadParameters.encodingName = encoding;
    11531153    loadParameters.baseURLString = baseURL;
     1154    loadParameters.shouldTreatAsContinuingLoad = shouldTreatAsContinuingLoad == ShouldTreatAsContinuingLoad::Yes;
    11541155    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
    11551156    loadParameters.websitePolicies = WTFMove(websitePolicies);
    … …  
    27282729    ASSERT(!navigation.currentRequest().isEmpty());
    27292730    if (auto& substituteData = navigation.substituteData())
    2730         loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get(), WTFMove(websitePolicies));
     2731        loadDataWithNavigation(navigation, { substituteData->content.data(), substituteData->content.size() }, substituteData->MIMEType, substituteData->encoding, substituteData->baseURL, substituteData->userData.get(), ShouldTreatAsContinuingLoad::Yes, WTFMove(websitePolicies));
    27312732    else
    27322733        loadRequestWithNavigation(navigation, ResourceRequest { navigation.currentRequest() }, WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemes, nullptr, ShouldTreatAsContinuingLoad::Yes, WTFMove(websitePolicies));

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r239219 r239223  
    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, std::optional<WebsitePoliciesData>&& = std::nullopt);
     1579    void loadDataWithNavigation(API::Navigation&, const IPC::DataReference&, const String& MIMEType, const String& encoding, const String& baseURL, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& = std::nullopt);
    15801580    void loadRequestWithNavigation(API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& = std::nullopt);
    15811581

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r239219 r239223  
    13801380}
    13811381
    1382 void 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)
     1382void WebPage::loadDataImpl(uint64_t navigationID, bool shouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&& websitePolicies, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData)
    13831383{
    13841384    SendStopResponsivenessTimer stopper;
    … …  
    13961396
    13971397    // Initate the load in WebCore.
    1398     m_mainFrame->coreFrame()->loader().load(FrameLoadRequest(*m_mainFrame->coreFrame(), request, ShouldOpenExternalURLsPolicy::ShouldNotAllow, substituteData));
     1398    FrameLoadRequest frameLoadRequest(*m_mainFrame->coreFrame(), request, ShouldOpenExternalURLsPolicy::ShouldNotAllow, substituteData);
     1399    frameLoadRequest.setShouldTreatAsContinuingLoad(shouldTreatAsContinuingLoad);
     1400    m_mainFrame->coreFrame()->loader().load(WTFMove(frameLoadRequest));
    13991401}
    14001402
    … …  
    14051407    auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(loadParameters.data.data()), loadParameters.data.size());
    14061408    URL baseURL = loadParameters.baseURLString.isEmpty() ? WTF::blankURL() : URL(URL(), loadParameters.baseURLString);
    1407     loadDataImpl(loadParameters.navigationID, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, URL(), loadParameters.userData);
     1409    loadDataImpl(loadParameters.navigationID, loadParameters.shouldTreatAsContinuingLoad, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, URL(), loadParameters.userData);
    14081410}
    14091411
    … …  
    14171419    auto sharedBuffer = SharedBuffer::create(reinterpret_cast<const char*>(loadParameters.data.data()), loadParameters.data.size());
    14181420    m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL(provisionalLoadErrorURL);   
    1419     loadDataImpl(loadParameters.navigationID, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, unreachableURL, loadParameters.userData);
     1421    loadDataImpl(loadParameters.navigationID, loadParameters.shouldTreatAsContinuingLoad, WTFMove(loadParameters.websitePolicies), WTFMove(sharedBuffer), loadParameters.MIMEType, loadParameters.encodingName, baseURL, unreachableURL, loadParameters.userData);
    14201422    m_mainFrame->coreFrame()->loader().setProvisionalLoadErrorBeingHandledURL({ });
    14211423}

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r239219 r239223  
    11971197    String sourceForFrame(WebFrame*);
    11981198
    1199     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&);
     1199    void loadDataImpl(uint64_t navigationID, bool shouldTreatAsContinuingLoad, std::optional<WebsitePoliciesData>&&, Ref<WebCore::SharedBuffer>&&, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& failingURL, const UserData&);
    12001200
    12011201    // Actions

  • trunk/Tools/ChangeLog

    r239210 r239223  
     12018-12-14  Chris Dumez  <cdumez@apple.com>
     2
     3        [PSON] Process-swapping on a loadHTMLString causes duplicate decidePolicyForNavigationAction delegate calls
     4        https://bugs.webkit.org/show_bug.cgi?id=192704
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Extend existing API test to reproduce the problem.
     9
     10        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     11
    1122018-12-14  Chris Dumez  <cdumez@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

    r239095 r239223  
    29982998    [webView setNavigationDelegate:delegate.get()];
    29992999
     3000    numberOfDecidePolicyCalls = 0;
     3001
    30003002    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
    30013003    [webView loadRequest:request];
    … …  
    30143016    auto pid2 = [webView _webProcessIdentifier];
    30153017    EXPECT_NE(pid1, pid2);
     3018
     3019    EXPECT_EQ(2, numberOfDecidePolicyCalls);
    30163020
    30173021    [webView evaluateJavaScript:@"document.body.innerText" completionHandler:^(id innerText, NSError *error) {
Note: See TracChangeset for help on using the changeset viewer.