Move most of FrameLoader::CheckCompleted() to Document

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 a6a1eb6302451d96462d21deb76bd8e46bb7e89f..5ee0499f8283f6181fe87c379e79144ea3ad60f1 100644
--- a/third_party/WebKit/Source/core/loader/FrameLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/FrameLoader.cpp
@@ -396,7 +396,7 @@ void FrameLoader::FinishedParsing() {
         document_loader_ ? document_loader_->IsCommittedButEmpty() : true);
   }
 
-  CheckCompleted();
+  frame_->GetDocument()->CheckCompleted();

[Nate Chapin, 2017/04/11 23:25:50]

FrameLoader::FinishedParsing is only called from Document::FinishedParsing, so
ideally this clause would be in Document. However, it does need to run before
ProcessFragment, which is FrameLoader-private and would ideally stay that way.

 
   if (!frame_->View())
     return;
@@ -408,15 +408,6 @@ void FrameLoader::FinishedParsing() {
                   kNavigationToDifferentDocument);
 }
 
-static bool AllDescendantsAreComplete(Frame* frame) {
-  for (Frame* child = frame->Tree().FirstChild(); child;
-       child = child->Tree().TraverseNext(frame)) {
-    if (child->IsLoading())
-      return false;
-  }
-  return true;
-}
-
 bool FrameLoader::AllAncestorsAreComplete() const {
   for (Frame* ancestor = frame_; ancestor;
        ancestor = ancestor->Tree().Parent()) {
@@ -426,96 +417,15 @@ bool FrameLoader::AllAncestorsAreComplete() const {
   return true;
 }
 
-static bool ShouldComplete(Document* document) {
-  if (!document->GetFrame())
-    return false;
-  if (document->Parsing() || document->IsInDOMContentLoaded())
-    return false;
-  if (!document->HaveImportsLoaded())
-    return false;
-  if (document->Fetcher()->BlockingRequestCount())
-    return false;
-  if (document->IsDelayingLoadEvent())
-    return false;
-  return AllDescendantsAreComplete(document->GetFrame());
-}
-
-static bool ShouldSendFinishNotification(LocalFrame* frame) {
-  // Don't send didFinishLoad more than once per DocumentLoader.
-  if (frame->Loader().GetDocumentLoader()->SentDidFinishLoad())
-    return false;
-
-  // We might have declined to run the load event due to an imminent
-  // content-initiated navigation.
-  if (!frame->GetDocument()->LoadEventFinished())
-    return false;
-
-  // An event might have restarted a child frame.
-  if (!AllDescendantsAreComplete(frame))
-    return false;
-
-  // Don't notify if the frame is being detached.
-  if (!frame->IsAttached())
-    return false;
-
-  return true;
-}
-
-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().GetDocumentLoader()->SentDidFinishLoad() &&
-         !frame->Loader().HasProvisionalNavigation();
-}
-
-void FrameLoader::CheckCompleted() {
-  if (!ShouldComplete(frame_->GetDocument()))
-    return;
-
-  if (Client()) {
-    Client()->RunScriptsAtDocumentIdle();
-
-    // Injected scripts may have disconnected this frame.
-    if (!Client())
-      return;
-
-    // Check again, because runScriptsAtDocumentIdle() may have delayed the load
-    // event.
-    if (!ShouldComplete(frame_->GetDocument()))
-      return;
-  }
-
-  // OK, completed.
-  frame_->GetDocument()->SetReadyState(Document::kComplete);
-  if (frame_->GetDocument()->LoadEventStillNeeded())
-    frame_->GetDocument()->ImplicitClose();
-
-  frame_->GetNavigationScheduler().StartTimer();
-
-  if (frame_->View())
-    frame_->View()->HandleLoadCompleted();
-
-  // The readystatechanged or load event may have disconnected this frame.
-  if (!frame_->Client())
+void FrameLoader::DidFinishNavigation() {
+  // We should have either finished the provisional or committed navigation if
+  // this is called. Only delcare the whole frame finished if neither is in
+  // progress.
+  DCHECK(document_loader_->SentDidFinishLoad() || !HasProvisionalNavigation());
+  if (!document_loader_->SentDidFinishLoad() || HasProvisionalNavigation())

[Nate Chapin, 2017/04/11 23:25:50]

So now:
* Failing a navigation, whether before or after it commits, happens via
DocumentLoader::LoadFailed()
* A successful navigation always goes through Document::CheckCompleted()

Those are the only two callers of DidFinishNavigation. This function just checks
whether both provisional and commiteed are done, and fires "I'm completely done"
messages as needed.

     return;
 
-  if (ShouldSendFinishNotification(frame_)) {
-    // Report mobile vs. desktop page statistics. This will only report on
-    // Android.
-    if (frame_->IsMainFrame())
-      frame_->GetDocument()->GetViewportDescription().ReportMobilePageStats(
-          frame_);
-    document_loader_->SetSentDidFinishLoad();
-    Client()->DispatchDidFinishLoad();
-    // Finishing the load can detach the frame when running layout tests.
-    if (!frame_->Client())
-      return;
-  }
-
-  if (ShouldSendCompleteNotification(frame_)) {
+  if (frame_->IsLoading()) {

[Nate Chapin, 2017/04/11 23:25:50]

It's a shame we need to handle the case where DidFinishNavigation() is called
when the Frame is already marked as not loading, but enough tests broke when I
tried to DCHECK(frame_->IsLoading()) that I figure that's a potential followup.

     progress_tracker_->ProgressCompleted();
     // Retry restoring scroll offset since finishing loading disables content
     // size clamping.
@@ -527,7 +437,7 @@ void FrameLoader::CheckCompleted() {
 
   Frame* parent = frame_->Tree().Parent();
   if (parent && parent->IsLocalFrame())
-    ToLocalFrame(parent)->Loader().CheckCompleted();
+    ToLocalFrame(parent)->GetDocument()->CheckCompleted();
 }
 
 void FrameLoader::CheckTimerFired(TimerBase*) {
@@ -535,7 +445,7 @@ void FrameLoader::CheckTimerFired(TimerBase*) {
     if (page->Suspended())
       return;
   }
-  CheckCompleted();
+  frame_->GetDocument()->CheckCompleted();
 }
 
 void FrameLoader::ScheduleCheckCompleted() {
@@ -643,7 +553,7 @@ void FrameLoader::LoadInSameDocument(
 
   document_loader_->GetInitialScrollState().was_scrolled_by_user = false;
 
-  CheckCompleted();
+  frame_->GetDocument()->CheckCompleted();
 
   frame_->DomWindow()->StatePopped(state_object
                                        ? std::move(state_object)