Cleanup blink-side PlzNavigate logic

third_party/WebKit/Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
@@ skipping 171 matching lines @@
 
 FrameLoader::FrameLoader(LocalFrame* frame)
     : m_frame(frame),
       m_progressTracker(ProgressTracker::create(frame)),
       m_inStopAllLoaders(false),
       m_checkTimer(TaskRunnerHelper::get(TaskType::Networking, frame),
                    this,
                    &FrameLoader::checkTimerFired),
       m_forcedSandboxFlags(SandboxNone),
       m_dispatchingDidClearWindowObjectInMainWorld(false),
-      m_protectProvisionalLoader(false),
-      m_isNavigationHandledByClient(false) {
+      m_protectProvisionalLoader(false) {
   DCHECK(m_frame);
   TRACE_EVENT_OBJECT_CREATED_WITH_ID("loading", "FrameLoader", this);
   takeObjectSnapshot();
 }
 
 FrameLoader::~FrameLoader() {
   // Verify that this FrameLoader has been detached.
   DCHECK(!m_progressTracker);
 }
 
@@ skipping 492 matching lines @@
   if (!allDescendantsAreComplete(frame))
     return false;
 
   // Don't notify if the frame is being detached.
   if (frame->isDetaching())
     return false;
 
   return true;
 }
 
-static bool shouldSendCompleteNotification(LocalFrame* frame,
-                                           bool isNavigationHandledByClient) {
+static bool shouldSendCompleteNotification(LocalFrame* frame) {
   // FIXME: We might have already sent stop notifications and be re-completing.
   if (!frame->isLoading())
     return false;
   // Only send didStopLoading() if there are no navigations in progress at all,
   // whether committed, provisional, or pending.
   return frame->loader().documentLoader()->sentDidFinishLoad() &&
          !frame->loader().hasProvisionalNavigation();
 }
 
 void FrameLoader::checkCompleted() {
@@ skipping 19 matching lines @@
     // Android.
     if (m_frame->isMainFrame())
       m_frame->document()->viewportDescription().reportMobilePageStats(m_frame);
     m_documentLoader->setSentDidFinishLoad();
     client()->dispatchDidFinishLoad();
     // Finishing the load can detach the frame when running layout tests.
     if (!m_frame->client())
       return;
   }
 
-  if (shouldSendCompleteNotification(m_frame, m_isNavigationHandledByClient)) {
+  if (shouldSendCompleteNotification(m_frame)) {
     m_progressTracker->progressCompleted();
     // Retry restoring scroll offset since finishing loading disables content
     // size clamping.
     restoreScrollPositionAndViewState();
     if (m_documentLoader)
       m_documentLoader->setLoadType(FrameLoadTypeStandard);
     m_frame->domWindow()->finishedLoading();
   }
 
   Frame* parent = m_frame->tree().parent();
@@ skipping 476 matching lines @@
       Document::NoDismissal)
     return;
 
   // If this method is called from within this method, infinite recursion can
   // occur (3442218). Avoid this.
   if (m_inStopAllLoaders)
     return;
 
   m_inStopAllLoaders = true;
 
-  if (m_isNavigationHandledByClient) {

[Nate Chapin, 2017/02/24 23:42:06]

Because we know have a dummy DocumentLoader for the client-handled navigation,
simply detaching m_provisionalDoucmentLoader will accomplish this.

[clamy, 2017/03/02 14:37:18]

Acknowledged.

-    client()->dispatchDidFailProvisionalLoad(
-        ResourceError::cancelledError(String()), StandardCommit);
-  }
-
-  clearNavigationHandledByClient();
-
   for (Frame* child = m_frame->tree().firstChild(); child;
        child = child->tree().nextSibling()) {
     if (child->isLocalFrame())
       toLocalFrame(child)->loader().stopAllLoaders();
   }
 
   m_frame->document()->suppressLoadEvent();
   if (m_documentLoader)
     m_documentLoader->fetcher()->stopFetching();
   m_frame->document()->cancelParsing();
@@ skipping 111 matching lines @@
     m_frame->page()->chromeClient().setEventListenerProperties(
         m_frame, WebEventListenerClass::MouseWheel,
         WebEventListenerProperties::Nothing);
     m_frame->page()->chromeClient().setEventListenerProperties(
         m_frame, WebEventListenerClass::TouchEndOrCancel,
         WebEventListenerProperties::Nothing);
   }
 
   client()->transitionToCommittedForNewPage();
 
