Remove kCommittedMultipleRealLoads from FrameLoaderStateMachine

Its purpose is to avoid creating spurious history entries in the case
where we replace the initial empty document with a different
about:blank document (which the spec says not to do, but I think every
major browser does). Explicitly track this case with a bit on
FrameLoader for whether we've ever loaded a non-empty document.

Change-Id: I2bd6d759ccf6f43d14feddeac1f72d2f871a80cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354995Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800149}

third_party/blink/renderer/core/frame/web_local_frame_impl.cc

@@ -2252,7 +2252,8 @@ bool WebLocalFrameImpl::IsNavigationScheduledWithin(
 void WebLocalFrameImpl::SetCommittedFirstRealLoad() {
   DCHECK(GetFrame());
   GetFrame()->Loader().StateMachine()->AdvanceTo(
-      FrameLoaderStateMachine::kCommittedMultipleRealLoads);
+      FrameLoaderStateMachine::kCommittedFirstRealLoad);
+  GetFrame()->Loader().SetDidLoadNonEmptyDocument();
   GetFrame()->SetShouldSendResourceTimingInfoToParent(false);
 }
 

third_party/blink/renderer/core/loader/document_loader.cc

@@ -1466,12 +1466,6 @@ void DocumentLoader::DidCommitNavigation() {
   if (commit_reason_ != CommitReason::kRegular)
     return;
 
-  if (!frame_->Loader().StateMachine()->CommittedMultipleRealLoads() &&
-      load_type_ == WebFrameLoadType::kStandard) {
-    frame_->Loader().StateMachine()->AdvanceTo(
-        FrameLoaderStateMachine::kCommittedMultipleRealLoads);
-  }
-
   WebHistoryCommitType commit_type = LoadTypeToCommitType(load_type_);
   frame_->GetFrameScheduler()->DidCommitProvisionalLoad(
       commit_type == kWebHistoryInertCommit,

third_party/blink/renderer/core/loader/frame_loader.cc

@@ -358,6 +358,7 @@ void FrameLoader::DidExplicitOpen() {
   // Calling document.open counts as committing the first real document load.
   if (!state_machine_.CommittedFirstRealDocumentLoad())
     state_machine_.AdvanceTo(FrameLoaderStateMachine::kCommittedFirstRealLoad);
+  has_loaded_non_empty_document_ = true;
 
   // Only model a document.open() as part of a navigation if its parent is not
   // done or in the process of completing.
@@ -509,8 +510,7 @@ WebFrameLoadType FrameLoader::DetermineFrameLoadType(
   // both in the renderer and in the browser.
   if (frame_load_type == WebFrameLoadType::kStandard ||
       frame_load_type == WebFrameLoadType::kReplaceCurrentItem) {
-    if (frame_->Tree().Parent() &&
-        !state_machine_.CommittedFirstRealDocumentLoad())
+    if (frame_->Tree().Parent() && !has_loaded_non_empty_document_)
       return WebFrameLoadType::kReplaceCurrentItem;
     if (!frame_->Tree().Parent() && !Client()->BackForwardLength()) {
       if (Opener() && url.IsEmpty())
@@ -520,13 +520,6 @@ WebFrameLoadType FrameLoader::DetermineFrameLoadType(
   }
   if (frame_load_type != WebFrameLoadType::kStandard)
     return frame_load_type;
-  // From the HTML5 spec for location.assign():
-  // "If the browsing context's session history contains only one Document,
-  // and that was the about:blank Document created when the browsing context
-  // was created, then the navigation must be done with replacement enabled."
-  if ((!state_machine_.CommittedMultipleRealLoads() &&
-       EqualIgnoringASCIICase(frame_->GetDocument()->Url(), BlankURL())))
-    return WebFrameLoadType::kReplaceCurrentItem;
 
   if (url == document_loader_->UrlForHistory()) {
     if (http_method == http_names::kPOST)
@@ -1038,6 +1031,9 @@ void FrameLoader::CommitNavigation(
 
   tls_version_warning_origins_.clear();
 
+  if (!DocumentLoader::WillLoadUrlAsEmpty(navigation_params->url))
+    has_loaded_non_empty_document_ = true;
+
   // TODO(dgozman): navigation type should probably be passed by the caller.
   // It seems incorrect to pass |false| for |have_event| and then use
   // determined navigation type to update resource request.

third_party/blink/renderer/core/loader/frame_loader.h

@@ -229,6 +229,8 @@ class CORE_EXPORT FrameLoader final {
 
   bool HasAccessedInitialDocument() { return has_accessed_initial_document_; }
 
+  void SetDidLoadNonEmptyDocument() { has_loaded_non_empty_document_ = true; }
+
   static bool NeedsHistoryItemRestore(WebFrameLoadType type);
 
  private:
@@ -308,6 +310,7 @@ class CORE_EXPORT FrameLoader final {
   bool detached_;
   bool committing_navigation_ = false;
   bool has_accessed_initial_document_ = false;
+  bool has_loaded_non_empty_document_ = false;
 
   WebScopedVirtualTimePauser virtual_time_pauser_;
 

third_party/blink/renderer/core/loader/frame_loader_state_machine.cc

@@ -46,10 +46,6 @@ bool FrameLoaderStateMachine::CreatingInitialEmptyDocument() const {
   return state_ == kCreatingInitialEmptyDocument;
 }
 
-bool FrameLoaderStateMachine::CommittedMultipleRealLoads() const {
-  return state_ == kCommittedMultipleRealLoads;
-}
-
 bool FrameLoaderStateMachine::IsDisplayingInitialEmptyDocument() const {
   return state_ >= kDisplayingInitialEmptyDocument &&
          state_ < kCommittedFirstRealLoad;
@@ -68,8 +64,6 @@ String FrameLoaderStateMachine::ToString() const {
       return "DisplayingInitialEmptyDocument";
     case kCommittedFirstRealLoad:
       return "CommittedFirstRealLoad";
-    case kCommittedMultipleRealLoads:
-      return "CommittedMultipleRealLoads";
     default:
       NOTREACHED();
   }

third_party/blink/renderer/core/loader/frame_loader_state_machine.h

@@ -50,14 +50,12 @@ class CORE_EXPORT FrameLoaderStateMachine {
   enum State {
     kCreatingInitialEmptyDocument,
     kDisplayingInitialEmptyDocument,
-    kCommittedFirstRealLoad,
-    kCommittedMultipleRealLoads
+    kCommittedFirstRealLoad
   };
 
   bool CommittedFirstRealDocumentLoad() const;
   bool CreatingInitialEmptyDocument() const;
   bool IsDisplayingInitialEmptyDocument() const;
-  bool CommittedMultipleRealLoads() const;
   void AdvanceTo(State);
 
   String ToString() const;

third_party/blink/web_tests/http/tests/misc/resources/check-window-reuse.html

+<script>
+top.document.body.appendChild(document.createTextNode(window.didreuse ? "FAIL" : "PASS"));
+if (window.testRunner)
+  testRunner.notifyDone();
+</script>

third_party/blink/web_tests/http/tests/misc/window-reuse-after-document-open-expected.txt

+ PASS

third_party/blink/web_tests/http/tests/misc/window-reuse-after-document-open.html

+<iframe id="i" src="https:/127.0.0.1:8443/misc/resources/hang-connection-before-head.php"></iframe>
+<script>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.waitUntilDone();
+}
+i.contentWindow.didreuse = true;
+i.contentDocument.open();
+i.contentDocument.close();
+i.contentWindow.location = 'resources/check-window-reuse.html';
+</script>