Skip to content

Commit

Permalink
Stop doing a synchronous DecidePolicyForNavigationAction IPC for frag…
Browse files Browse the repository at this point in the history
…ment navigations

https://bugs.webkit.org/show_bug.cgi?id=262206
rdar://116480976

Reviewed by Alex Christensen.

Stop doing a synchronous DecidePolicyForNavigationAction IPC for fragment navigations.
We still call the client delegate (but asynchronously) and proceed with the navigation
synchronously, without waiting for the client decision. This is consistent with what
we did for empty document loads (about:blank). Navigation to fragments are not true
navigations and merely cause scrolling.

The behavior change is gated on a linked-on-after check to mitigate compatibility
risks.

* LayoutTests/http/tests/navigation/fragment-navigation-policy-ignore-expected.txt: Removed.
* LayoutTests/http/tests/navigation/fragment-navigation-policy-ignore.html: Removed.
* Source/WTF/wtf/cocoa/RuntimeApplicationChecksCocoa.h:
* Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction):

Canonical link: https://commits.webkit.org/270416@main
  • Loading branch information
cdumez committed Nov 9, 2023
1 parent 64d1b1b commit cee3065
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 45 deletions.

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions Source/WTF/wtf/cocoa/RuntimeApplicationChecksCocoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ enum class SDKAlignedBehavior {
EvaluateJavaScriptWithoutTransientActivation,
ResettingTransitionCancelsRunningTransitionQuirk,
OnlyLoadWellKnownAboutURLs,
AsyncFragmentNavigationPolicyDecision,

NumberOfBehaviors
};
Expand Down
27 changes: 19 additions & 8 deletions Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
#include <WebCore/LocalFrame.h>
#include <WebCore/PolicyChecker.h>

#if PLATFORM(COCOA)
#include <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
#endif

#define WebFrameLoaderClient_PREFIX_PARAMETERS "%p - [webFrame=%p, webFrameID=%" PRIu64 ", webPage=%p, webPageID=%" PRIu64 "] WebFrameLoaderClient::"
#define WebFrameLoaderClient_WEBFRAME (&webFrame())
#define WebFrameLoaderClient_WEBFRAMEID (webFrame().frameID().object().toUInt64())
Expand Down Expand Up @@ -154,16 +158,23 @@ void WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(const Navigat

// Notify the UIProcess.
if (policyDecisionMode == PolicyDecisionMode::Synchronous) {
auto sendResult = webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->info(), documentLoader ? documentLoader->navigationID() : 0, navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }));
if (!sendResult.succeeded()) {
WebFrameLoaderClient_RELEASE_LOG_ERROR(Network, "dispatchDecidePolicyForNavigationAction: ignoring because of failing to send sync IPC with error %" PUBLIC_LOG_STRING, IPC::errorAsString(sendResult.error));
m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyDecision { });
#if PLATFORM(COCOA)
if (!linkedOnOrAfterSDKWithBehavior(SDKAlignedBehavior::AsyncFragmentNavigationPolicyDecision)) {
auto sendResult = webPage->sendSync(Messages::WebPageProxy::DecidePolicyForNavigationActionSync(m_frame->info(), documentLoader ? documentLoader->navigationID() : 0, navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }));
if (!sendResult.succeeded()) {
WebFrameLoaderClient_RELEASE_LOG_ERROR(Network, "dispatchDecidePolicyForNavigationAction: ignoring because of failing to send sync IPC with error %" PUBLIC_LOG_STRING, IPC::errorAsString(sendResult.error));
m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyDecision { });
return;
}

auto [policyDecision] = sendResult.takeReply();
WebFrameLoaderClient_RELEASE_LOG(Network, "dispatchDecidePolicyForNavigationAction: Got policyAction %u from sync IPC", (unsigned)policyDecision.policyAction);
m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyDecision { policyDecision.isNavigatingToAppBoundDomain, policyDecision.policyAction, 0, policyDecision.downloadID });
return;
}

auto [policyDecision] = sendResult.takeReply();
WebFrameLoaderClient_RELEASE_LOG(Network, "dispatchDecidePolicyForNavigationAction: Got policyAction %u from sync IPC", (unsigned)policyDecision.policyAction);
m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyDecision { policyDecision.isNavigatingToAppBoundDomain, policyDecision.policyAction, 0, policyDecision.downloadID });
#endif
webPage->sendWithAsyncReply(Messages::WebPageProxy::DecidePolicyForNavigationActionAsync(m_frame->info(), documentLoader ? documentLoader->navigationID() : 0, navigationActionData, originatingFrameInfoData, originatingPageID, navigationAction.resourceRequest(), request, IPC::FormDataReference { request.httpBody() }), [] (PolicyDecision&&) { });
m_frame->didReceivePolicyDecision(listenerID, requestIdentifier, PolicyDecision { std::nullopt, PolicyAction::Use });
return;
}