-  // PlzNavigate: We need to ensure that script initiated navigations are
-  // honored.
-  if (!m_isNavigationHandledByClient)

[Nate Chapin, 2017/02/24 23:42:06]

I'm not sure this was right. If we committed a load, any and all navigations,
whether scheduled or actually started, should be preempted.

-    m_frame->navigationScheduler().cancel();
+  m_frame->navigationScheduler().cancel();
 
   m_frame->editor().clearLastEditCommand();
 
   // If we are still in the process of initializing an empty document then its
   // frame is not in a consistent state for rendering, so avoid
   // setJSStatusBarText since it may cause clients to attempt to render the
   // frame.
   if (!m_stateMachine.creatingInitialEmptyDocument()) {
     LocalDOMWindow* window = m_frame->domWindow();
     window->setStatus(String());
@@ skipping 108 matching lines @@
                              const ResourceError& error) {
   if (!error.isCancellation() && m_frame->owner()) {
     // FIXME: For now, fallback content doesn't work cross process.
     if (m_frame->owner()->isLocal())
       m_frame->deprecatedLocalOwner()->renderFallbackContent();
   }
 
   HistoryCommitType historyCommitType =
       loadTypeToCommitType(loader->loadType());
   if (loader == m_provisionalDocumentLoader) {
+    if (!m_provisionalDocumentLoader->didStart())
+      InspectorInstrumentation::frameClearedScheduledClientNavigation(m_frame);
     m_provisionalDocumentLoader->setSentDidFinishLoad();
     client()->dispatchDidFailProvisionalLoad(error, historyCommitType);
     if (loader != m_provisionalDocumentLoader)
       return;
     detachDocumentLoader(m_provisionalDocumentLoader);
   } else {
     DCHECK_EQ(loader, m_documentLoader);
     if (m_frame->document()->parser())
       m_frame->document()->parser()->stopParsing();
     if (!m_documentLoader->sentDidFinishLoad()) {
@@ skipping 91 matching lines @@
         break;
     }
 
     if (i == targetFrames.size())
       shouldClose = true;
   }
 
   return shouldClose;
 }
 
