Diff
Modified: branches/safari-607-branch/Source/WebCore/ChangeLog (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/ChangeLog 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/ChangeLog 2019-02-06 22:17:28 UTC (rev 241080)
@@ -1,5 +1,78 @@
2019-02-05 Alan Coon <[email protected]>
+ Cherry-pick r240880. rdar://problem/47774545
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ Source/WebCore:
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ (WebCore::DocumentLoader::continueAfterContentPolicy):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/FrameLoader.h:
+ * loader/FrameLoaderTypes.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
+ Source/WebKit:
+
+ The issue was happening when the page is triggering a cross-site navigation while in the middle of parsing. This would cause us to
+ start a new provisional load in a new process before the previous process sends the DidFinishLoadForFrame() IPC to the UIProcess.
+ Getting such IPC after a provisional load has started would mess up our state machine and trip assertions.
+
+ This patch restores non-PSON behavior which is that the previous load in the old process now gets stopped so that no DidFinishLoadForFrame()
+ / DidFailLoadForFrame() gets sent. To achieve this behavior, I introduced a new "StopAllLoads" PolicyAction that we now send the old
+ process when the load is continuing in a new process, instead of sending it "Ignore".
+
+ * NetworkProcess/NetworkDataTaskBlob.cpp:
+ (WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse):
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (toNSURLSessionResponseDisposition):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240880 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-01 Chris Dumez <[email protected]>
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ (WebCore::DocumentLoader::continueAfterContentPolicy):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/FrameLoader.h:
+ * loader/FrameLoaderTypes.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
+2019-02-05 Alan Coon <[email protected]>
+
Cherry-pick r240833. rdar://problem/47774523
[Cocoa][EME] AirPlaying a FairPlay-protected HLS stream fails to decrypt
Modified: branches/safari-607-branch/Source/WebCore/loader/DocumentLoader.cpp (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/DocumentLoader.cpp 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/DocumentLoader.cpp 2019-02-06 22:17:28 UTC (rev 241080)
@@ -644,13 +644,14 @@
if (!didReceiveRedirectResponse)
return completionHandler(WTFMove(newRequest));
- auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) mutable {
+ auto navigationPolicyCompletionHandler = [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (ResourceRequest&& request, WeakPtr<FormState>&&, NavigationPolicyDecision navigationPolicyDecision) mutable {
m_waitingForNavigationPolicy = false;
- switch (shouldContinue) {
- case ShouldContinue::No:
+ switch (navigationPolicyDecision) {
+ case NavigationPolicyDecision::IgnoreLoad:
+ case NavigationPolicyDecision::StopAllLoads:
stopLoadingForPolicyChange();
break;
- case ShouldContinue::Yes:
+ case NavigationPolicyDecision::ContinueLoad:
break;
}
@@ -934,6 +935,11 @@
static_cast<ResourceLoader*>(mainResourceLoader())->didFail(interruptedForPolicyChangeError());
return;
}
+ case PolicyAction::StopAllLoads:
+ ASSERT_NOT_REACHED();
+#if ASSERT_DISABLED
+ FALLTHROUGH;
+#endif
case PolicyAction::Ignore:
if (ResourceLoader* mainResourceLoader = this->mainResourceLoader())
InspectorInstrumentation::continueWithPolicyIgnore(*m_frame, mainResourceLoader->identifier(), *this, m_response);
Modified: branches/safari-607-branch/Source/WebCore/loader/FrameLoader.cpp (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/FrameLoader.cpp 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/FrameLoader.cpp 2019-02-06 22:17:28 UTC (rev 241080)
@@ -1399,8 +1399,8 @@
oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
policyChecker().stopCheck();
policyChecker().setLoadType(newLoadType);
- policyChecker().checkNavigationPolicy(WTFMove(request), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
- continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
+ policyChecker().checkNavigationPolicy(WTFMove(request), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, NavigationPolicyDecision navigationPolicyDecision) {
+ continueFragmentScrollAfterNavigationPolicy(request, navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad);
}, PolicyDecisionMode::Synchronous);
return;
}
@@ -1596,8 +1596,8 @@
oldDocumentLoader->setTriggeringAction(WTFMove(action));
oldDocumentLoader->setLastCheckedRequest(ResourceRequest());
policyChecker().stopCheck();
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, ShouldContinue shouldContinue) {
- continueFragmentScrollAfterNavigationPolicy(request, shouldContinue == ShouldContinue::Yes);
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, oldDocumentLoader.get(), WTFMove(formState), [this, protectedFrame = makeRef(m_frame)] (const ResourceRequest& request, WeakPtr<FormState>&&, NavigationPolicyDecision navigationPolicyDecision) {
+ continueFragmentScrollAfterNavigationPolicy(request, navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad);
}, PolicyDecisionMode::Synchronous);
return;
}
@@ -1618,7 +1618,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.get(), ShouldContinue::No, allowNavigationToInvalidURL);
+ continueLoadAfterNavigationPolicy(loader->request(), formState.get(), NavigationPolicyDecision::IgnoreLoad, allowNavigationToInvalidURL);
return;
}
}
@@ -1626,12 +1626,12 @@
m_frame.navigationScheduler().cancel(NewLoadInProgress::Yes);
if (shouldTreatCurrentLoadAsContinuingLoad()) {
- continueLoadAfterNavigationPolicy(loader->request(), formState.get(), ShouldContinue::Yes, allowNavigationToInvalidURL);
+ continueLoadAfterNavigationPolicy(loader->request(), formState.get(), NavigationPolicyDecision::ContinueLoad, allowNavigationToInvalidURL);
return;
}
- policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, loader, WTFMove(formState), [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, ShouldContinue shouldContinue) mutable {
- continueLoadAfterNavigationPolicy(request, formState.get(), shouldContinue, allowNavigationToInvalidURL);
+ policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), ResourceResponse { } /* redirectResponse */, loader, WTFMove(formState), [this, protectedFrame = makeRef(m_frame), allowNavigationToInvalidURL, completionHandler = completionHandlerCaller.release()] (const ResourceRequest& request, WeakPtr<FormState>&& formState, NavigationPolicyDecision navigationPolicyDecision) mutable {
+ continueLoadAfterNavigationPolicy(request, formState.get(), navigationPolicyDecision, allowNavigationToInvalidURL);
completionHandler();
}, PolicyDecisionMode::Asynchronous);
}
@@ -3335,7 +3335,7 @@
return chrome.runBeforeUnloadConfirmPanel(text, m_frame);
}
-void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& request, FormState* formState, ShouldContinue shouldContinue, AllowNavigationToInvalidURL allowNavigationToInvalidURL)
+void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& request, FormState* formState, NavigationPolicyDecision navigationPolicyDecision, 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
@@ -3345,7 +3345,7 @@
bool isTargetItem = history().provisionalItem() ? history().provisionalItem()->isTargetItem() : false;
bool urlIsDisallowed = allowNavigationToInvalidURL == AllowNavigationToInvalidURL::No && !request.url().isValid();
- bool canContinue = shouldContinue != ShouldContinue::No && shouldClose() && !urlIsDisallowed;
+ bool canContinue = navigationPolicyDecision == NavigationPolicyDecision::ContinueLoad && shouldClose() && !urlIsDisallowed;
if (!canContinue) {
RELEASE_LOG_IF_ALLOWED("continueLoadAfterNavigationPolicy: can't continue loading frame due to the following reasons ("
@@ -3353,12 +3353,12 @@
"main = %d, "
"allowNavigationToInvalidURL = %d, "
"requestURLIsValid = %d, "
- "shouldContinue = %d)",
+ "navigationPolicyDecision = %d)",
&m_frame,
m_frame.isMainFrame(),
static_cast<int>(allowNavigationToInvalidURL),
request.url().isValid(),
- static_cast<int>(shouldContinue));
+ static_cast<int>(navigationPolicyDecision));
// If we were waiting for a quick redirect, but the policy delegate decided to ignore it, then we
// need to report that the client redirect was cancelled.
@@ -3366,10 +3366,17 @@
if (m_quickRedirectComing)
clientRedirectCancelledOrFinished(NewLoadInProgress::No);
+ if (navigationPolicyDecision == NavigationPolicyDecision::StopAllLoads) {
+ stopAllLoaders();
+ m_checkTimer.stop();
+ }
+
setPolicyDocumentLoader(nullptr);
checkCompleted();
- checkLoadComplete();
+ if (navigationPolicyDecision != NavigationPolicyDecision::StopAllLoads)
+ checkLoadComplete();
+
// If the navigation request came from the back/forward menu, and we punt on it, we have the
// problem that we have optimistically moved the b/f cursor already, so move it back. For sanity,
// we only do this when punting a navigation for the target frame or top-level frame.
Modified: branches/safari-607-branch/Source/WebCore/loader/FrameLoader.h (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/FrameLoader.h 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/FrameLoader.h 2019-02-06 22:17:28 UTC (rev 241080)
@@ -84,6 +84,7 @@
enum class NewLoadInProgress : bool;
enum class ShouldContinue;
+enum class NavigationPolicyDecision : uint8_t;
enum class ShouldTreatAsContinuingLoad : bool;
struct WindowFeatures;
@@ -347,7 +348,7 @@
bool dispatchBeforeUnloadEvent(Chrome&, FrameLoader* frameLoaderBeingNavigated);
void dispatchUnloadEvents(UnloadEventPolicy);
- void continueLoadAfterNavigationPolicy(const ResourceRequest&, FormState*, ShouldContinue, AllowNavigationToInvalidURL);
+ void continueLoadAfterNavigationPolicy(const ResourceRequest&, FormState*, NavigationPolicyDecision, AllowNavigationToInvalidURL);
void continueLoadAfterNewWindowPolicy(const ResourceRequest&, FormState*, const String& frameName, const NavigationAction&, ShouldContinue, AllowNavigationToInvalidURL, NewFrameOpenerPolicy);
void continueFragmentScrollAfterNavigationPolicy(const ResourceRequest&, bool shouldContinue);
Modified: branches/safari-607-branch/Source/WebCore/loader/FrameLoaderTypes.h (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/FrameLoaderTypes.h 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/FrameLoaderTypes.h 2019-02-06 22:17:28 UTC (rev 241080)
@@ -44,6 +44,7 @@
Use,
Download,
Ignore,
+ StopAllLoads
};
enum class ReloadOption : uint8_t {
@@ -152,7 +153,8 @@
WebCore::PolicyAction,
WebCore::PolicyAction::Use,
WebCore::PolicyAction::Download,
- WebCore::PolicyAction::Ignore
+ WebCore::PolicyAction::Ignore,
+ WebCore::PolicyAction::StopAllLoads
>;
};
Modified: branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.cpp (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.cpp 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.cpp 2019-02-06 22:17:28 UTC (rev 241080)
@@ -113,7 +113,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), { }, ShouldContinue::Yes);
+ function(ResourceRequest(request), { }, NavigationPolicyDecision::ContinueLoad);
loader->setLastCheckedRequest(WTFMove(request));
return;
}
@@ -128,7 +128,7 @@
#endif
if (isBackForwardLoadType(m_loadType))
m_loadType = FrameLoadType::Reload;
- function(WTFMove(request), { }, shouldContinue ? ShouldContinue::Yes : ShouldContinue::No);
+ function(WTFMove(request), { }, shouldContinue ? NavigationPolicyDecision::ContinueLoad : NavigationPolicyDecision::IgnoreLoad);
return;
}
@@ -138,7 +138,7 @@
// 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, Event::CanBubble::No, Event::IsCancelable::No));
}
- function(WTFMove(request), { }, ShouldContinue::No);
+ function(WTFMove(request), { }, NavigationPolicyDecision::IgnoreLoad);
return;
}
@@ -151,7 +151,7 @@
#if USE(QUICK_LOOK)
// Always allow QuickLook-generated URLs based on the protocol scheme.
if (!request.isNull() && isQuickLookPreviewURL(request.url()))
- return function(WTFMove(request), makeWeakPtr(formState.get()), ShouldContinue::Yes);
+ return function(WTFMove(request), makeWeakPtr(formState.get()), NavigationPolicyDecision::ContinueLoad);
#endif
#if ENABLE(CONTENT_FILTERING)
@@ -161,7 +161,7 @@
if (unblocked)
frame->loader().reload();
});
- return function({ }, nullptr, ShouldContinue::No);
+ return function({ }, nullptr, NavigationPolicyDecision::IgnoreLoad);
}
m_contentFilterUnblockHandler = { };
#endif
@@ -181,13 +181,16 @@
m_frame.loader().client().startDownload(request, suggestedFilename);
FALLTHROUGH;
case PolicyAction::Ignore:
- return function({ }, nullptr, ShouldContinue::No);
+ return function({ }, nullptr, NavigationPolicyDecision::IgnoreLoad);
+ case PolicyAction::StopAllLoads:
+ function({ }, nullptr, NavigationPolicyDecision::StopAllLoads);
+ return;
case PolicyAction::Use:
if (!m_frame.loader().client().canHandleRequest(request)) {
handleUnimplementablePolicy(m_frame.loader().client().cannotShowURLError(request));
- return function({ }, { }, ShouldContinue::No);
+ return function({ }, { }, NavigationPolicyDecision::IgnoreLoad);
}
- return function(WTFMove(request), makeWeakPtr(formState.get()), ShouldContinue::Yes);
+ return function(WTFMove(request), makeWeakPtr(formState.get()), NavigationPolicyDecision::ContinueLoad);
}
ASSERT_NOT_REACHED();
});
@@ -211,6 +214,10 @@
case PolicyAction::Ignore:
function({ }, nullptr, { }, { }, ShouldContinue::No);
return;
+ case PolicyAction::StopAllLoads:
+ ASSERT_NOT_REACHED();
+ function({ }, nullptr, { }, { }, ShouldContinue::No);
+ return;
case PolicyAction::Use:
function(request, makeWeakPtr(formState.get()), frameName, navigationAction, ShouldContinue::Yes);
return;
Modified: branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.h (241079 => 241080)
--- branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.h 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebCore/loader/PolicyChecker.h 2019-02-06 22:17:28 UTC (rev 241080)
@@ -57,10 +57,16 @@
No
};
+enum class NavigationPolicyDecision : uint8_t {
+ ContinueLoad,
+ IgnoreLoad,
+ StopAllLoads,
+};
+
enum class PolicyDecisionMode { Synchronous, Asynchronous };
using NewWindowPolicyDecisionFunction = CompletionHandler<void(const ResourceRequest&, WeakPtr<FormState>&&, const String& frameName, const NavigationAction&, ShouldContinue)>;
-using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, WeakPtr<FormState>&&, ShouldContinue)>;
+using NavigationPolicyDecisionFunction = CompletionHandler<void(ResourceRequest&&, WeakPtr<FormState>&&, NavigationPolicyDecision)>;
class PolicyChecker {
WTF_MAKE_NONCOPYABLE(PolicyChecker);
Modified: branches/safari-607-branch/Source/WebKit/ChangeLog (241079 => 241080)
--- branches/safari-607-branch/Source/WebKit/ChangeLog 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebKit/ChangeLog 2019-02-06 22:17:28 UTC (rev 241080)
@@ -1,5 +1,79 @@
2019-02-05 Alan Coon <[email protected]>
+ Cherry-pick r240880. rdar://problem/47774545
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ Source/WebCore:
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ (WebCore::DocumentLoader::continueAfterContentPolicy):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/FrameLoader.h:
+ * loader/FrameLoaderTypes.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
+ Source/WebKit:
+
+ The issue was happening when the page is triggering a cross-site navigation while in the middle of parsing. This would cause us to
+ start a new provisional load in a new process before the previous process sends the DidFinishLoadForFrame() IPC to the UIProcess.
+ Getting such IPC after a provisional load has started would mess up our state machine and trip assertions.
+
+ This patch restores non-PSON behavior which is that the previous load in the old process now gets stopped so that no DidFinishLoadForFrame()
+ / DidFailLoadForFrame() gets sent. To achieve this behavior, I introduced a new "StopAllLoads" PolicyAction that we now send the old
+ process when the load is continuing in a new process, instead of sending it "Ignore".
+
+ * NetworkProcess/NetworkDataTaskBlob.cpp:
+ (WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse):
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (toNSURLSessionResponseDisposition):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240880 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-01 Chris Dumez <[email protected]>
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ The issue was happening when the page is triggering a cross-site navigation while in the middle of parsing. This would cause us to
+ start a new provisional load in a new process before the previous process sends the DidFinishLoadForFrame() IPC to the UIProcess.
+ Getting such IPC after a provisional load has started would mess up our state machine and trip assertions.
+
+ This patch restores non-PSON behavior which is that the previous load in the old process now gets stopped so that no DidFinishLoadForFrame()
+ / DidFailLoadForFrame() gets sent. To achieve this behavior, I introduced a new "StopAllLoads" PolicyAction that we now send the old
+ process when the load is continuing in a new process, instead of sending it "Ignore".
+
+ * NetworkProcess/NetworkDataTaskBlob.cpp:
+ (WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse):
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (toNSURLSessionResponseDisposition):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+
+2019-02-05 Alan Coon <[email protected]>
+
Cherry-pick r240818. rdar://problem/47774549
Page zoom level is lost after a process swap or a crash
Modified: branches/safari-607-branch/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp (241079 => 241080)
--- branches/safari-607-branch/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebKit/NetworkProcess/NetworkDataTaskBlob.cpp 2019-02-06 22:17:28 UTC (rev 241080)
@@ -311,6 +311,9 @@
m_buffer.resize(bufferSize);
read();
break;
+ case PolicyAction::StopAllLoads:
+ ASSERT_NOT_REACHED();
+ break;
case PolicyAction::Ignore:
break;
case PolicyAction::Download:
Modified: branches/safari-607-branch/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm (241079 => 241080)
--- branches/safari-607-branch/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm 2019-02-06 22:17:28 UTC (rev 241080)
@@ -61,6 +61,11 @@
static NSURLSessionResponseDisposition toNSURLSessionResponseDisposition(WebCore::PolicyAction disposition)
{
switch (disposition) {
+ case WebCore::PolicyAction::StopAllLoads:
+ ASSERT_NOT_REACHED();
+#if ASSERT_DISABLED
+ FALLTHROUGH;
+#endif
case WebCore::PolicyAction::Ignore:
return NSURLSessionResponseCancel;
case WebCore::PolicyAction::Use:
Modified: branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (241079 => 241080)
--- branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-06 22:17:28 UTC (rev 241080)
@@ -2709,7 +2709,7 @@
}
if (processForNavigation.ptr() != &process()) {
- policyAction = PolicyAction::Ignore;
+ policyAction = PolicyAction::StopAllLoads;
RELEASE_LOG_IF_ALLOWED(ProcessSwapping, "decidePolicyForNavigationAction, swapping process %i with process %i for navigation, reason: %{public}s", processIdentifier(), processForNavigation->processIdentifier(), reason.utf8().data());
LOG(ProcessSwapping, "(ProcessSwapping) Switching from process %i to new process (%i) for navigation %" PRIu64 " '%s'", processIdentifier(), processForNavigation->processIdentifier(), navigation->navigationID(), navigation->loggingString());
} else
Modified: branches/safari-607-branch/Tools/ChangeLog (241079 => 241080)
--- branches/safari-607-branch/Tools/ChangeLog 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Tools/ChangeLog 2019-02-06 22:17:28 UTC (rev 241080)
@@ -1,5 +1,68 @@
2019-02-05 Alan Coon <[email protected]>
+ Cherry-pick r240880. rdar://problem/47774545
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ Source/WebCore:
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::willSendRequest):
+ (WebCore::DocumentLoader::continueAfterContentPolicy):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadURL):
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+ (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+ * loader/FrameLoader.h:
+ * loader/FrameLoaderTypes.h:
+ * loader/PolicyChecker.cpp:
+ (WebCore::PolicyChecker::checkNavigationPolicy):
+ (WebCore::PolicyChecker::checkNewWindowPolicy):
+ * loader/PolicyChecker.h:
+
+ Source/WebKit:
+
+ The issue was happening when the page is triggering a cross-site navigation while in the middle of parsing. This would cause us to
+ start a new provisional load in a new process before the previous process sends the DidFinishLoadForFrame() IPC to the UIProcess.
+ Getting such IPC after a provisional load has started would mess up our state machine and trip assertions.
+
+ This patch restores non-PSON behavior which is that the previous load in the old process now gets stopped so that no DidFinishLoadForFrame()
+ / DidFailLoadForFrame() gets sent. To achieve this behavior, I introduced a new "StopAllLoads" PolicyAction that we now send the old
+ process when the load is continuing in a new process, instead of sending it "Ignore".
+
+ * NetworkProcess/NetworkDataTaskBlob.cpp:
+ (WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse):
+ * NetworkProcess/cocoa/NetworkSessionCocoa.mm:
+ (toNSURLSessionResponseDisposition):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::receivedNavigationPolicyDecision):
+
+ Tools:
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240880 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-02-01 Chris Dumez <[email protected]>
+
+ REGRESSION: Flaky ASSERTION FAILED: m_uncommittedState.state == State::Committed on http/tests/cookies/same-site/fetch-after-top-level-navigation-initiated-from-iframe-in-cross-origin-page.html
+ https://bugs.webkit.org/show_bug.cgi?id=193740
+ <rdar://problem/47527267>
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
+2019-02-05 Alan Coon <[email protected]>
+
Cherry-pick r240856. rdar://problem/47774509
API Test broken: TestWebKitAPI.WebKit2.GetUserMediaReprompt
Modified: branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (241079 => 241080)
--- branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-06 22:17:23 UTC (rev 241079)
+++ branches/safari-607-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm 2019-02-06 22:17:28 UTC (rev 241080)
@@ -2575,6 +2575,53 @@
#endif // PLATFORM(MAC)
+static const char* navigateBeforePageLoadEndBytes = R"PSONRESOURCE(
+<body>
+<a id="testLink" href=""
+<script>
+ testLink.click();
+</script>
+<p>TEST</p>
+<script src=""
+<script src=""
+<iframe src=""
+<iframe src=""
+<iframe src=""
+<iframe src=""
+</body>
+)PSONRESOURCE";
+
+TEST(ProcessSwap, NavigateCrossSiteBeforePageLoadEnd)
+{
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ [processPoolConfiguration setProcessSwapsOnNavigation:YES];
+ [processPoolConfiguration setPrewarmsProcessesAutomatically: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/main.html" toData:navigateBeforePageLoadEndBytes];
+ [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()];
+
+ [webView configuration].preferences.safeBrowsingEnabled = NO;
+
+ failed = false;
+ NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+ [webView loadRequest:request];
+
+ TestWebKitAPI::Util::run(&done);
+ done = false;
+
+ EXPECT_FALSE(failed);
+ EXPECT_WK_STREQ(@"pson://www.apple.com/main.html", [[webView URL] absoluteString]);
+}
+
+
TEST(ProcessSwap, DoSameSiteNavigationAfterCrossSiteProvisionalLoadStarted)
{
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