Expand Down
4 changes: 3 additions & 1 deletion Tools/TestWebKitAPI/Tests/WebKitCocoa/StorageQuota.mm
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,9 @@ static inline void setVisible(TestWKWebView *webView)

receivedQuotaDelegateCalled = false;
[webView loadRequest:server.request("/main.html#fragment"_s)];
[webView stringByEvaluatingJavaScript:@"doTestAgain()"];
[webView _test_waitForDidSameDocumentNavigation];

[webView evaluateJavaScript:@"doTestAgain()" completionHandler:nil];

[messageHandler setExpectedMessage: @"start"];
receivedMessage = false;
Expand Down
3 changes: 3 additions & 0 deletions Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
@property (nonatomic, copy) void (^didCommitNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^didCommitLoadWithRequestInFrame)(WKWebView *, NSURLRequest *, WKFrameInfo *);
@property (nonatomic, copy) void (^didFinishNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^didSameDocumentNavigation)(WKWebView *, WKNavigation *);
@property (nonatomic, copy) void (^renderingProgressDidChange)(WKWebView *, _WKRenderingProgressEvents);
@property (nonatomic, copy) void (^webContentProcessDidTerminate)(WKWebView *);
@property (nonatomic, copy) void (^didReceiveAuthenticationChallenge)(WKWebView *, NSURLAuthenticationChallenge *, void (^)(NSURLSessionAuthChallengeDisposition, NSURLCredential *));
Expand All @@ -48,13 +49,15 @@
- (void)waitForDidStartProvisionalNavigation;
- (void)waitForDidFinishNavigation;
- (void)waitForDidFinishNavigationWithPreferences:(WKWebpagePreferences *)preferences;
- (void)waitForDidSameDocumentNavigation;
- (NSError *)waitForDidFailProvisionalNavigation;

@end

@interface WKWebView (TestWebKitAPIExtras)
- (void)_test_waitForDidStartProvisionalNavigation;
- (void)_test_waitForDidFinishNavigation;
- (void)_test_waitForDidSameDocumentNavigation;
- (void)_test_waitForDidFinishNavigationWithPreferences:(WKWebpagePreferences *)preferences;
- (void)_test_waitForDidFinishNavigationWithoutPresentationUpdate;
- (void)_test_waitForDidFinishNavigationWhileIgnoringSSLErrors;
Expand Down
31 changes: 31 additions & 0 deletions Tools/TestWebKitAPI/cocoa/TestNavigationDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ - (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigat
_didFinishNavigation(webView, navigation);
}

- (void)_webView:(WKWebView *)webView navigation:(WKNavigation *)navigation didSameDocumentNavigation:(_WKSameDocumentNavigationType)navigationType
{
if (_didSameDocumentNavigation)
_didSameDocumentNavigation(webView, navigation);
}

- (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
{
if (_webContentProcessDidTerminate)
Expand Down Expand Up @@ -142,6 +148,20 @@ - (void)waitForDidFinishNavigation
self.didFinishNavigation = nil;
}

- (void)waitForDidSameDocumentNavigation
{
EXPECT_FALSE(self.didSameDocumentNavigation);

__block bool finished = false;
self.didSameDocumentNavigation = ^(WKWebView *, WKNavigation *) {
finished = true;
};

TestWebKitAPI::Util::run(&finished);

self.didSameDocumentNavigation = nil;
}

- (void)waitForWebContentProcessDidTerminate
{
EXPECT_FALSE(self.webContentProcessDidTerminate);
Expand Down Expand Up @@ -264,6 +284,17 @@ - (void)_test_waitForDidFinishNavigation
#endif
}

- (void)_test_waitForDidSameDocumentNavigation
{
EXPECT_FALSE(self.navigationDelegate);

auto navigationDelegate = adoptNS([[TestNavigationDelegate alloc] init]);
self.navigationDelegate = navigationDelegate.get();
[navigationDelegate waitForDidSameDocumentNavigation];

self.navigationDelegate = nil;
}

- (void)_test_waitForDidFinishNavigationWhileIgnoringSSLErrors
{
EXPECT_FALSE(self.navigationDelegate);
Expand Down

0 comments on commit cee3065

Please sign in to comment.