-bool FrameLoader::shouldContinueForNavigationPolicy(
+NavigationPolicy FrameLoader::shouldContinueForNavigationPolicy(
     const ResourceRequest& request,
     const SubstituteData& substituteData,
     DocumentLoader* loader,
     ContentSecurityPolicyDisposition shouldCheckMainWorldContentSecurityPolicy,
     NavigationType type,
     NavigationPolicy policy,
     FrameLoadType frameLoadType,
     bool isClientRedirect,
     HTMLFormElement* form) {
   // Don't ask if we are loading an empty URL.
   if (request.url().isEmpty() || substituteData.isValid())
-    return true;
+    return NavigationPolicyCurrentTab;
 
   // If we're loading content into |m_frame| (NavigationPolicyCurrentTab), check
   // against the parent's Content Security Policy and kill the load if that
   // check fails, unless we should bypass the main world's CSP.
   if (policy == NavigationPolicyCurrentTab &&
       shouldCheckMainWorldContentSecurityPolicy == CheckContentSecurityPolicy) {
     Frame* parentFrame = m_frame->tree().parent();
     if (parentFrame) {
       ContentSecurityPolicy* parentPolicy =
           parentFrame->securityContext()->contentSecurityPolicy();
       if (!parentPolicy->allowFrameFromSource(request.url(),
                                               request.redirectStatus())) {
         // Fire a load event, as timing attacks would otherwise reveal that the
         // frame was blocked. This way, it looks like every other cross-origin
         // page load.
         m_frame->document()->enforceSandboxFlags(SandboxOrigin);
         m_frame->owner()->dispatchLoad();
-        return false;
+        return NavigationPolicyIgnore;
       }
     }
   }
 
   bool isFormSubmission = type == NavigationTypeFormSubmitted ||
                           type == NavigationTypeFormResubmitted;
   if (isFormSubmission &&
       !m_frame->document()->contentSecurityPolicy()->allowFormAction(
           request.url()))
-    return false;
+    return NavigationPolicyIgnore;
 
   bool replacesCurrentHistoryItem =
       frameLoadType == FrameLoadTypeReplaceCurrentItem;
   policy = client()->decidePolicyForNavigation(request, loader, type, policy,
                                                replacesCurrentHistoryItem,
                                                isClientRedirect, form);
-  if (policy == NavigationPolicyCurrentTab)
-    return true;
-  if (policy == NavigationPolicyIgnore)
-    return false;
-  if (policy == NavigationPolicyHandledByClient) {

[Nate Chapin, 2017/02/24 23:42:06]

All of this case should be handling in the main startLoad() path.

-    setNavigationHandledByClient();
-    // Mark the frame as loading since the embedder is handling the navigation.
-    m_progressTracker->progressStarted(frameLoadType);
+  if (policy == NavigationPolicyCurrentTab ||
+      policy == NavigationPolicyIgnore ||
+      policy == NavigationPolicyHandledByClient ||
+      policy == NavigationPolicyHandledByClientForInitialHistory) {
+    return policy;
+  }
 
-    m_frame->navigationScheduler().cancel();
-
-    // If this is a form submit, dispatch that a form is being submitted
-    // since the embedder is handling the navigation.
-    if (form)
-      client()->dispatchWillSubmitForm(form);
-
-    m_frame->document()->cancelParsing();
-
-    return false;
-  }
   if (!LocalDOMWindow::allowPopUp(*m_frame) &&
       !UserGestureIndicator::utilizeUserGesture())
-    return false;
+    return NavigationPolicyIgnore;
   client()->loadURLExternally(request, policy, String(),
                               replacesCurrentHistoryItem);
-  return false;
+  return NavigationPolicyIgnore;
 }
 
-bool FrameLoader::checkLoadCanStart(FrameLoadRequest& frameLoadRequest,
-                                    FrameLoadType type,
-                                    NavigationPolicy navigationPolicy,
-                                    NavigationType navigationType) {
+NavigationPolicy FrameLoader::checkLoadCanStart(
+    FrameLoadRequest& frameLoadRequest,
+    FrameLoadType type,
+    NavigationPolicy navigationPolicy,
+    NavigationType navigationType) {
   if (m_frame->document()->pageDismissalEventBeingDispatched() !=
       Document::NoDismissal) {
-    return false;
+    return NavigationPolicyIgnore;
   }
 
   // Record the latest requiredCSP value that will be used when sending this
   // request.
   ResourceRequest& resourceRequest = frameLoadRequest.resourceRequest();
   recordLatestRequiredCSP();
   modifyRequestForCSP(resourceRequest, nullptr);
 
-  if (!shouldContinueForNavigationPolicy(
-          resourceRequest, frameLoadRequest.substituteData(), nullptr,
-          frameLoadRequest.shouldCheckMainWorldContentSecurityPolicy(),
-          navigationType, navigationPolicy, type,
-          frameLoadRequest.clientRedirect() ==
-              ClientRedirectPolicy::ClientRedirect,
-          frameLoadRequest.form())) {
-    return false;
-  }
-
-  m_frame->document()->cancelParsing();

[Nate Chapin, 2017/02/24 23:42:06]

Here and below moved in to startLoad(), since they need some special handling if
a PlzNavigate load was in progress, and that way we can keep the
PlzNavigate-specific logic in fewer functions.

-  detachDocumentLoader(m_provisionalDocumentLoader);
-
-  // beforeunload fired above, and detaching a DocumentLoader can fire events,
-  // which can detach this frame.
-  if (!m_frame->host())
-    return false;
-
-  return true;
+  return shouldContinueForNavigationPolicy(
+      resourceRequest, frameLoadRequest.substituteData(), nullptr,
+      frameLoadRequest.shouldCheckMainWorldContentSecurityPolicy(),
+      navigationType, navigationPolicy, type,
+      frameLoadRequest.clientRedirect() == ClientRedirectPolicy::ClientRedirect,
+      frameLoadRequest.form());
 }
 
 void FrameLoader::startLoad(FrameLoadRequest& frameLoadRequest,
                             FrameLoadType type,
                             NavigationPolicy navigationPolicy) {
   DCHECK(client()->hasWebView());
   ResourceRequest& resourceRequest = frameLoadRequest.resourceRequest();
   NavigationType navigationType = determineNavigationType(
       type, resourceRequest.httpBody() || frameLoadRequest.form(),
       frameLoadRequest.triggeringEvent());
   resourceRequest.setRequestContext(
       determineRequestContextFromNavigationType(navigationType));
   resourceRequest.setFrameType(m_frame->isMainFrame()
                                    ? WebURLRequest::FrameTypeTopLevel
                                    : WebURLRequest::FrameTypeNested);
 
-  if (!checkLoadCanStart(frameLoadRequest, type, navigationPolicy,
-                         navigationType)) {
-    if (m_isNavigationHandledByClient) {
-      // PlzNavigate: if the navigation is a commit of a client-handled
-      // navigation, record that there is no longer a navigation handled by the
-      // client.
-      if (!frameLoadRequest.resourceRequest().checkForBrowserSideNavigation()) {
-        clearNavigationHandledByClient();
-      } else {
-        DocumentLoader* loader = createDocumentLoader(
-            resourceRequest, frameLoadRequest, type, navigationType);
-        // PlzNavigate: If the navigation is handled by the client, then the
-        // didFinishDocumentLoad() event occurs before the
-        // didStartProvisionalLoad() notification which occurs after the
-        // navigation
-        // is committed. This causes a number of layout tests to fail. We
-        // workaround this by invoking the didStartProvisionalLoad()
-        // notification
-        // here. Consumers of the didStartProvisionalLoad() notification rely on
-        // the provisional loader and save navigation state in it. We want to
-        // avoid
-        // this dependency on the provisional loader. For now we create a
-        // temporary
-        // loader and pass it to the didStartProvisionalLoad() function.
-        // TODO(ananta)
-        // We should get rid of the dependency on the DocumentLoader in
-        // consumers
-        // of
-        // the didStartProvisionalLoad() notification.
-        client()->dispatchDidStartProvisionalLoad(loader);

[Nate Chapin, 2017/02/24 23:42:06]

All of this is handled in the main path below, though we obviously leave the
DocumentLoader attached as m_provisionalDocumentLoader now.

-        DCHECK(loader);
-        loader->setSentDidFinishLoad();
-        loader->detachFromFrame();
-      }
+  bool hadPlaceholderClientDocumentLoader =

[Nate Chapin, 2017/02/24 23:42:05]

I don't like this name. Suggestions welcome if you have something better.

+      m_provisionalDocumentLoader && !m_provisionalDocumentLoader->didStart();
+  navigationPolicy = checkLoadCanStart(frameLoadRequest, type, navigationPolicy,
+                                       navigationType);
+  if (navigationPolicy == NavigationPolicyIgnore) {
+    if (hadPlaceholderClientDocumentLoader &&
+        !resourceRequest.checkForBrowserSideNavigation()) {
+      detachDocumentLoader(m_provisionalDocumentLoader);
     }
     return;
   }
 
+  // For PlzNavigate placeholer DocumentLoaders, don't send failure callbacks
+  // for a placeholder simply being replaced with a new DocumentLoader.
+  if (hadPlaceholderClientDocumentLoader)
+    m_provisionalDocumentLoader->setSentDidFinishLoad();
+  m_frame->document()->cancelParsing();
+  detachDocumentLoader(m_provisionalDocumentLoader);
+
+  // beforeunload fired above, and detaching a DocumentLoader can fire events,
+  // which can detach this frame.
+  if (!m_frame->host())
+    return;
+
+  m_progressTracker->progressStarted(type);
+  // TODO(japhet): This case wants to flag the frame as loading and do nothing
+  // else. It'd be nice if it could go through the placeholder DocumentLoader
+  // path, too.
+  if (navigationPolicy == NavigationPolicyHandledByClientForInitialHistory)
+    return;
+  DCHECK(navigationPolicy == NavigationPolicyCurrentTab ||
+         navigationPolicy == NavigationPolicyHandledByClient);
+
   m_provisionalDocumentLoader = createDocumentLoader(
       resourceRequest, frameLoadRequest, type, navigationType);
 
   // PlzNavigate: We need to ensure that script initiated navigations are
   // honored.
-  if (!m_isNavigationHandledByClient) {
+  if (!hadPlaceholderClientDocumentLoader ||
+      navigationPolicy == NavigationPolicyHandledByClient) {
     m_frame->navigationScheduler().cancel();
     m_checkTimer.stop();
   }
 
   if (frameLoadRequest.form())
     client()->dispatchWillSubmitForm(frameLoadRequest.form());
 
-  bool isNavigationHandledByClient = m_isNavigationHandledByClient;
-  // If the loader wasn't waiting for the client to handle a navigation, update
-  // the progress tracker. Otherwise don't, as it was already notified before
-  // sending the navigation to teh client.
-  if (!m_isNavigationHandledByClient)

[Nate Chapin, 2017/02/24 23:42:06]

Remove the need for this if() by teaching progressStarted to filter out repeated
calls.

-    m_progressTracker->progressStarted(type);
-  else
-    m_isNavigationHandledByClient = false;
-
   m_provisionalDocumentLoader->appendRedirect(
       m_provisionalDocumentLoader->getRequest().url());
-  // TODO(ananta)
+  // TODO(ananta):
   // We should get rid of the dependency on the DocumentLoader in consumers of
   // the didStartProvisionalLoad() notification.
   client()->dispatchDidStartProvisionalLoad(m_provisionalDocumentLoader);
   DCHECK(m_provisionalDocumentLoader);
 
-  m_provisionalDocumentLoader->startLoadingMainResource();
-
-  // This should happen after the request is sent, so we don't use
-  // clearNavigationHandledByClient() above.
-  if (isNavigationHandledByClient)
+  if (navigationPolicy == NavigationPolicyCurrentTab) {
+    m_provisionalDocumentLoader->startLoadingMainResource();
+    // This should happen after the request is sent, so that the state
+    // the inspector stored in the matching frameScheduledClientNavigation()
+    // is available while sending the request.
     InspectorInstrumentation::frameClearedScheduledClientNavigation(m_frame);
+  } else {
+    InspectorInstrumentation::frameScheduledClientNavigation(m_frame);
+  }
 
   takeObjectSnapshot();
 }
 
 void FrameLoader::applyUserAgent(ResourceRequest& request) {
   String userAgent = this->userAgent();
   DCHECK(!userAgent.isNull());
   request.setHTTPUserAgent(AtomicString(userAgent));
 }
 
@@ skipping 187 matching lines @@
                             : defaultSubstituteDataForURL(request.url()),
       frameLoadRequest.clientRedirect());
 
   loader->setLoadType(loadType);
   loader->setNavigationType(navigationType);
   loader->setReplacesCurrentHistoryItem(loadType ==
                                         FrameLoadTypeReplaceCurrentItem);
   return loader;
 }
 
-void FrameLoader::setNavigationHandledByClient() {
-  m_isNavigationHandledByClient = true;
-  InspectorInstrumentation::frameScheduledClientNavigation(m_frame);
-}
-
-void FrameLoader::clearNavigationHandledByClient() {
-  m_isNavigationHandledByClient = false;
-  InspectorInstrumentation::frameClearedScheduledClientNavigation(m_frame);
-}
-
 }  // namespace blink