Move most of FrameLoader::CheckCompleted() to Document

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

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 412 matching lines @@
                                                          history_commit_type);
     if (!frame_)
       return;
     loader.DetachProvisionalDocumentLoader(this);
   } else if (state_ == kCommitted) {
     if (frame_->GetDocument()->Parser())
       frame_->GetDocument()->Parser()->StopParsing();
     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) {
   DCHECK(frame_->Loader().StateMachine()->CreatingInitialEmptyDocument() ||
          !frame_->GetPage()->Suspended() ||
          MainThreadDebugger::Instance()->IsPaused());
 
   double response_end_time = finish_time;
   if (!response_end_time)
     response_end_time = time_of_last_data_received_;
@@ skipping 670 matching lines @@
                      InstallNewDocumentReason::kJavascriptURL,
                      kForceSynchronousParsing, KURL());
   if (!source.IsNull())
     writer_->AppendReplacingData(source);
   EndWriting();
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 }  // namespace blink