Diff
Modified: trunk/Source/WebKit/ChangeLog (259170 => 259171)
--- trunk/Source/WebKit/ChangeLog 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/ChangeLog 2020-03-29 02:58:56 UTC (rev 259171)
@@ -1,3 +1,50 @@
+2020-03-28 Alex Christensen <[email protected]>
+
+ REGRESSION(r257963) UI process crashes when setting navigation delegate inside navigation delegate callbacks
+ https://bugs.webkit.org/show_bug.cgi?id=209705
+ <rdar://problem/60814765>
+
+ Reviewed by Darin Adler.
+
+ I introduced a pattern of making multiple delegate calls sequentially. This is bad because the delegate can change.
+ We need to go back to the WebPageProxy and get the navigation client again between calls.
+ I manually verified this fixes the crash in the radar.
+ Covered by modifying an existing API test to modify the navigation delegate in a callback.
+
+ * UIProcess/API/APINavigationClient.h:
+ (API::NavigationClient::didStartProvisionalNavigation):
+ (API::NavigationClient::didStartProvisionalLoadForFrame):
+ (API::NavigationClient::didFailProvisionalNavigationWithError):
+ (API::NavigationClient::didFailProvisionalLoadWithErrorForFrame):
+ (API::NavigationClient::didCommitNavigation):
+ (API::NavigationClient::didCommitLoadForFrame):
+ (API::NavigationClient::didFinishNavigation):
+ (API::NavigationClient::didFinishLoadForFrame):
+ (API::NavigationClient::didFailNavigationWithError):
+ (API::NavigationClient::didFailLoadWithErrorForFrame):
+ * UIProcess/API/C/WKPage.cpp:
+ (WKPageSetPageNavigationClient):
+ * UIProcess/API/glib/WebKitNavigationClient.cpp:
+ * UIProcess/Cocoa/NavigationState.h:
+ * UIProcess/Cocoa/NavigationState.mm:
+ (WebKit::NavigationState::~NavigationState):
+ (WebKit::NavigationState::NavigationClient::didStartProvisionalNavigation):
+ (WebKit::NavigationState::NavigationClient::didStartProvisionalLoadForFrame):
+ (WebKit::NavigationState::NavigationClient::didFailProvisionalNavigationWithError):
+ (WebKit::NavigationState::NavigationClient::didFailProvisionalLoadWithErrorForFrame):
+ (WebKit::NavigationState::NavigationClient::didCommitNavigation):
+ (WebKit::NavigationState::NavigationClient::didCommitLoadForFrame):
+ (WebKit::NavigationState::NavigationClient::didFinishNavigation):
+ (WebKit::NavigationState::NavigationClient::didFinishLoadForFrame):
+ (WebKit::NavigationState::NavigationClient::didFailNavigationWithError):
+ (WebKit::NavigationState::NavigationClient::didFailLoadWithErrorForFrame):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::didStartProvisionalLoadForFrameShared):
+ (WebKit::WebPageProxy::didFailProvisionalLoadForFrameShared):
+ (WebKit::WebPageProxy::didCommitLoadForFrame):
+ (WebKit::WebPageProxy::didFinishLoadForFrame):
+ (WebKit::WebPageProxy::didFailLoadForFrame):
+
2020-03-28 David Kilzer <[email protected]>
Use-after-move in NetworkProcess::addServiceWorkerSession()
Modified: trunk/Source/WebKit/UIProcess/API/APINavigationClient.h (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/API/APINavigationClient.h 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/API/APINavigationClient.h 2020-03-29 02:58:56 UTC (rev 259171)
@@ -78,16 +78,21 @@
public:
virtual ~NavigationClient() { }
- virtual void didStartProvisionalNavigation(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, Navigation*, Object*) { }
+ virtual void didStartProvisionalNavigation(WebKit::WebPageProxy&, const WebCore::ResourceRequest&, Navigation*, Object*) { }
+ virtual void didStartProvisionalLoadForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, WebKit::FrameInfoData&&) { }
virtual void didReceiveServerRedirectForProvisionalNavigation(WebKit::WebPageProxy&, Navigation*, Object*) { }
virtual void willPerformClientRedirect(WebKit::WebPageProxy&, const WTF::String& destinationURL, double) { }
virtual void didPerformClientRedirect(WebKit::WebPageProxy&, const WTF::String& sourceURL, const WTF::String& destinationURL) { }
virtual void didCancelClientRedirect(WebKit::WebPageProxy&) { }
- virtual void didFailProvisionalNavigationWithError(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, Navigation*, const WebCore::ResourceError&, Object*) { }
- virtual void didCommitNavigation(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, Navigation*, Object*) { }
+ virtual void didFailProvisionalNavigationWithError(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, Navigation*, const WebCore::ResourceError&, Object*) { }
+ virtual void didFailProvisionalLoadWithErrorForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, const WebCore::ResourceError&, WebKit::FrameInfoData&&) { }
+ virtual void didCommitNavigation(WebKit::WebPageProxy&, Navigation*, Object*) { }
+ virtual void didCommitLoadForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, WebKit::FrameInfoData&&) { }
virtual void didFinishDocumentLoad(WebKit::WebPageProxy&, Navigation*, Object*) { }
- virtual void didFinishNavigation(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, Navigation*, Object*) { }
- virtual void didFailNavigationWithError(WebKit::WebPageProxy&, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, Navigation*, const WebCore::ResourceError&, Object*) { }
+ virtual void didFinishNavigation(WebKit::WebPageProxy&, Navigation*, Object*) { }
+ virtual void didFinishLoadForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, WebKit::FrameInfoData&&) { }
+ virtual void didFailNavigationWithError(WebKit::WebPageProxy&, const WebKit::FrameInfoData&, Navigation*, const WebCore::ResourceError&, Object*) { }
+ virtual void didFailLoadWithErrorForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, const WebCore::ResourceError&, WebKit::FrameInfoData&&) { }
virtual void didSameDocumentNavigation(WebKit::WebPageProxy&, Navigation*, WebKit::SameDocumentNavigationType, Object*) { }
virtual void didDisplayInsecureContent(WebKit::WebPageProxy&, API::Object*) { }
Modified: trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/API/C/WKPage.cpp 2020-03-29 02:58:56 UTC (rev 259171)
@@ -2148,13 +2148,10 @@
m_client.decidePolicyForNavigationResponse(toAPI(&page), toAPI(navigationResponse.ptr()), toAPI(listener.ptr()), toAPI(userData), m_client.base.clientInfo);
}
- void didStartProvisionalNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation* navigation, API::Object* userData) override
+ void didStartProvisionalNavigation(WebPageProxy& page, const ResourceRequest&, API::Navigation* navigation, API::Object* userData) override
{
- if (!frameInfo.isMainFrame)
- return;
- if (!m_client.didStartProvisionalNavigation)
- return;
- m_client.didStartProvisionalNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
+ if (m_client.didStartProvisionalNavigation)
+ m_client.didStartProvisionalNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
}
void didReceiveServerRedirectForProvisionalNavigation(WebPageProxy& page, API::Navigation* navigation, API::Object* userData) override
@@ -2164,7 +2161,7 @@
m_client.didReceiveServerRedirectForProvisionalNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
}
- void didFailProvisionalNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, WebCore::ResourceRequest&&, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userData) override
+ void didFailProvisionalNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userData) override
{
if (frameInfo.isMainFrame) {
if (m_client.didFailProvisionalNavigation)
@@ -2175,32 +2172,22 @@
}
}
- void didCommitNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation* navigation, API::Object* userData) override
+ void didCommitNavigation(WebPageProxy& page, API::Navigation* navigation, API::Object* userData) override
{
- if (!frameInfo.isMainFrame)
- return;
-
- if (!m_client.didCommitNavigation)
- return;
- m_client.didCommitNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
+ if (m_client.didCommitNavigation)
+ m_client.didCommitNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
}
- void didFinishNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation* navigation, API::Object* userData) override
+ void didFinishNavigation(WebPageProxy& page, API::Navigation* navigation, API::Object* userData) override
{
- if (!frameInfo.isMainFrame)
- return;
- if (!m_client.didFinishNavigation)
- return;
- m_client.didFinishNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
+ if (m_client.didFinishNavigation)
+ m_client.didFinishNavigation(toAPI(&page), toAPI(navigation), toAPI(userData), m_client.base.clientInfo);
}
- void didFailNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, WebCore::ResourceRequest&&, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userData) override
+ void didFailNavigationWithError(WebPageProxy& page, const FrameInfoData&, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userData) override
{
- if (!frameInfo.isMainFrame)
- return;
- if (!m_client.didFailNavigation)
- return;
- m_client.didFailNavigation(toAPI(&page), toAPI(navigation), toAPI(error), toAPI(userData), m_client.base.clientInfo);
+ if (m_client.didFailNavigation)
+ m_client.didFailNavigation(toAPI(&page), toAPI(navigation), toAPI(error), toAPI(userData), m_client.base.clientInfo);
}
void didFinishDocumentLoad(WebPageProxy& page, API::Navigation* navigation, API::Object* userData) override
Modified: trunk/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/API/glib/WebKitNavigationClient.cpp 2020-03-29 02:58:56 UTC (rev 259171)
@@ -43,10 +43,8 @@
}
private:
- void didStartProvisionalNavigation(WebPageProxy&, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation*, API::Object* /* userData */) override
+ void didStartProvisionalNavigation(WebPageProxy&, const ResourceRequest&, API::Navigation*, API::Object* /* userData */) override
{
- if (!frameInfo.isMainFrame)
- return;
webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_STARTED);
}
@@ -55,7 +53,7 @@
webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_REDIRECTED);
}
- void didFailProvisionalNavigationWithError(WebPageProxy&, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation*, const ResourceError& resourceError, API::Object* /* userData */) override
+ void didFailProvisionalNavigationWithError(WebPageProxy&, FrameInfoData&& frameInfo, API::Navigation*, const ResourceError& resourceError, API::Object* /* userData */) override
{
if (!frameInfo.isMainFrame)
return;
@@ -68,21 +66,17 @@
webkitWebViewLoadFailed(m_webView, WEBKIT_LOAD_STARTED, resourceError.failingURL().string().utf8().data(), error.get());
}
- void didCommitNavigation(WebPageProxy&, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation*, API::Object* /* userData */) override
+ void didCommitNavigation(WebPageProxy&, API::Navigation*, API::Object* /* userData */) override
{
- if (!frameInfo.isMainFrame)
- return;
webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_COMMITTED);
}
- void didFinishNavigation(WebPageProxy&, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation*, API::Object* /* userData */) override
+ void didFinishNavigation(WebPageProxy&, API::Navigation*, API::Object* /* userData */) override
{
- if (!frameInfo.isMainFrame)
- return;
webkitWebViewLoadChanged(m_webView, WEBKIT_LOAD_FINISHED);
}
- void didFailNavigationWithError(WebPageProxy&, FrameInfoData&& frameInfo, ResourceRequest&&, API::Navigation*, const ResourceError& resourceError, API::Object* /* userData */) override
+ void didFailNavigationWithError(WebPageProxy&, const FrameInfoData& frameInfo, API::Navigation*, const ResourceError& resourceError, API::Object* /* userData */) override
{
if (!frameInfo.isMainFrame)
return;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.h (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.h 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.h 2020-03-29 02:58:56 UTC (rev 259171)
@@ -94,16 +94,21 @@
~NavigationClient();
private:
- void didStartProvisionalNavigation(WebPageProxy&, FrameInfoData&&, WebCore::ResourceRequest&&, API::Navigation*, API::Object*) override;
+ void didStartProvisionalNavigation(WebPageProxy&, const WebCore::ResourceRequest&, API::Navigation*, API::Object*) override;
+ void didStartProvisionalLoadForFrame(WebPageProxy&, WebCore::ResourceRequest&&, FrameInfoData&&) override;
void didReceiveServerRedirectForProvisionalNavigation(WebPageProxy&, API::Navigation*, API::Object*) override;
void willPerformClientRedirect(WebPageProxy&, const WTF::String&, double) override;
void didPerformClientRedirect(WebPageProxy&, const WTF::String&, const WTF::String&) override;
void didCancelClientRedirect(WebPageProxy&) override;
- void didFailProvisionalNavigationWithError(WebPageProxy&, FrameInfoData&&, WebCore::ResourceRequest&&, API::Navigation*, const WebCore::ResourceError&, API::Object*) override;
- void didCommitNavigation(WebPageProxy&, FrameInfoData&&, WebCore::ResourceRequest&&, API::Navigation*, API::Object*) override;
+ void didFailProvisionalNavigationWithError(WebPageProxy&, FrameInfoData&&, API::Navigation*, const WebCore::ResourceError&, API::Object*) override;
+ void didFailProvisionalLoadWithErrorForFrame(WebPageProxy&, WebCore::ResourceRequest&&, const WebCore::ResourceError&, FrameInfoData&&) override;
+ void didCommitNavigation(WebPageProxy&, API::Navigation*, API::Object*) override;
+ void didCommitLoadForFrame(WebKit::WebPageProxy&, WebCore::ResourceRequest&&, FrameInfoData&&) override;
void didFinishDocumentLoad(WebPageProxy&, API::Navigation*, API::Object*) override;
- void didFinishNavigation(WebPageProxy&, FrameInfoData&&, WebCore::ResourceRequest&&, API::Navigation*, API::Object*) override;
- void didFailNavigationWithError(WebPageProxy&, FrameInfoData&&, WebCore::ResourceRequest&&, API::Navigation*, const WebCore::ResourceError&, API::Object*) override;
+ void didFinishNavigation(WebPageProxy&, API::Navigation*, API::Object*) override;
+ void didFinishLoadForFrame(WebPageProxy&, WebCore::ResourceRequest&&, FrameInfoData&&) override;
+ void didFailNavigationWithError(WebPageProxy&, const FrameInfoData&, API::Navigation*, const WebCore::ResourceError&, API::Object*) override;
+ void didFailLoadWithErrorForFrame(WebPageProxy&, WebCore::ResourceRequest&&, const WebCore::ResourceError&, FrameInfoData&&) override;
void didSameDocumentNavigation(WebPageProxy&, API::Navigation*, SameDocumentNavigationType, API::Object*) override;
void renderingProgressDidChange(WebPageProxy&, OptionSet<WebCore::LayoutMilestone>) override;
Modified: trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/Cocoa/NavigationState.mm 2020-03-29 02:58:56 UTC (rev 259171)
@@ -755,19 +755,25 @@
}).get()];
}
-void NavigationState::NavigationClient::didStartProvisionalNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, WebCore::ResourceRequest&& request, API::Navigation* navigation, API::Object* userInfo)
+void NavigationState::NavigationClient::didStartProvisionalNavigation(WebPageProxy&, const WebCore::ResourceRequest& request, API::Navigation* navigation, API::Object* userInfo)
{
auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
if (!navigationDelegate)
return;
- if (frameInfo.isMainFrame) {
- // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
- if (m_navigationState.m_navigationDelegateMethods.webViewDidStartProvisionalNavigationUserInfo)
- [(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didStartProvisionalNavigation:wrapper(navigation) userInfo:userInfo ? static_cast<id <NSSecureCoding>>(userInfo->wrapper()) : nil];
- else if (m_navigationState.m_navigationDelegateMethods.webViewDidStartProvisionalNavigation)
- [navigationDelegate webView:m_navigationState.m_webView didStartProvisionalNavigation:wrapper(navigation)];
- }
+ // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
+ if (m_navigationState.m_navigationDelegateMethods.webViewDidStartProvisionalNavigationUserInfo)
+ [(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didStartProvisionalNavigation:wrapper(navigation) userInfo:userInfo ? static_cast<id <NSSecureCoding>>(userInfo->wrapper()) : nil];
+ else if (m_navigationState.m_navigationDelegateMethods.webViewDidStartProvisionalNavigation)
+ [navigationDelegate webView:m_navigationState.m_webView didStartProvisionalNavigation:wrapper(navigation)];
+}
+
+void NavigationState::NavigationClient::didStartProvisionalLoadForFrame(WebPageProxy& page, WebCore::ResourceRequest&& request, FrameInfoData&& frameInfo)
+{
+ auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
+ if (!navigationDelegate)
+ return;
+
if (m_navigationState.m_navigationDelegateMethods.webViewDidStartProvisionalLoadWithRequestInFrame)
[(id <WKNavigationDelegatePrivate>)navigationDelegate.get() _webView:m_navigationState.m_webView didStartProvisionalLoadWithRequest:request.nsURLRequest(HTTPBodyUpdatePolicy::DoNotUpdateHTTPBody) inFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page))];
}
@@ -841,7 +847,7 @@
return adoptNS([[NSError alloc] initWithDomain:originalError.domain code:originalError.code userInfo:userInfo.get()]);
}
-void NavigationState::NavigationClient::didFailProvisionalNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, WebCore::ResourceRequest&& request, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object*)
+void NavigationState::NavigationClient::didFailProvisionalNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object*)
{
auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
if (!navigationDelegate)
@@ -857,21 +863,37 @@
if (m_navigationState.m_navigationDelegateMethods.webViewNavigationDidFailProvisionalLoadInSubframeWithError)
[(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView navigation:nil didFailProvisionalLoadInSubframe:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page)) withError:errorWithRecoveryAttempter.get()];
}
+}
+
+void NavigationState::NavigationClient::didFailProvisionalLoadWithErrorForFrame(WebPageProxy& page, WebCore::ResourceRequest&& request, const WebCore::ResourceError& error, FrameInfoData&& frameInfo)
+{
+ auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
+ if (!navigationDelegate)
+ return;
+
+ auto errorWithRecoveryAttempter = createErrorWithRecoveryAttempter(m_navigationState.m_webView, frameInfo, error);
+
if (m_navigationState.m_navigationDelegateMethods.webViewDidFailProvisionalLoadWithRequestInFrameWithError)
[(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didFailProvisionalLoadWithRequest:request.nsURLRequest(HTTPBodyUpdatePolicy::DoNotUpdateHTTPBody) inFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page)) withError:errorWithRecoveryAttempter.get()];
}
-void NavigationState::NavigationClient::didCommitNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&& request, API::Navigation* navigation, API::Object*)
+void NavigationState::NavigationClient::didCommitNavigation(WebPageProxy& page, API::Navigation* navigation, API::Object*)
{
auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
if (!navigationDelegate)
return;
- if (frameInfo.isMainFrame) {
- // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
- if (m_navigationState.m_navigationDelegateMethods.webViewDidCommitNavigation)
- [navigationDelegate webView:m_navigationState.m_webView didCommitNavigation:wrapper(navigation)];
- }
+ // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
+ if (m_navigationState.m_navigationDelegateMethods.webViewDidCommitNavigation)
+ [navigationDelegate webView:m_navigationState.m_webView didCommitNavigation:wrapper(navigation)];
+}
+
+void NavigationState::NavigationClient::didCommitLoadForFrame(WebKit::WebPageProxy& page, WebCore::ResourceRequest&& request, FrameInfoData&& frameInfo)
+{
+ auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
+ if (!navigationDelegate)
+ return;
+
if (m_navigationState.m_navigationDelegateMethods.webViewDidCommitLoadWithRequestInFrame)
[static_cast<id <WKNavigationDelegatePrivate>>(navigationDelegate.get()) _webView:m_navigationState.m_webView didCommitLoadWithRequest:request.nsURLRequest(HTTPBodyUpdatePolicy::DoNotUpdateHTTPBody) inFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page))];
}
@@ -890,22 +912,28 @@
[static_cast<id <WKNavigationDelegatePrivate>>(navigationDelegate.get()) _webView:m_navigationState.m_webView navigationDidFinishDocumentLoad:wrapper(navigation)];
}
-void NavigationState::NavigationClient::didFinishNavigation(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&& request, API::Navigation* navigation, API::Object*)
+void NavigationState::NavigationClient::didFinishNavigation(WebPageProxy&, API::Navigation* navigation, API::Object*)
{
auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
if (!navigationDelegate)
return;
- if (frameInfo.isMainFrame) {
- // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
- if (m_navigationState.m_navigationDelegateMethods.webViewDidFinishNavigation)
- [navigationDelegate webView:m_navigationState.m_webView didFinishNavigation:wrapper(navigation)];
- }
+ // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
+ if (m_navigationState.m_navigationDelegateMethods.webViewDidFinishNavigation)
+ [navigationDelegate webView:m_navigationState.m_webView didFinishNavigation:wrapper(navigation)];
+}
+
+void NavigationState::NavigationClient::didFinishLoadForFrame(WebPageProxy& page, WebCore::ResourceRequest&& request, FrameInfoData&& frameInfo)
+{
+ auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
+ if (!navigationDelegate)
+ return;
+
if (m_navigationState.m_navigationDelegateMethods.webViewDidFinishLoadWithRequestInFrame)
[(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didFinishLoadWithRequest:request.nsURLRequest(HTTPBodyUpdatePolicy::DoNotUpdateHTTPBody) inFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page))];
}
-void NavigationState::NavigationClient::didFailNavigationWithError(WebPageProxy& page, FrameInfoData&& frameInfo, ResourceRequest&& request, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userInfo)
+void NavigationState::NavigationClient::didFailNavigationWithError(WebPageProxy& page, const FrameInfoData& frameInfo, API::Navigation* navigation, const WebCore::ResourceError& error, API::Object* userInfo)
{
auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
if (!navigationDelegate)
@@ -913,13 +941,21 @@
auto errorWithRecoveryAttempter = createErrorWithRecoveryAttempter(m_navigationState.m_webView, frameInfo, error);
- if (frameInfo.isMainFrame) {
- // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
- if (m_navigationState.m_navigationDelegateMethods.webViewDidFailNavigationWithErrorUserInfo)
- [(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didFailNavigation:wrapper(navigation) withError:errorWithRecoveryAttempter.get() userInfo:userInfo ? static_cast<id <NSSecureCoding>>(userInfo->wrapper()) : nil];
- else if (m_navigationState.m_navigationDelegateMethods.webViewDidFailNavigationWithError)
- [navigationDelegate webView:m_navigationState.m_webView didFailNavigation:wrapper(navigation) withError:errorWithRecoveryAttempter.get()];
- }
+ // FIXME: We should assert that navigation is not null here, but it's currently null for some navigations through the back/forward cache.
+ if (m_navigationState.m_navigationDelegateMethods.webViewDidFailNavigationWithErrorUserInfo)
+ [(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didFailNavigation:wrapper(navigation) withError:errorWithRecoveryAttempter.get() userInfo:userInfo ? static_cast<id <NSSecureCoding>>(userInfo->wrapper()) : nil];
+ else if (m_navigationState.m_navigationDelegateMethods.webViewDidFailNavigationWithError)
+ [navigationDelegate webView:m_navigationState.m_webView didFailNavigation:wrapper(navigation) withError:errorWithRecoveryAttempter.get()];
+}
+
+void NavigationState::NavigationClient::didFailLoadWithErrorForFrame(WebPageProxy& page, WebCore::ResourceRequest&& request, const WebCore::ResourceError& error, FrameInfoData&& frameInfo)
+{
+ auto navigationDelegate = m_navigationState.m_navigationDelegate.get();
+ if (!navigationDelegate)
+ return;
+
+ auto errorWithRecoveryAttempter = createErrorWithRecoveryAttempter(m_navigationState.m_webView, frameInfo, error);
+
if (m_navigationState.m_navigationDelegateMethods.webViewDidFailLoadWithRequestInFrameWithError)
[(id <WKNavigationDelegatePrivate>)navigationDelegate _webView:m_navigationState.m_webView didFailLoadWithRequest:request.nsURLRequest(HTTPBodyUpdatePolicy::DoNotUpdateHTTPBody) inFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page)) withError:errorWithRecoveryAttempter.get()];
}
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (259170 => 259171)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-03-29 02:58:56 UTC (rev 259171)
@@ -4398,8 +4398,11 @@
m_pageLoadState.commitChanges();
if (m_loaderClient)
m_loaderClient->didStartProvisionalLoadForFrame(*this, *frame, navigation.get(), process->transformHandlesToObjects(userData.object()).get());
- else
- m_navigationClient->didStartProvisionalNavigation(*this, WTFMove(frameInfo), WTFMove(request), navigation.get(), process->transformHandlesToObjects(userData.object()).get());
+ else {
+ if (frameInfo.isMainFrame)
+ m_navigationClient->didStartProvisionalNavigation(*this, request, navigation.get(), process->transformHandlesToObjects(userData.object()).get());
+ m_navigationClient->didStartProvisionalLoadForFrame(*this, WTFMove(request), WTFMove(frameInfo));
+ }
#if ENABLE(WEB_AUTHN)
m_websiteDataStore->authenticatorManager().cancelRequest(m_webPageID, frameID);
@@ -4563,8 +4566,10 @@
if (m_loaderClient)
m_loaderClient->didFailProvisionalLoadWithErrorForFrame(*this, *frame, navigation.get(), error, process->transformHandlesToObjects(userData.object()).get());
- else
- m_navigationClient->didFailProvisionalNavigationWithError(*this, WTFMove(frameInfo), WTFMove(request), navigation.get(), error, process->transformHandlesToObjects(userData.object()).get());
+ else {
+ m_navigationClient->didFailProvisionalNavigationWithError(*this, FrameInfoData { frameInfo }, navigation.get(), error, process->transformHandlesToObjects(userData.object()).get());
+ m_navigationClient->didFailProvisionalLoadWithErrorForFrame(*this, WTFMove(request), error, WTFMove(frameInfo));
+ }
m_failingProvisionalLoadURL = { };
@@ -4701,9 +4706,11 @@
m_pageLoadState.commitChanges();
if (m_loaderClient)
m_loaderClient->didCommitLoadForFrame(*this, *frame, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
- else
- m_navigationClient->didCommitNavigation(*this, WTFMove(frameInfo), WTFMove(request), navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
-
+ else {
+ if (frameInfo.isMainFrame)
+ m_navigationClient->didCommitNavigation(*this, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
+ m_navigationClient->didCommitLoadForFrame(*this, WTFMove(request), WTFMove(frameInfo));
+ }
#if ENABLE(ATTACHMENT_ELEMENT)
if (frame->isMainFrame())
invalidateAllAttachments();
@@ -4773,8 +4780,11 @@
m_pageLoadState.commitChanges();
if (m_loaderClient)
m_loaderClient->didFinishLoadForFrame(*this, *frame, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
- else
- m_navigationClient->didFinishNavigation(*this, WTFMove(frameInfo), WTFMove(request), navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
+ else {
+ if (frameInfo.isMainFrame)
+ m_navigationClient->didFinishNavigation(*this, navigation.get(), m_process->transformHandlesToObjects(userData.object()).get());
+ m_navigationClient->didFinishLoadForFrame(*this, WTFMove(request), WTFMove(frameInfo));
+ }
if (isMainFrame) {
reportPageLoadResult();
@@ -4824,8 +4834,11 @@
m_pageLoadState.commitChanges();
if (m_loaderClient)
m_loaderClient->didFailLoadWithErrorForFrame(*this, *frame, navigation.get(), error, m_process->transformHandlesToObjects(userData.object()).get());
- else
- m_navigationClient->didFailNavigationWithError(*this, WTFMove(frameInfo), WTFMove(request), navigation.get(), error, m_process->transformHandlesToObjects(userData.object()).get());
+ else {
+ if (frameInfo.isMainFrame)
+ m_navigationClient->didFailNavigationWithError(*this, frameInfo, navigation.get(), error, m_process->transformHandlesToObjects(userData.object()).get());
+ m_navigationClient->didFailLoadWithErrorForFrame(*this, WTFMove(request), error, WTFMove(frameInfo));
+ }
if (isMainFrame) {
reportPageLoadResult(error);
Modified: trunk/Tools/ChangeLog (259170 => 259171)
--- trunk/Tools/ChangeLog 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Tools/ChangeLog 2020-03-29 02:58:56 UTC (rev 259171)
@@ -1,5 +1,17 @@
2020-03-28 Alex Christensen <[email protected]>
+ REGRESSION(r257963) UI process crashes when setting navigation delegate inside navigation delegate callbacks
+ https://bugs.webkit.org/show_bug.cgi?id=209705
+ <rdar://problem/60814765>
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm:
+ (-[TestAsyncNavigationDelegate webView:didFailNavigation:withError:]):
+ (-[TestAsyncNavigationDelegate webView:didFailProvisionalNavigation:withError:]):
+
+2020-03-28 Alex Christensen <[email protected]>
+
Deprecate injected bundle page group SPI
https://bugs.webkit.org/show_bug.cgi?id=209687
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm (259170 => 259171)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm 2020-03-29 02:56:05 UTC (rev 259170)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncPolicyForNavigationResponse.mm 2020-03-29 02:58:56 UTC (rev 259171)
@@ -48,6 +48,7 @@
{
navigationFailed = true;
navigationComplete = true;
+ webView.navigationDelegate = nil;
}
- (void)webView:(WKWebView *)webView didFailProvisionalNavigation:(null_unspecified WKNavigation *)navigation withError:(NSError *)error
@@ -54,6 +55,7 @@
{
navigationFailed = true;
navigationComplete = true;
+ webView.navigationDelegate = nil;
}
- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
