PlzNavigate: Invoke didStartProvisionalLoad() when the renderer initiates a navigation in startLoad(

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 1481 matching lines @@
   }
 
   HistoryCommitType historyCommitType =
       loadTypeToCommitType(loader->loadType());
   if (loader == m_provisionalDocumentLoader) {
     m_provisionalDocumentLoader->setSentDidFinishLoad();
     client()->dispatchDidFailProvisionalLoad(error, historyCommitType);
     if (loader != m_provisionalDocumentLoader)
       return;
     detachDocumentLoader(m_provisionalDocumentLoader);
-  } else {
-    DCHECK_EQ(loader, m_documentLoader);

[Nate Chapin, 2017/02/02 19:16:48]

I assume this is because tempLoader->detachFromFrame() calls in to
FrameLoader::loadFailed()?

[ananta, 2017/02/02 23:54:02]

Reverted this change

+  } else if (loader == m_documentLoader) {
     if (m_frame->document()->parser())
       m_frame->document()->parser()->stopParsing();
     if (!m_documentLoader->sentDidFinishLoad()) {
       m_documentLoader->setSentDidFinishLoad();
       client()->dispatchDidFailLoad(error, historyCommitType);
     }
   }
   checkCompleted();
 }
 
@@ skipping 160 matching lines @@
       !UserGestureIndicator::utilizeUserGesture())
     return false;
   client()->loadURLExternally(request, policy, String(),
                               replacesCurrentHistoryItem);
   return false;
 }
 
 bool FrameLoader::checkLoadCanStart(FrameLoadRequest& frameLoadRequest,
                                     FrameLoadType type,
                                     NavigationPolicy navigationPolicy,
-                                    NavigationType navigationType) {
+                                    NavigationType navigationType,
+                                    DocumentLoader* loader) {
   if (m_frame->document()->pageDismissalEventBeingDispatched() !=
       Document::NoDismissal) {
     return false;
   }
 
   // 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,
+          resourceRequest, frameLoadRequest.substituteData(), loader,
           frameLoadRequest.shouldCheckMainWorldContentSecurityPolicy(),
           navigationType, navigationPolicy, type,
           frameLoadRequest.clientRedirect() ==
               ClientRedirectPolicy::ClientRedirect,
           frameLoadRequest.form())) {
     return false;
   }
 
   m_frame->document()->cancelParsing();
   detachDocumentLoader(m_provisionalDocumentLoader);
@@ skipping 13 matching lines @@
   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);
 
+  // In PlzNavigate, the didStartProvisionalLoad() notification happens after
+  // the navigation event, which causes a number of layout tests to fail. We
+  // can call didStartProvisionalLoad() early before a navigation attempt is
+  // sent to the browser. This should work correctly in the non PlzNavigate
+  // case as well. However consumers of didStartProvisionalLoad() rely on the
+  // provisionalLoader being created and save state in it. To workaround this
+  // we create a temporary DocumentLoader instance and pass it to the
+  // didStartProvisionalLoad() function. This is then promoted to be the
+  // provisional loader if navigation is handled in blink.
+  // TODO(ananta)
+  // We should get rid of the dependency on the DocumentLoader in consumers of
+  // the didStartProvisionalLoad() notification.
+  DocumentLoader* tempLoader(client()->createDocumentLoader(

[Nate Chapin, 2017/02/02 19:16:48]

I don't love the name tempLoader. newLoader? Just plain 'loader'? policyLoader
(i.e., what it was called in the WebKit era)?

If you think all of these are worse than tempLoader, feel free to leave it
as-is.

[ananta, 2017/02/02 23:54:02]

Moved this code to a helper function FrameLoader::createDocumentLoader()

+      m_frame, resourceRequest,
+      frameLoadRequest.substituteData().isValid()
+          ? frameLoadRequest.substituteData()
+          : defaultSubstituteDataForURL(resourceRequest.url()),
+      frameLoadRequest.clientRedirect()));
+  tempLoader->setLoadType(type);
+  tempLoader->setNavigationType(navigationType);
+  tempLoader->setReplacesCurrentHistoryItem(type ==
+                                            FrameLoadTypeReplaceCurrentItem);
+
+  client()->dispatchDidStartProvisionalLoad(tempLoader);
+  DCHECK(tempLoader);
+
   if (!checkLoadCanStart(frameLoadRequest, type, navigationPolicy,
-                         navigationType)) {
+                         navigationType, tempLoader)) {
     // 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 (m_isNavigationHandledByClient &&
         !frameLoadRequest.resourceRequest().checkForBrowserSideNavigation()) {
       m_isNavigationHandledByClient = false;
     }
+    tempLoader->setSentDidFinishLoad();
+    tempLoader->detachFromFrame();
     return;
   }
 
-  m_provisionalDocumentLoader = client()->createDocumentLoader(
-      m_frame, resourceRequest,
-      frameLoadRequest.substituteData().isValid()
-          ? frameLoadRequest.substituteData()
-          : defaultSubstituteDataForURL(resourceRequest.url()),
-      frameLoadRequest.clientRedirect());
-  m_provisionalDocumentLoader->setLoadType(type);
-  m_provisionalDocumentLoader->setNavigationType(navigationType);
-  m_provisionalDocumentLoader->setReplacesCurrentHistoryItem(
-      type == FrameLoadTypeReplaceCurrentItem);
+  m_provisionalDocumentLoader = tempLoader;
 
   // PlzNavigate: We need to ensure that script initiated navigations are
   // honored.
   if (!m_isNavigationHandledByClient) {
     m_frame->navigationScheduler().cancel();
     m_checkTimer.stop();
   }
 
   if (frameLoadRequest.form())
     client()->dispatchWillSubmitForm(frameLoadRequest.form());
 
   // 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)
     m_progressTracker->progressStarted(type);
   else
     m_isNavigationHandledByClient = false;
 
   m_provisionalDocumentLoader->appendRedirect(
       m_provisionalDocumentLoader->getRequest().url());
-  client()->dispatchDidStartProvisionalLoad();
-  DCHECK(m_provisionalDocumentLoader);
+
   m_provisionalDocumentLoader->startLoadingMainResource();
 
   takeObjectSnapshot();
 }
 
 void FrameLoader::applyUserAgent(ResourceRequest& request) {
   String userAgent = this->userAgent();
   DCHECK(!userAgent.isNull());
   request.setHTTPUserAgent(AtomicString(userAgent));
 }
@@ skipping 171 matching lines @@
                          m_documentLoader ? m_documentLoader->url() : String());
   return tracedValue;
 }
 
 inline void FrameLoader::takeObjectSnapshot() const {
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID("loading", "FrameLoader", this,
                                       toTracedValue());
 }
 
 }  // namespace blink