Move most of FrameLoader::CheckCompleted() to Document

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

Index: third_party/WebKit/Source/core/loader/DocumentLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
index 70564978071f6b189b909478e2956d26c96ee5ff..4b165a134c12965e67ed3c5de8475e8c793279a2 100644
--- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
@@ -430,7 +430,8 @@ void DocumentLoader::LoadFailed(const ResourceError& error) {
     state_ = kSentDidFinishLoad;
     GetLocalFrameClient().DispatchDidFailLoad(error, history_commit_type);
   }
-  loader.CheckCompleted();
+  DCHECK_EQ(kSentDidFinishLoad, state_);
+  loader.DidFinishNavigation();

[yhirano, 2017/04/14 10:33:35]

Can you tell me why FrameLoader::DidFinishNavigation is called instead of
Document::CheckCompleted here?

[Nate Chapin, 2017/04/18 21:56:38]

Because all the tests pass? :D

For provisional navigations, we should just call DidFinishNavigation(), becuase
a navigation definitely finished, and that's independent from whether the
committed document is still loading.

For committed navigations, it's a bit trickier. If this is a cancellation (by
far the most common reason for LoadFailed() after commit), then we should be
more or less guaranteed that the document was made done by some other code path
(e.g., parsing completing), but it's not 100% certain. And if the load somehow
failed *during* parsing, after some subresource loads had started...yeah. There
are a lot of cases, and our test coverage problem isn't complete, so I should
err on the side of caution.

Changed to:
* Use a switch
* For the provisional case, have FrameLoder::DetachProvisionalDocumentLoader()
handle calling DidFinishNavigation()
* For the committed case, call checkCompleted(). This still messes with the
ordering of kSentDidFinishLoad getting set relative to onload, but that's a much
larger change.
* For the case where this is called and we're already in the
kSentDidFinishLoad...call DidFinishNavigation() again. I don't know why it's
needed, but tests fail without it. Added a TODO about that.

 }
 
 void DocumentLoader::FinishedLoading(double finish_time) {