WebKit Bugzilla
Attachment 360036 Details for
Bug 193779
: Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction()
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-193779-20190124140554.patch (text/plain), 10.10 KB, created by
Chris Dumez
on 2019-01-24 14:05:55 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Chris Dumez
Created:
2019-01-24 14:05:55 PST
Size:
10.10 KB
patch
obsolete
>Subversion Revision: 240443 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index 034c6545c8969630194377b0f2d7e8ca56950d15..24f7d35cfa8cfcfeec23ecf8742bab527c7231cc 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,22 @@ >+2019-01-24 Chris Dumez <cdumez@apple.com> >+ >+ Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction() >+ https://bugs.webkit.org/show_bug.cgi?id=193779 >+ <rdar://problem/46170903> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * 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. >+ > 2019-01-24 Antti Koivisto <antti@apple.com> > > [PSON] Flash on back navigation on Mac >diff --git a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm >index 285d66b784c3b258254c5e36c39bdd34414d33ee..1f732ca66583112de76742ba758cb2144395cd0f 100644 >--- a/Source/WebKit/UIProcess/Cocoa/NavigationState.mm >+++ b/Source/WebKit/UIProcess/Cocoa/NavigationState.mm >@@ -464,7 +464,7 @@ bool NavigationState::NavigationClient::willGoToBackForwardListItem(WebPageProxy > } > #endif > >-static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const String& currentMainFrameURL, WTF::Function<void(bool)>&& completionHandler) >+static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, WTF::Function<void(bool)>&& completionHandler) > { > #if HAVE(APP_LINKS) > if (!navigationAction->shouldOpenAppLinks()) { >@@ -486,8 +486,6 @@ static void tryAppLink(Ref<API::NavigationAction>&& navigationAction, const Stri > > void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageProxy& webPageProxy, Ref<API::NavigationAction>&& navigationAction, Ref<WebFramePolicyListenerProxy>&& listener, API::Object* userInfo) > { >- ASSERT(webPageProxy.mainFrame()); >- String mainFrameURLString = webPageProxy.mainFrame()->url(); > bool subframeNavigation = navigationAction->targetFrame() && !navigationAction->targetFrame()->isMainFrame(); > > if (!m_navigationState.m_navigationDelegateMethods.webViewDecidePolicyForNavigationActionDecisionHandler >@@ -521,7 +519,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP > #endif > listener->ignore(); > }; >- tryAppLink(WTFMove(navigationAction), mainFrameURLString, WTFMove(completionHandler)); >+ tryAppLink(WTFMove(navigationAction), WTFMove(completionHandler)); > return; > } > >@@ -533,7 +531,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP > > auto checker = CompletionHandlerCallChecker::create(navigationDelegate.get(), delegateHasWebsitePolicies ? @selector(_webView:decidePolicyForNavigationAction:decisionHandler:) : @selector(webView:decidePolicyForNavigationAction:decisionHandler:)); > >- auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), mainFrameURLString, webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable { >+ auto decisionHandlerWithPolicies = [localListener = WTFMove(listener), navigationAction = navigationAction.copyRef(), checker = WTFMove(checker), webPageProxy = makeRef(webPageProxy), subframeNavigation](WKNavigationActionPolicy actionPolicy, _WKWebsitePolicies *websitePolicies) mutable { > if (checker->completionHandlerHasBeenCalled()) > return; > checker->didCallCompletionHandler(); >@@ -556,7 +554,7 @@ void NavigationState::NavigationClient::decidePolicyForNavigationAction(WebPageP > switch (actionPolicy) { > case WKNavigationActionPolicyAllow: > case _WKNavigationActionPolicyAllowInNewProcess: >- tryAppLink(WTFMove(navigationAction), mainFrameURLString, [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable { >+ tryAppLink(WTFMove(navigationAction), [actionPolicy, localListener = WTFMove(localListener), websitePolicies = WTFMove(apiWebsitePolicies)](bool followedLinkToApp) mutable { > if (followedLinkToApp) { > localListener->ignore(); > return; >diff --git a/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp b/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp >index e0fc58b81f89c8f541d00aeb61c3c9bc95ac6ab1..8d44829cda146ff1a29532c743f76f78b21c7efa 100644 >--- a/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp >+++ b/Source/WebKit/UIProcess/ProvisionalPageProxy.cpp >@@ -264,6 +264,8 @@ void ProvisionalPageProxy::didChangeProvisionalURLForFrame(uint64_t frameID, uin > > void 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) > { >+ ASSERT(m_mainFrame); >+ ASSERT(m_mainFrame->frameID() == frameID); > m_page.decidePolicyForNavigationActionAsyncShared(m_process.copyRef(), frameID, WTFMove(frameSecurityOrigin), navigationID, WTFMove(navigationActionData), WTFMove(frameInfoData), originatingPageID, originalRequest, WTFMove(request), WTFMove(requestBody), WTFMove(redirectResponse), userData, listenerID); > } > >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 13073c5cf042a81372b99d84c831e165c94173d4..4dade385bf72a28e7328ae25b018f7d37e6dbd91 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,17 @@ >+2019-01-24 Chris Dumez <cdumez@apple.com> >+ >+ Regression(PSON?) Crash under NavigationState::NavigationClient::decidePolicyForNavigationAction() >+ https://bugs.webkit.org/show_bug.cgi?id=193779 >+ <rdar://problem/46170903> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ 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: >+ > 2019-01-24 Antti Koivisto <antti@apple.com> > > [PSON] Flash on back navigation on Mac >diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >index 2e1b4d2b78031b4089cbcd0f6ec3b8e7086a1c56..c1661879bea2aabca9d9917f2865836e53908628 100644 >--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm >@@ -2136,6 +2136,75 @@ TEST(ProcessSwap, HistoryItemIDConfusion) > EXPECT_WK_STREQ(@"pson://www.google.com/main.html", [backForwardList.forwardList[1].URL absoluteString]); > } > >+static const char* keepNavigatingFrameBytes = R"PSONRESOURCE( >+<body> >+<iframe id="testFrame1" src="about:blank"></iframe> >+<iframe id="testFrame2" src="about:blank"></iframe> >+<iframe id="testFrame3" src="about:blank"></iframe> >+<script> >+window.addEventListener('pagehide', function(event) { >+ for (var j = 0; j < 10000; j++); >+}); >+ >+var i = 0; >+function navigateFrames() >+{ >+ testFrame1.src = "pson://www.google.com/main" + i + ".html"; >+ testFrame2.src = "pson://www.google.com/main" + i + ".html"; >+ testFrame3.src = "pson://www.google.com/main" + i + ".html"; >+ i++; >+} >+ >+navigateFrames(); >+setInterval(() => { >+ navigateFrames(); >+}, 0); >+</script> >+</body> >+)PSONRESOURCE"; >+ >+TEST(ProcessSwap, ReuseSuspendedProcessForRegularNavigation) >+{ >+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]); >+ [processPoolConfiguration setProcessSwapsOnNavigation:YES]; >+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]); >+ >+ auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]); >+ [webViewConfiguration setProcessPool:processPool.get()]; >+ auto handler = adoptNS([[PSONScheme alloc] init]); >+ [handler addMappingFromURLString:@"pson://www.webkit.org/main1.html" toData:keepNavigatingFrameBytes]; >+ [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"]; >+ >+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:webViewConfiguration.get()]); >+ auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]); >+ [webView setNavigationDelegate:delegate.get()]; >+ >+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main1.html"]]; >+ [webView loadRequest:request]; >+ >+ TestWebKitAPI::Util::run(&done); >+ done = false; >+ >+ auto webkitPID = [webView _webProcessIdentifier]; >+ >+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]]; >+ [webView loadRequest:request]; >+ >+ TestWebKitAPI::Util::run(&done); >+ done = false; >+ >+ auto applePID = [webView _webProcessIdentifier]; >+ EXPECT_NE(webkitPID, applePID); >+ >+ request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main2.html"]]; >+ [webView loadRequest:request]; >+ >+ TestWebKitAPI::Util::run(&done); >+ done = false; >+ >+ EXPECT_EQ(webkitPID, [webView _webProcessIdentifier]); >+} >+ > static const char* mainFramesOnlyMainFrame = R"PSONRESOURCE( > <script> > function loaded() {
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193779
: 360036