Changeset 240477 in webkit


Ignore:
Timestamp:
Jan 25, 2019 9:32:53 AM (5 years ago)
Author:
Chris Dumez
Message:

Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
​https://bugs.webkit.org/show_bug.cgi?id=193779
<rdar://problem/46170903>

Reviewed by Antti Koivisto.

Source/WebKit:

  • UIProcess/Cocoa/NavigationState.mm:

(WebKit::tryAppLink):
(WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
We were crashing when trying to get the URL of the main frame, which was sad because we never
ended up using the main frame URL. Therefore, this patch drops the code in question.

  • UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
Add assertion to make sure that the DecidePolicyForNavigationActionAsync IPC it is getting
from the process is related to its main frame.

Tools:

Add API test that quickly navigates forward to a previous process without waiting for it to
suspend. I suspect the crash could have been happening due to receiving leftover IPC from
the process' previous page when reconnecting the it for the forward navigation.

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

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r240476 r240477  
     12019-01-25  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
     4        https://bugs.webkit.org/show_bug.cgi?id=193779
     5        <rdar://problem/46170903>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        * UIProcess/Cocoa/NavigationState.mm:
     10        (WebKit::tryAppLink):
     11        (WebKit::NavigationState::NavigationClient::decidePolicyForNavigationAction):
     12        We were crashing when trying to get the URL of the main frame, which was sad because we never
     13        ended up using the main frame URL. Therefore, this patch drops the code in question.
     14
     15        * UIProcess/ProvisionalPageProxy.cpp:
     16        (WebKit::ProvisionalPageProxy::decidePolicyForNavigationActionAsync):
     17        Add assertion to make sure that the DecidePolicyForNavigationActionAsync IPC it is getting
     18        from the process is related to its main frame.
     19
    1202019-01-25  Wenson Hsieh  <wenson_hsieh@apple.com>
    221

  • trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm

    r239535 r240477  
    465465#endif
    466466
    467 static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const String& currentMainFrameURL, WTF::Function<void(bool)>&& completionHandler)
     467static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, WTF::Function<void(bool)>&& completionHandler)
    468468{
    469469#if HAVE(APP_LINKS)
    … …  
    487487void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageProxy& webPageProxy, Ref<API::NavigationAction>&& navigationAction, Ref<WebFramePolicyListenerProxy>&& listener, API::Object* userInfo)
    488488{
    489     ASSERT(webPageProxy.mainFrame());
    490     String mainFrameURLString = webPageProxy.mainFrame()->url();
    491489    bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame();
    492490
    … …  
    522520            listener->ignore();
    523521        };
    524         tryAppLink(WTFMove(navigationAction), mainFrameURLString, WTFMove(completionHandler));
     522        tryAppLink(WTFMove(navigationAction), WTFMove(completionHandler));
    525523        return;
    526524    }
    … …  
    534532    auto checker = CompletionHandlerCallChecker::create(navigationDelegate.get(), delegateHasWebsitePolicies ? @selector(_webView:decidePolicyForNavigationAction:decisionHandler:) : @selector(webView:decidePolicyForNavigationAction:decisionHandler:));
    535533   
    536     auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), mainFrameURLString, webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
     534    auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable {
    537535        if (checker->completionHandlerHasBeenCalled())
    538536            return;
    … …  
    557555        case WKNavigationActionPolicyAllow:
    558556        case _WKNavigationActionPolicyAllowInNewProcess:
    559             tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
     557            tryAppLink(WTFMove(navigationAction), [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable {
    560558                if (followedLinkToApp) {
    561559                    localListener->ignore();

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

    r240055 r240477  
    265265void ProvisionalPageProxy::decidePolicyForNavigationActionAsync(uint64_t frameID, WebCore::SecurityOriginData&& frameSecurityOrigin, uint64_t navigationID, NavigationActionData&& navigationActionData, FrameInfoData&& frameInfoData, uint64_t originatingPageID, const WebCore::ResourceRequest& originalRequest, WebCore::ResourceRequest&& request, IPC::FormDataReference&& requestBody, WebCore::ResourceResponse&& redirectResponse, const UserData& userData, uint64_t listenerID)
    266266{
     267    ASSERT(m_mainFrame);
     268    ASSERT(m_mainFrame->frameID() == frameID);
    267269    m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID);
    268270}

  • trunk/Tools/ChangeLog

    r240476 r240477  
     12019-01-25  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
     4        https://bugs.webkit.org/show_bug.cgi?id=193779
     5        <rdar://problem/46170903>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        Add API test that quickly navigates forward to a previous process without waiting for it to
     10        suspend. I suspect the crash could have been happening due to receiving leftover IPC from
     11        the process' previous page when reconnecting the it for the forward navigation.
     12
     13        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
     14
    1152019-01-25  Wenson Hsieh  <wenson_hsieh@apple.com>
    216

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

    r240443 r240477  
    21372137}
    21382138
     2139static const char* keepNavigatingFrameBytes = R"PSONRESOURCE(
     2140<body>
     2141<iframe id="testFrame1" src="about:blank"></iframe>
     2142<iframe id="testFrame2" src="about:blank"></iframe>
     2143<iframe id="testFrame3" src="about:blank"></iframe>
     2144<script>
     2145window.addEventListener('pagehide', function(event) {
     2146    for (var j = 0; j < 10000; j++);
     2147});
     2148
     2149var i = 0;
     2150function navigateFrames()
     2151{
     2152    testFrame1.src = "pson://www.google.com/main" + i + ".html";
     2153    testFrame2.src = "pson://www.google.com/main" + i + ".html";
     2154    testFrame3.src = "pson://www.google.com/main" + i + ".html";
     2155    i++;
     2156}
     2157
     2158navigateFrames();
     2159setInterval(() => {
     2160    navigateFrames();
     2161}, 0);
     2162</script>
     2163</body>
     2164)PSONRESOURCE";
     2165
     2166TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation)
     2167{
     2168    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
     2169    [processPoolConfiguration setProcessSwapsOnNavigation:YES];
     2170    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
     2171
     2172    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
     2173    [webViewConfiguration setProcessPool:processPool.get()];
     2174    auto handler = adoptNS([[PSONScheme alloc] init]);
     2175    [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:keepNavigatingFrameBytes];
     2176    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
     2177
     2178    auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
     2179    auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
     2180    [webView setNavigationDelegate:delegate.get()];
     2181
     2182    NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]];
     2183    [webView loadRequest:request];
     2184
     2185    TestWebKitAPI::Util::run(&done);
     2186    done = false;
     2187
     2188    auto webkitPID = [webView _webProcessIdentifier];
     2189
     2190    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
     2191    [webView loadRequest:request];
     2192
     2193    TestWebKitAPI::Util::run(&done);
     2194    done = false;
     2195
     2196    auto applePID = [webView _webProcessIdentifier];
     2197    EXPECT_NE(webkitPID, applePID);
     2198
     2199    request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]];
     2200    [webView loadRequest:request];
     2201
     2202    TestWebKitAPI::Util::run(&done);
     2203    done = false;
     2204
     2205    EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]);
     2206}
     2207
    21392208static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE(
    21402209<script>
Note: See TracChangeset for help on using the changeset viewer.