Commit b487e622 authored by Yi Su's avatar Yi Su Committed by Commit Bot

Replace DCHECK with 'if' for mismatching NavigationContext

Currently we assume that if a NavigationContext can be found when
"decidePolicyForNavigationAction" is invoked, the navigation should be
either user-initiated or JS back-forward. However this can be wrong if
the web page uses JS to load the same URL during the navigation. The
root cause here is that we are matching URL to find the
NavigationContext for WKNavigationAction, which is not reliable.

Bug: 1000309,1003467
Change-Id: I7163474df5ffde9cf784ef348f48e652ca878df3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807832Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697511}
parent 0cba9023
......@@ -221,9 +221,21 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
if (isMainFrameNavigationAction) {
web::NavigationContextImpl* context =
[self contextForPendingMainFrameNavigationWithURL:requestURL];
if (context) {
DCHECK(!context->IsRendererInitiated() ||
(context->GetPageTransition() & ui::PAGE_TRANSITION_FORWARD_BACK));
// Theoretically if |context| can be found here, the navigation should be
// either user-initiated or JS back/forward. The second part in the "if"
// condition used to be a DCHECK, but it would fail in this case:
// 1. Multiple render-initiated navigation with the same URL happens at the
// same time;
// 2. One of these navigations gets the "didStartProvisonalNavigation"
// callback and creates a NavigationContext;
// 3. Another navigation reaches here and retrieves that NavigationContext
// by matching URL.
// The DCHECK is now turned into a "if" condition, but can be reverted if a
// more reliable way of matching NavigationContext with WKNavigationAction
// is found.
if (context &&
(!context->IsRendererInitiated() ||
(context->GetPageTransition() & ui::PAGE_TRANSITION_FORWARD_BACK))) {
transition = context->GetPageTransition();
if (context->IsLoadingErrorPage()) {
// loadHTMLString: navigation which loads error page into WKWebView.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment