Improve initial empty document state tracking

Currently knowing whether a frame is showing the initial empty document
or not is hard to do because there are multiple places that try to
track the "initial empty document" state but actually fail to do so
due to various reasons:
- is_on_initial_empty_document_or_subsequent_empty_documents_ in
  FrameTreeNode and empty_document_status_ in FrameLoader are still
  true after multiple about:blank (and other about: URLs) commits that
  are not the initial/synchronous about:blank.
- has_committed_real_load_ in FrameTreeNode is used to determine the
  initial empty document state (it is used as input to
  empty_document_status_ in FrameLoader and IsInitialEmptyDocument() in
  Document), even though it does not consider document.open()

This CL fixes those problems and more:
- Sets is_on_initial_empty_document_or_subsequent_empty_documents_ and
  empty_document_status_ to false on the first commit that is not the
  initial/synchronous about:blank commit (and renames the former to
  is_on_initial_empty_document now that it actually represents the
  exact state)
- Removes uses of has_committed_real_load_ that use it to determine
  the initial empty document state (it's going to be removed completely
  in crrev.com/c/3244814)
- Renames and adds comments to various bits for easier understanding

After this CL, the "initial empty document" state tracking matches the
spec except that Chrome treats the initial about:blank document and the
synchronously loaded about:blank document as the same document (the
initial empty document), while the synchronously loaded about:blank
document does not exist in the spec (especially after
whatwg/html#6863).

Some subtle changes:
- about:blank documents that committed immediately after the
  initial/synchronous about:blank document are no longer treated as the
  initial empty document. This mainly affects history replacement
  decisions (since the initial empty document's history entry always
  gets replaced on the next navigation), so these documents' history
  entry will no longer be replaced based on their
  initial-empty-document-ness.
  Example of previous behavior:
  1. <iframe src="about:blank">. This will commit the synchronous
     about:blank document, and is correctly treated as the initial
     empty document.
  2. Navigate to about:blank or any other variants of it, e.g.
     about:blank?foo. This is a new document that is not the initial
     about:blank or the synchronous about:blank, but since it's an
     about:blank URL, it used to be treated as the initial empty
     document. With this CL, it's no longer treated as the initial
     empty document.
  3. Navigate to some other URL. This used to do replacement because
     we thought we're still on the initial empty document, but we
     actually are not. This CL fixes this case.
- Non-about:blank commits that accidentally go through the synchronous
  about:blank commit path are now correctly treated as non-initial
  empty document commits (but still go through the path for now, might
  be removed later). This includes all variants of about: URLs except
  for about:srcdoc that are not about:blank (e.g. about:mumble) - see
  the conditions in RenderFrameImpl::BeginNavigation(). This makes our
  behavior closer to the spec, which only allows about:blank
  variations to be treated as the initial empty document.

Bug: 1215096
Change-Id: Ic52bc562a4fbfdd92a63d980c75dd952a4e099d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3238476
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936787}
NOKEYCHECK=True
GitOrigin-RevId: 86c88faacbba594af79e9e90af6eac0e451c6c20

blink/public/web/web_navigation_control.h

@@ -57,11 +57,11 @@ class WebNavigationControl : public WebLocalFrame {
       bool is_browser_initiated,
       std::unique_ptr<WebDocumentLoader::ExtraData> extra_data) = 0;
 
-  // Override the normal rules for whether a load has successfully committed
-  // in this frame. Used to propagate state when this frame has navigated
-  // cross process.
-  virtual void SetCommittedFirstRealLoad() = 0;
-  virtual bool HasCommittedFirstRealLoad() = 0;
+  // Override the normal rules that determine whether the frame is on the
+  // initial empty document or not. Used to propagate state when this frame has
+  // navigated cross process.
+  virtual void SetIsNotOnInitialEmptyDocument() = 0;
+  virtual bool IsOnInitialEmptyDocument() = 0;
 
   // Marks the frame as loading, before WebLocalFrameClient issues a navigation
   // request through the browser process on behalf of the frame.

