Always send a fail or finish notification for each navigation.

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

Index: third_party/WebKit/Source/core/loader/FrameLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/FrameLoader.cpp b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
index 4cbd51bfa9ecb2615c9f93bd6a81801b4e308834..7f93747776577d9cf55e0cfc3eaa669095408cd2 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -226,6 +226,10 @@ void FrameLoader::init() {
   m_frame->document()->cancelParsing();
   m_stateMachine.advanceTo(
       FrameLoaderStateMachine::DisplayingInitialEmptyDocument);
+  // Suppress finish notifications for inital empty documents, since they don't
+  // generate start notifications.

[dcheng, 2016/12/14 23:01:28]

Unrelated, but do you think it'd be problematic to send start / finish for
initial empty documents?

[Nate Chapin, 2016/12/14 23:59:18]

Extensions might inject content scripts into initial empty documents? I haven't
tested to see what if anything would break, it's just been the convention to
keep the initial empty document as a mostly-invisible blink implementation
detail.

+  if (m_documentLoader)
+    m_documentLoader->setSentDidFinishLoad();
   // Self-suspend if created in an already suspended Page. Note that both
   // startLoadingMainResource() and cancelParsing() may have already detached
   // the frame, since they both fire JS events.
@@ -673,11 +677,6 @@ static bool shouldComplete(Document* document) {
 }
 
 static bool shouldSendFinishNotification(LocalFrame* frame) {
-  // Don't send stop notifications for inital empty documents, since they don't
-  // generate start notifications.
-  if (!frame->loader().stateMachine()->committedFirstRealDocumentLoad())

[Nate Chapin, 2016/12/12 21:40:38]

setSentDidFinishLoad() in init() instead.

-    return false;
-
   // Don't send didFinishLoad more than once per DocumentLoader.
   if (frame->loader().documentLoader()->sentDidFinishLoad())
     return false;
@@ -1469,19 +1468,18 @@ void FrameLoader::loadFailed(DocumentLoader* loader,
 
   HistoryCommitType historyCommitType = loadTypeToCommitType(m_loadType);
   if (loader == m_provisionalDocumentLoader) {
+    m_provisionalDocumentLoader->setSentDidFinishLoad();
     client()->dispatchDidFailProvisionalLoad(error, historyCommitType);
     if (loader != m_provisionalDocumentLoader)
       return;
     detachDocumentLoader(m_provisionalDocumentLoader);
-    m_progressTracker->progressCompleted();

[Nate Chapin, 2016/12/12 21:40:38]

progressCompleted() should be called if appropriate in the checkCompleted() call
below, so long as setSentDidFinishLoad() is called.

[dcheng, 2016/12/14 23:01:28]

Seems subtle, but I agree this should work.

   } else {
     DCHECK_EQ(loader, m_documentLoader);
     if (m_frame->document()->parser())
       m_frame->document()->parser()->stopParsing();
-    m_documentLoader->setSentDidFinishLoad();
-    if (!m_provisionalDocumentLoader && m_frame->isLoading()) {
+    if (!m_documentLoader->sentDidFinishLoad()) {
+      m_documentLoader->setSentDidFinishLoad();
       client()->dispatchDidFailLoad(error, historyCommitType);
-      m_progressTracker->progressCompleted();
     }
   }
   checkCompleted();