Changeset 241823 in webkit


Ignore:
Timestamp:
Feb 20, 2019 11:19:26 AM (5 years ago)
Author:
Chris Dumez
Message:

Regression(PSON) Crash under WebKit::WebPageProxy::decidePolicyForNavigationActionSync
​https://bugs.webkit.org/show_bug.cgi?id=194857
<rdar://problem/47759323>

Reviewed by Alex Christensen.

Source/WebKit:

The ProvisionalPageProxy was blindly forwarding the DecidePolicyForNavigationActionSync
synchronous IPC to the WebPageProxy, without passing it the process the IPC came from.
As a result, WebPageProxy::decidePolicyForNavigationActionSync() would try to look up
a WebFrameProxy using the provided frameID from the wrong process and we would end up
hitting a RELEASE_ASSERT().

  • UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync):
(WebKit::ProvisionalPageProxy::didReceiveSyncMessage):

  • UIProcess/ProvisionalPageProxy.h:
  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
(WebKit::WebPageProxy::decidePolicyForNavigationActionSyncShared):

  • UIProcess/WebPageProxy.h:

Tools:

Add API test coverage.

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

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r241820 r241823  
     12019-02-20  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(PSON) Crash under WebKit::WebPageProxy::decidePolicyForNavigationActionSync
     4        https://bugs.webkit.org/show_bug.cgi?id=194857
     5        <rdar://problem/47759323>
     6
     7        Reviewed by Alex Christensen.
     8
     9        The ProvisionalPageProxy was blindly forwarding the DecidePolicyForNavigationActionSync
     10        synchronous IPC to the WebPageProxy, without passing it the process the IPC came from.
     11        As a result, WebPageProxy::decidePolicyForNavigationActionSync() would try to look up
     12        a WebFrameProxy using the provided frameID from the wrong process and we would end up
     13        hitting a RELEASE_ASSERT().
     14
     15        * UIProcess/ProvisionalPageProxy.cpp:
     16        (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionSync):
     17        (WebKit::ProvisionalPageProxy::didReceiveSyncMessage):
     18        * UIProcess/ProvisionalPageProxy.h:
     19        * UIProcess/WebPageProxy.cpp:
     20        (WebKit::WebPageProxy::decidePolicyForNavigationActionSync):
     21        (WebKit::WebPageProxy::decidePolicyForNavigationActionSyncShared):
     22        * UIProcess/WebPageProxy.h:
     23
    1242019-02-20  Don Olmstead  <don.olmstead@sony.com>
    225

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

    r241752 r241823  
    305305}
    306306
     307void ProvisionalPageProxy::decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, WebCore::PolicyCheckIdentifier identifier,
     308    uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID,
     309    const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
     310    const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
     311{
     312    ASSERT(isMainFrame);
     313    ASSERT(!m_mainFrame || m_mainFrame->frameID() == frameID);
     314
     315    if (!isMainFrame || (m_mainFrame && m_mainFrame->frameID() != frameID)) {
     316        reply(identifier, WebCore::PolicyAction::Ignore, navigationID, DownloadID(), WTF::nullopt);
     317        return;
     318    }
     319
     320    if (!m_mainFrame) {
     321        // This synchronous IPC message was processed before the asynchronous DidCreateMainFrame one so we do not know about this frameID yet.
     322        didCreateMainFrame(frameID);
     323    }
     324    ASSERT(m_mainFrame);
     325
     326    m_page.decidePolicyForNavigationActionSyncShared(m_process.copyRef(), frameID, isMainFrame, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData),
     327        WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
     328}
     329
    307330#if PLATFORM(COCOA)
    308331void ProvisionalPageProxy::registerWebProcessAccessibilityToken(const IPC::DataReference& data)
    … …  
    405428    }
    406429
     430    if (decoder.messageName() == Messages::WebPageProxy::DecidePolicyForNavigationActionSync::name()) {
     431        IPC::handleMessageDelayed<Messages::WebPageProxy::DecidePolicyForNavigationActionSync>(connection, decoder, replyEncoder, this, &ProvisionalPageProxy::decidePolicyForNavigationActionSync);
     432        return;
     433    }
     434
    407435    m_page.didReceiveSyncMessage(connection, decoder, replyEncoder);
    408436}

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

    r241752 r241823  
    103103    void startURLSchemeTask(URLSchemeTaskParameters&&);
    104104    void backForwardGoToItem(const WebCore::BackForwardItemIdentifier&, SandboxExtension::Handle&);
     105    void decidePolicyForNavigationActionSync(uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&,
     106        FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
     107        WebCore::ResourceResponse&& redirectResponse, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
    105108#if PLATFORM(COCOA)
    106109    void registerWebProcessAccessibilityToken(const IPC::DataReference&);

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

    r241754 r241823  
    46224622    const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
    46234623{
    4624     auto sender = PolicyDecisionSender::create(identifier, WTFMove(reply));
    4625 
    46264624    auto* frame = m_process->webFrame(frameID);
    46274625    if (!frame) {
    … …  
    46314629        else
    46324630            didCreateSubframe(frameID);
    4633 
    4634         frame = m_process->webFrame(frameID);
    4635         RELEASE_ASSERT(frame);
    4636     }
    4637 
    4638     decidePolicyForNavigationAction(m_process.copyRef(), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData),
     4631    }
     4632
     4633    decidePolicyForNavigationActionSyncShared(m_process.copyRef(), frameID, isMainFrame, WTFMove(frameSecurityOrigin), identifier, navigationID, WTFMove(navigationActionData),
     4634        WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, WTFMove(reply));
     4635}
     4636
     4637void WebPageProxy::decidePolicyForNavigationActionSyncShared(Ref<WebProcessProxy>&& process, uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&& frameSecurityOrigin, PolicyCheckIdentifier identifier,
     4638    uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID,
     4639    const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse,
     4640    const UserData& userData, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&& reply)
     4641{
     4642    auto sender = PolicyDecisionSender::create(identifier, WTFMove(reply));
     4643
     4644    auto* frame = process->webFrame(frameID);
     4645    MESSAGE_CHECK(process, frame);
     4646
     4647    decidePolicyForNavigationAction(WTFMove(process), *frame, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData),
    46394648        originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, sender.copyRef());
    46404649

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

    r241752 r241823  
    14561456    void loadRequestWithNavigationShared(Ref<WebProcessProxy>&&, API::Navigation&, WebCore::ResourceRequest&&, WebCore::ShouldOpenExternalURLsPolicy, API::Object* userData, WebCore::ShouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& = WTF::nullopt);
    14571457    void backForwardGoToItemShared(Ref<WebProcessProxy>&&, const WebCore::BackForwardItemIdentifier&, SandboxExtension::Handle&);
     1458    void decidePolicyForNavigationActionSyncShared(Ref<WebProcessProxy>&&, uint64_t frameID, bool isMainFrame, WebCore::SecurityOriginData&&, WebCore::PolicyCheckIdentifier, uint64_t navigationID, NavigationActionData&&,
     1459        FrameInfoData&&, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&&, IPC::FormDataReference&& requestBody,
     1460        WebCore::ResourceResponse&& redirectResponse, const UserData&, Messages::WebPageProxy::DecidePolicyForNavigationActionSync::DelayedReply&&);
    14581461
    14591462    void dumpAdClickAttribution(CompletionHandler<void(const String&)>&&);

  • trunk/Tools/ChangeLog

    r241821 r241823  
     12019-02-20  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(PSON) Crash under WebKit::WebPageProxy::decidePolicyForNavigationActionSync
     4        https://bugs.webkit.org/show_bug.cgi?id=194857
     5        <rdar://problem/47759323>
     6
     7        Reviewed by Alex Christensen.
     8
     9        Add API test coverage.
     10
     11        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     12
    1132019-02-20  Chris Dumez  <cdumez@apple.com>
    214

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

    r241752 r241823  
    14841484}
    14851485
     1486TEST(ProcessSwap, ServerRedirectToAboutBlank)
     1487{
     1488    auto processPoolConfiguration = psonProcessPoolConfiguration();
     1489    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     1490
     1491    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
     1492    [webViewConfiguration setProcessPool:processPool.get()];
     1493    auto handler = adoptNS([[PSONScheme alloc] init]);
     1494    [handler addRedirectFromURLString:@"pson://www.webkit.org/main.html" toURLString:@"about:blank"];
     1495    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"pson"];
     1496
     1497    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     1498    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
     1499    [webView setNavigationDelegate:delegate.get()];
     1500
     1501    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
     1502    [webView loadRequest:request];
     1503
     1504    TestWebKitAPI::Util::run(&done);
     1505    done = false;
     1506
     1507    auto pidAfterFirstLoad = [webView _webProcessIdentifier];
     1508
     1509    EXPECT_EQ(1, numberOfDecidePolicyCalls);
     1510    EXPECT_EQ(1u, seenPIDs.size());
     1511    EXPECT_TRUE(*seenPIDs.begin() == pidAfterFirstLoad);
     1512
     1513    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
     1514    [webView loadRequest:request];
     1515
     1516    TestWebKitAPI::Util::run(&serverRedirected);
     1517    serverRedirected = false;
     1518
     1519    seenPIDs.add([webView _webProcessIdentifier]);
     1520    if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
     1521        seenPIDs.add(provisionalPID);
     1522
     1523    TestWebKitAPI::Util::run(&done);
     1524    done = false;
     1525
     1526    seenPIDs.add([webView _webProcessIdentifier]);
     1527    if (auto provisionalPID = [webView _provisionalWebProcessIdentifier])
     1528        seenPIDs.add(provisionalPID);
     1529
     1530    EXPECT_FALSE(serverRedirected);
     1531    EXPECT_EQ(3, numberOfDecidePolicyCalls);
     1532    EXPECT_EQ(2u, seenPIDs.size());
     1533}
     1534
    14861535enum class ShouldCacheProcessFirst { No, Yes };
    14871536static void runSameOriginServerRedirectTest(ShouldCacheProcessFirst shouldCacheProcessFirst)
Note: See TracChangeset for help on using the changeset viewer.