Expand WebCore policy concept of "shouldContinue" to allow for more than true/false
https://bugs.webkit.org/show_bug.cgi?id=184424
Reviewed by Alex Christensen.
No new tests (No behavior change, refactor only)
Specifically this expands the "shouldContinue" bool to be an enum class with:
-Yes
-No
-ForSuspension
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::willSendRequest):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadURL):
(WebCore::FrameLoader::load):
(WebCore::FrameLoader::loadWithDocumentLoader):
(WebCore::FrameLoader::loadPostRequest):
(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
(WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
* loader/FrameLoader.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
(WebCore::PolicyChecker::checkNewWindowPolicy):
* loader/PolicyChecker.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@230458 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index d0892fa..c243160 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2018-04-09 Brady Eidson <beidson@apple.com>
+
+ Expand WebCore policy concept of "shouldContinue" to allow for more than true/false
+ https://bugs.webkit.org/show_bug.cgi?id=184424
+
+ Reviewed by Alex Christensen.
+
+ No new tests (No behavior change, refactor only)
+
+ Specifically this expands the "shouldContinue" bool to be an enum class with:
+ -Yes
+ -No
+ -ForSuspension
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::load):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::loadPostRequest):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ (WebCore::FrameLoader::continueLoadAfterNewWindowPolicy):
+ * loader/FrameLoader.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
2018-04-09 Sihui Liu <sihui_liu@apple.com>
REGRESSION(r229929): localStorage is broken for WebInspector
diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
index 376d56a..80dbf13 100644
--- a/Source/WebCore/loader/DocumentLoader.cpp
+++ b/Source/WebCore/loader/DocumentLoader.cpp
@@ -639,10 +639,19 @@
ASSERT(!m_waitingForNavigationPolicy);
m_waitingForNavigationPolicy = true;
- frameLoader()->policyChecker().checkNavigationPolicy(ResourceRequest(newRequest), didReceiveRedirectResponse, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, bool shouldContinue) mutable {
+ frameLoader()->policyChecker().checkNavigationPolicy(ResourceRequest(newRequest), didReceiveRedirectResponse, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, FormState*, ShouldContinue shouldContinue) mutable {
m_waitingForNavigationPolicy = false;
- if (!shouldContinue)
+ switch (shouldContinue) {
+ case ShouldContinue::ForSuspension:
+ FALLTHROUGH;
+ // FIXME: Setup this page for suspension (e.g. Page Cache) here.
+ case ShouldContinue::No:
stopLoadingForPolicyChange();
+ break;
+ case ShouldContinue::Yes:
+ break;
+ }
+
completionHandler(WTFMove(request));
});
}
diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
index d55d0a2..fe0558d 100644
--- a/Source/WebCore/loader/FrameLoader.cpp
+++ b/Source/WebCore/loader/FrameLoader.cpp
@@ -1316,7 +1316,7 @@
if (!targetFrame && !frameName.isEmpty()) {
action = action.copyWithShouldOpenExternalURLsPolicy(shouldOpenExternalURLsPolicyToApply(m_frame, frameLoadRequest));
- policyChecker().checkNewWindowPolicy(WTFMove(action), request, formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), request, formState, frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
});
@@ -1337,10 +1337,10 @@
policyChecker().stopCheck();
policyChecker().setLoadType(newLoadType);
auto completionHandlerCalled = adoptRef(*new SharedBool);
- policyChecker().checkNavigationPolicy(ResourceRequest(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(request), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
if (!completionHandlerCalled->value) {
completionHandlerCalled->value = true;
- continueFragmentScrollAfterNavigationPolicy(request, shouldContinue);
+ continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
}
});
if (!completionHandlerCalled->value) {
@@ -1399,7 +1399,7 @@
if (request.shouldCheckNewWindowPolicy()) {
NavigationAction action { request.requester(), request.resourceRequest(), InitiatedByMainFrame::Unknown, NavigationType::Other, request.shouldOpenExternalURLsPolicy() };
- policyChecker().checkNewWindowPolicy(WTFMove(action), request.resourceRequest(), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), request.resourceRequest(), nullptr, request.frameName(), [this] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, AllowNavigationToInvalidURL::Yes, NewFrameOpenerPolicy::Suppress);
});
@@ -1513,10 +1513,10 @@
oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
policyChecker().stopCheck();
auto completionHandlerCalled = adoptRef(*new SharedBool);
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, oldDocumentLoader.get(), formState, [this, protectedFrame = makeRef(m_frame), completionHandlerCalled = completionHandlerCalled.copyRef()] (const ResourceRequest& request, FormState*, ShouldContinue shouldContinue) {
if (!completionHandlerCalled->value) {
completionHandlerCalled->value = true;
- continueFragmentScrollAfterNavigationPolicy(request, shouldContinue);
+ continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
}
});
if (!completionHandlerCalled->value) {
@@ -1542,7 +1542,7 @@
// a new URL, the parent frame shouldn't learn the URL.
if (!m_stateMachine.committedFirstRealDocumentLoad()
&& !ownerElement->dispatchBeforeLoadEvent(loader->request().url().string())) {
- continueLoadAfterNavigationPolicy(loader->request(), formState, false, allowNavigationToInvalidURL);
+ continueLoadAfterNavigationPolicy(loader->request(), formState, ShouldContinue::No, allowNavigationToInvalidURL);
return;
}
}
@@ -1550,11 +1550,11 @@
m_frame.navigationScheduler().cancel(true);
if (!m_currentLoadShouldCheckNavigationPolicy) {
- continueLoadAfterNavigationPolicy(loader->request(), formState, true, allowNavigationToInvalidURL);
+ continueLoadAfterNavigationPolicy(loader->request(), formState, ShouldContinue::Yes, allowNavigationToInvalidURL);
return;
}
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, FormState* formState, ShouldContinue shouldContinue) {
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
completionHandler();
});
@@ -2814,7 +2814,7 @@
return;
}
- policyChecker().checkNewWindowPolicy(WTFMove(action), workingResourceRequest, WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue) {
+ policyChecker().checkNewWindowPolicy(WTFMove(action), workingResourceRequest, WTFMove(formState), frameName, [this, allowNavigationToInvalidURL, openerPolicy, completionHandler = WTFMove(completionHandler)] (const ResourceRequest& request, FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue) {
continueLoadAfterNewWindowPolicy(request, formState, frameName, action, shouldContinue, allowNavigationToInvalidURL, openerPolicy);
completionHandler();
});
@@ -3169,7 +3169,7 @@
return chrome.runBeforeUnloadConfirmPanel(text, m_frame);
}
-void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& request, FormState* formState, bool shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& request, FormState* formState, ShouldContinue shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
{
// If we loaded an alternate page to replace an unreachableURL, we'll get in here with a
// nil policyDataSource because loading the alternate page will have passed
@@ -3179,13 +3179,7 @@
bool isTargetItem = history().provisionalItem() ? history().provisionalItem()->isTargetItem() : false;
bool urlIsDisallowed = allowNavigationToInvalidURL == AllowNavigationToInvalidURL::No && !request.url().isValid();
-
- // Three reasons we can't continue:
- // 1) Navigation policy delegate said we can't so request is nil. A primary case of this
- // is the user responding Cancel to the form repost nag sheet.
- // 2) User responded Cancel to an alert popped up by the before unload event handler.
- // 3) The request's URL is invalid and navigation to invalid URLs is disallowed.
- bool canContinue = shouldContinue && shouldClose() && !urlIsDisallowed;
+ bool canContinue = shouldContinue != ShouldContinue::No && shouldClose() && !urlIsDisallowed;
if (!canContinue) {
// If we were waiting for a quick redirect, but the policy delegate decided to ignore it, then we
@@ -3269,9 +3263,10 @@
}
void FrameLoader::continueLoadAfterNewWindowPolicy(const ResourceRequest& request,
- FormState* formState, const String& frameName, const NavigationAction& action, bool shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
+ FormState* formState, const String& frameName, const NavigationAction& action, ShouldContinue shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL, NewFrameOpenerPolicy openerPolicy)
{
- if (!shouldContinue)
+ ASSERT(shouldContinue != ShouldContinue::ForSuspension);
+ if (shouldContinue != ShouldContinue::Yes)
return;
Ref<Frame> frame(m_frame);
diff --git a/Source/WebCore/loader/FrameLoader.h b/Source/WebCore/loader/FrameLoader.h
index 7e5ed5d..129a83c 100644
--- a/Source/WebCore/loader/FrameLoader.h
+++ b/Source/WebCore/loader/FrameLoader.h
@@ -81,6 +81,7 @@
class SubstituteData;
enum class NavigationPolicyCheck;
+enum class ShouldContinue;
struct WindowFeatures;
@@ -338,8 +339,8 @@
bool dispatchBeforeUnloadEvent(Chrome&, FrameLoader* frameLoaderBeingNavigated);
void dispatchUnloadEvents(UnloadEventPolicy);
- void continueLoadAfterNavigationPolicy(const ResourceRequest&, FormState*, bool shouldContinue, AllowNavigationToInvalidURL);
- void continueLoadAfterNewWindowPolicy(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, bool shouldContinue, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
+ void continueLoadAfterNavigationPolicy(const ResourceRequest&, FormState*, ShouldContinue, AllowNavigationToInvalidURL);
+ void continueLoadAfterNewWindowPolicy(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
void continueFragmentScrollAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
bool shouldPerformFragmentNavigation(bool isFormSubmission, const String& httpMethod, FrameLoadType, const URL&);
diff --git a/Source/WebCore/loader/PolicyChecker.cpp b/Source/WebCore/loader/PolicyChecker.cpp
index 42d52c1..3702580 100644
--- a/Source/WebCore/loader/PolicyChecker.cpp
+++ b/Source/WebCore/loader/PolicyChecker.cpp
@@ -92,7 +92,7 @@
// Don't ask more than once for the same request or if we are loading an empty URL.
// This avoids confusion on the part of the client.
if (equalIgnoringHeaderFields(request, loader->lastCheckedRequest()) || (!request.isNull() && request.url().isEmpty())) {
- function(ResourceRequest(request), nullptr, true);
+ function(ResourceRequest(request), nullptr, ShouldContinue::Yes);
loader->setLastCheckedRequest(WTFMove(request));
return;
}
@@ -107,7 +107,7 @@
#endif
if (isBackForwardLoadType(m_loadType))
m_loadType = FrameLoadType::Reload;
- function(WTFMove(request), nullptr, shouldContinue);
+ function(WTFMove(request), nullptr, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
return;
}
@@ -117,19 +117,19 @@
// reveal that the frame was blocked. This way, it looks like any other cross-origin page load.
m_frame.ownerElement()->dispatchEvent(Event::create(eventNames().loadEvent, false, false));
}
- function(WTFMove(request), nullptr, false);
+ function(WTFMove(request), nullptr, ShouldContinue::No);
return;
}
loader->setLastCheckedRequest(ResourceRequest(request));
if (request.url() == blankURL())
- return function(WTFMove(request), formState, true);
+ return function(WTFMove(request), formState, ShouldContinue::Yes);
#if USE(QUICK_LOOK)
// Always allow QuickLook-generated URLs based on the protocol scheme.
if (!request.isNull() && isQuickLookPreviewURL(request.url()))
- return function(WTFMove(request), formState, true);
+ return function(WTFMove(request), formState, ShouldContinue::Yes);
#endif
#if ENABLE(CONTENT_FILTERING)
@@ -139,7 +139,7 @@
if (unblocked)
frame->loader().reload();
});
- return function({ }, nullptr, false);
+ return function({ }, nullptr, ShouldContinue::No);
}
m_contentFilterUnblockHandler = { };
#endif
@@ -160,16 +160,15 @@
m_frame.loader().client().startDownload(request, suggestedFilename);
FALLTHROUGH;
case PolicyAction::Ignore:
- return function({ }, nullptr, false);
+ return function({ }, nullptr, ShouldContinue::No);
case PolicyAction::Suspend:
- LOG(Loading, "PolicyAction::Suspend encountered - Treating as PolicyAction::Ignore for now");
- return function({ }, nullptr, false);
+ return function({ }, nullptr, ShouldContinue::ForSuspension);
case PolicyAction::Use:
if (!m_frame.loader().client().canHandleRequest(request)) {
handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
- return function({ }, nullptr, false);
+ return function({ }, nullptr, ShouldContinue::No);
}
- return function(WTFMove(request), formState.get(), true);
+ return function(WTFMove(request), formState.get(), ShouldContinue::Yes);
}
ASSERT_NOT_REACHED();
});
@@ -178,10 +177,10 @@
void PolicyChecker::checkNewWindowPolicy(NavigationAction&& navigationAction, const ResourceRequest& request, FormState* formState, const String& frameName, NewWindowPolicyDecisionFunction&& function)
{
if (m_frame.document() && m_frame.document()->isSandboxed(SandboxPopups))
- return function({ }, nullptr, { }, { }, false);
+ return function({ }, nullptr, { }, { }, ShouldContinue::No);
if (!DOMWindow::allowPopUp(m_frame))
- return function({ }, nullptr, { }, { }, false);
+ return function({ }, nullptr, { }, { }, ShouldContinue::No);
m_frame.loader().client().dispatchDecidePolicyForNewWindowAction(navigationAction, request, formState, frameName, [frame = makeRef(m_frame), request, formState = makeRefPtr(formState), frameName, navigationAction, function = WTFMove(function)](PolicyAction policyAction) mutable {
switch (policyAction) {
@@ -189,13 +188,13 @@
frame->loader().client().startDownload(request);
FALLTHROUGH;
case PolicyAction::Ignore:
- function({ }, nullptr, { }, { }, false);
+ function({ }, nullptr, { }, { }, ShouldContinue::No);
return;
case PolicyAction::Suspend:
// It is invalid to get a "Suspend" policy for new windows, as the old document is not going away.
RELEASE_ASSERT_NOT_REACHED();
case PolicyAction::Use:
- function(request, formState.get(), frameName, navigationAction, true);
+ function(request, formState.get(), frameName, navigationAction, ShouldContinue::Yes);
return;
}
ASSERT_NOT_REACHED();
diff --git a/Source/WebCore/loader/PolicyChecker.h b/Source/WebCore/loader/PolicyChecker.h
index d0f0bc2..2fdb250 100644
--- a/Source/WebCore/loader/PolicyChecker.h
+++ b/Source/WebCore/loader/PolicyChecker.h
@@ -50,8 +50,14 @@
class ResourceError;
class ResourceResponse;
-using NewWindowPolicyDecisionFunction = WTF::CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, bool shouldContinue)>;
-using NavigationPolicyDecisionFunction = WTF::CompletionHandler<void(ResourceRequest&&, FormState*, bool shouldContinue)>;
+enum class ShouldContinue {
+ Yes,
+ No,
+ ForSuspension
+};
+
+using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue)>;
+using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, FormState*, ShouldContinue)>;
class PolicyChecker {
WTF_MAKE_NONCOPYABLE(PolicyChecker);