blink/renderer/core/app_history/app_history.cc

@@ -588,7 +588,11 @@ void AppHistory::PromoteUpcomingNavigationToOngoing(const String& key) {
 
 bool AppHistory::HasEntriesAndEventsDisabled() const {
   auto* frame = GetSupplementable()->GetFrame();
-  return !frame || !frame->Loader().HasLoadedNonEmptyDocument() ||
+  return !frame ||
+         !GetSupplementable()
+              ->GetFrame()
+              ->Loader()
+              .HasLoadedNonInitialEmptyDocument() ||
          GetSupplementable()->GetSecurityOrigin()->IsOpaque();
 }
 

blink/renderer/core/frame/frame_test_helpers.cc

@@ -716,7 +716,7 @@ void TestWebFrameClient::BeginNavigation(
     std::unique_ptr<WebNavigationInfo> info) {
   navigation_callback_.Cancel();
   if (DocumentLoader::WillLoadUrlAsEmpty(info->url_request.Url()) &&
-      !frame_->HasCommittedFirstRealLoad()) {
+      frame_->IsOnInitialEmptyDocument()) {
     CommitNavigation(std::move(info));
     return;
   }

blink/renderer/core/frame/web_frame_test.cc

@@ -9674,7 +9674,7 @@ TEST_F(WebFrameSwapTest, HistoryCommitTypeAfterExistingRemoteToLocalSwap) {
   RemoteToLocalSwapWebFrameClient client;
   WebLocalFrameImpl* local_frame =
       web_view_helper_.CreateProvisional(*remote_frame, &client);
-  local_frame->SetCommittedFirstRealLoad();
+  local_frame->SetIsNotOnInitialEmptyDocument();
   frame_test_helpers::LoadFrame(local_frame, base_url_ + "subframe-hello.html");
   EXPECT_EQ(kWebStandardCommit, client.HistoryCommitType());
 

blink/renderer/core/frame/web_local_frame_impl.cc

@@ -2389,16 +2389,16 @@ bool WebLocalFrameImpl::IsNavigationScheduledWithin(
          GetFrame()->GetDocument()->IsHttpRefreshScheduledWithin(interval);
 }
 
-void WebLocalFrameImpl::SetCommittedFirstRealLoad() {
+void WebLocalFrameImpl::SetIsNotOnInitialEmptyDocument() {
   DCHECK(GetFrame());
   GetFrame()->GetDocument()->OverrideIsInitialEmptyDocument();
-  GetFrame()->Loader().SetDidLoadNonEmptyDocument();
+  GetFrame()->Loader().SetIsNotOnInitialEmptyDocument();
   GetFrame()->SetShouldSendResourceTimingInfoToParent(false);
 }
 
-bool WebLocalFrameImpl::HasCommittedFirstRealLoad() {
+bool WebLocalFrameImpl::IsOnInitialEmptyDocument() {
   DCHECK(GetFrame());
-  return !GetFrame()->GetDocument()->IsInitialEmptyDocument();
+  return GetFrame()->GetDocument()->IsInitialEmptyDocument();
 }
 
 void WebLocalFrameImpl::BlinkFeatureUsageReport(

blink/renderer/core/frame/web_local_frame_impl.h

@@ -351,8 +351,8 @@ class CORE_EXPORT WebLocalFrameImpl final
       const WebSecurityOrigin& initiator_origin,
       bool is_browser_initiated,
       std::unique_ptr<WebDocumentLoader::ExtraData> extra_data) override;
-  void SetCommittedFirstRealLoad() override;
-  bool HasCommittedFirstRealLoad() override;
+  void SetIsNotOnInitialEmptyDocument() override;
+  bool IsOnInitialEmptyDocument() override;
   bool WillStartNavigation(const WebNavigationInfo&) override;
   void DidDropNavigation() override;
   void DownloadURL(

blink/renderer/core/loader/frame_loader.cc

@@ -338,8 +338,11 @@ void FrameLoader::DispatchUnloadEvent(
 
 void FrameLoader::DidExplicitOpen() {
   probe::DidOpenDocument(frame_, GetDocumentLoader());
-  if (empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty)
-    empty_document_status_ = EmptyDocumentStatus::kOnlyEmptyButExplicitlyOpened;
+  if (initial_empty_document_status_ ==
+      InitialEmptyDocumentStatus::kInitialOrSynchronousAboutBlank) {
+    initial_empty_document_status_ = InitialEmptyDocumentStatus::
+        kInitialOrSynchronousAboutBlankButExplicitlyOpened;
+  }
 
   // Only model a document.open() as part of a navigation if its parent is not
   // done or in the process of completing.
@@ -479,8 +482,7 @@ WebFrameLoadType FrameLoader::HandleInitialEmptyDocumentReplacementIfNeeded(
   // needed.
   if (frame_load_type == WebFrameLoadType::kStandard ||
       frame_load_type == WebFrameLoadType::kReplaceCurrentItem) {
-    if (frame_->Tree().Parent() &&
-        empty_document_status_ == EmptyDocumentStatus::kOnlyEmpty) {
+    if (frame_->Tree().Parent() && IsOnInitialEmptyDocument()) {
       // Subframe navigations from the initial empty document should always do
       // replacement.
       return WebFrameLoadType::kReplaceCurrentItem;
@@ -1053,8 +1055,27 @@ void FrameLoader::CommitNavigation(
 
   tls_version_warning_origins_.clear();
 
-  if (!DocumentLoader::WillLoadUrlAsEmpty(navigation_params->url))
-    empty_document_status_ = EmptyDocumentStatus::kNonEmpty;
+  if (!navigation_params->is_synchronous_commit_for_bug_778318 ||
+      (!navigation_params->url.IsEmpty() &&
+       !KURL(navigation_params->url).IsAboutBlankURL())) {
+    // The new document is not the synchronously committed about:blank document,
+    // so lose the initial empty document status.
+    // Note 1: The actual initial empty document commit (with commit_reason set
+    // to CommitReason::kInitialization) won't go through this path since it
+    // immediately commits the DocumentLoader, so we only check for the
+    // synchronous about:blank commit here.
+    // Note 2: Even if the navigation is a synchronous one, it might be a
+    // non-about:blank/empty URL commit that is accidentally got caught by the
+    // synchronous about:blank path but can't easily be removed due to failing
+    // tests/compatibility risk (e.g. about:mumble).
+    // TODO(https://crbug.com/1215096): Tighten the conditions in
+    // RenderFrameImpl::BeginNavigation() for a navigation to enter the
+    // synchronous commit path to only accept about:blank or an empty URL which
+    // defaults to about:blank, per the spec:
+    // https://html.spec.whatwg.org/multipage/iframe-embed-object.html#the-iframe-element:about:blank
+    DCHECK_NE(commit_reason, CommitReason::kInitialization);
+    SetIsNotOnInitialEmptyDocument();
+  }
 
   // TODO(dgozman): navigation type should probably be passed by the caller.
   // It seems incorrect to pass |false| for |have_event| and then use

blink/renderer/core/loader/frame_loader.h

@@ -220,11 +220,35 @@ class CORE_EXPORT FrameLoader final {
 
   bool HasAccessedInitialDocument() { return has_accessed_initial_document_; }
 
-  void SetDidLoadNonEmptyDocument() {
-    empty_document_status_ = EmptyDocumentStatus::kNonEmpty;
+  void SetIsNotOnInitialEmptyDocument() {
+    // The "initial empty document" state can be false if the frame has loaded
+    // a non-initial/synchronous about:blank document, or if the document has
+    // done a document.open() before. However, this function can only be called
+    // when a frame is first re-created in a new renderer, which can only be
+    // caused by a new document load. So, we know that the state must be set to
+    // kNotInitialOrSynchronousAboutBlank instead of
+    // kInitialOrSynchronousAboutBlankButExplicitlyOpened here.
+    initial_empty_document_status_ =
+        InitialEmptyDocumentStatus::kNotInitialOrSynchronousAboutBlank;
   }
-  bool HasLoadedNonEmptyDocument() {
-    return empty_document_status_ == EmptyDocumentStatus::kNonEmpty;
+
+  // Whether the frame's current document is still considered as the "initial
+  // empty document" or not. Might be false even when
+  // HasLoadedNonInitialEmptyDocument() is false, if the frame is still on the
+  // first about:blank document that loaded in the frame, but it has done
+  // a document.open(), causing it to lose its "initial empty document"-ness
+  // even though it's still on the same document.
+  bool IsOnInitialEmptyDocument() {
+    return initial_empty_document_status_ ==
+           InitialEmptyDocumentStatus::kInitialOrSynchronousAboutBlank;
+  }
+
+  // Whether the frame has loaded a document that is not the initial empty
+  // document. Might be false even when IsOnInitialEmptyDocument() is false (see
+  // comment for IsOnInitialEmptyDocument() for details).
+  bool HasLoadedNonInitialEmptyDocument() {
+    return initial_empty_document_status_ ==
+           InitialEmptyDocumentStatus::kNotInitialOrSynchronousAboutBlank;
   }
 
   static bool NeedsHistoryItemRestore(WebFrameLoadType type);
@@ -292,12 +316,30 @@ class CORE_EXPORT FrameLoader final {
   bool committing_navigation_ = false;
   bool has_accessed_initial_document_ = false;
 
-  enum class EmptyDocumentStatus {
-    kOnlyEmpty,
-    kOnlyEmptyButExplicitlyOpened,
-    kNonEmpty
+  // Enum to determine the frame's "initial empty document"-ness.
+  // NOTE: we treat both the "initial about:blank document" and the
+  // "synchronously committed about:blank document" as the initial empty
+  // document. In the future, we plan to remove the synchronous about:blank
+  // commit so that this enum only considers the true "initial about:blank"
+  // document. See also:
+  // - https://github.com/whatwg/html/issues/6863
+  // - https://crbug.com/1215096
+  enum class InitialEmptyDocumentStatus {
+    // The document is the initial about:blank document or the synchronously
+    // committed about:blank document.
+    kInitialOrSynchronousAboutBlank,
+    // The document is the initial about:blank document or the synchronously
+    // committed about:blank document, but the document's input stream has been
+    // opened with document.open(), so the document lost its "initial empty
+    // document" status, per the spec:
+    // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#opening-the-input-stream:is-initial-about:blank
+    kInitialOrSynchronousAboutBlankButExplicitlyOpened,
+    // The document is neither the initial about:blank document nor the
+    // synchronously committed about:blank document.
+    kNotInitialOrSynchronousAboutBlank
   };
-  EmptyDocumentStatus empty_document_status_ = EmptyDocumentStatus::kOnlyEmpty;
+  InitialEmptyDocumentStatus initial_empty_document_status_ =
+      InitialEmptyDocumentStatus::kInitialOrSynchronousAboutBlank;
 
   WebScopedVirtualTimePauser virtual_time_pauser_;
 

blink/renderer/core/loader/http_refresh_scheduler.cc

@@ -114,7 +114,7 @@ void HttpRefreshScheduler::NavigateTask() {
   // in a frame where there hasn't actually been a navigation yet. Therefore,
   // don't treat as a reload if all this frame has ever seen is empty documents.
   if (EqualIgnoringFragmentIdentifier(document_->Url(), refresh->url) &&
-      document_->GetFrame()->Loader().HasLoadedNonEmptyDocument()) {
+      document_->GetFrame()->Loader().HasLoadedNonInitialEmptyDocument()) {
     request.GetResourceRequest().SetCacheMode(
         mojom::FetchCacheMode::kValidateCache);
     load_type = WebFrameLoadType::kReload;