Reland "[bfcache] Add notRestoredReasons field in PerformanceNavigati…

…onTiming" This is a reland of commit e1af9facf232a11871b0c668b90ec17c5a5d5e16 To address the test failure, in this reland I added IsOutermostMainFrame() condition in web_view_impl.cc. Original change's description: > [bfcache] Add notRestoredReasons field in PerformanceNavigationTiming > > This CL adds back/forward cache not restored reasons in Performance > NavigationTiming API. > It's proposed here: > w3c/navigation-timing#171 > Explainer: https://github.com/rubberyuzu/bfcache-not-retored-reason/blob/main/NotRestoredReason.md > > This CL does two things: > - exposes not restored reasons to PerformanceNavigationTiming API > - adds WPT > > The WPT have to run on all platforms because this API is intended to > collect bfcache metrics from the wild, regardless of the platforms. > > LOW_COVERAGE_REASON=Adding coverage for new field. Just missing the tests for the existing fields > > Bug: 1349228 > Change-Id: I8dd96a60188bdff94a21c4e4e34cacf181a823db > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3835534 > Reviewed-by: Daniel Cheng <[email protected]> > Reviewed-by: Kentaro Hara <[email protected]> > Reviewed-by: Weizhong Xia <[email protected]> > Commit-Queue: Yuzu Saijo <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1054800} Bug: 1349228 Change-Id: I010009593e15dcb9da20cc9ee4982294e935f57a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3934434 Reviewed-by: Kentaro Hara <[email protected]> Reviewed-by: Kent Tamura <[email protected]> Commit-Queue: Yuzu Saijo <[email protected]> Reviewed-by: Daniel Cheng <[email protected]> Cr-Commit-Position: refs/heads/main@{#1055140} NOKEYCHECK=True GitOrigin-RevId: 5a21d0235141012a993fd2b9aca8479dc6ab0a76
mjfroman · Oct 5, 2022 · cc17b80 · cc17b80
1 parent 1951986
commit cc17b80
Show file tree

Hide file tree

Showing 19 changed files with 447 additions and 25 deletions.
diff --git a/blink/renderer/core/exported/web_view_impl.cc b/blink/renderer/core/exported/web_view_impl.cc
@@ -2592,7 +2592,7 @@ void WebViewImpl::DispatchPersistedPageshow(base::TimeTicks navigation_start) {
   // history navigation is completed. For history navigation that is not
   // restored from back/forward cache, we set a new value in
   // |CommitNavigationWithParams()|.
-  if (MainFrame()->IsWebLocalFrame()) {
+  if (MainFrame()->IsWebLocalFrame() && MainFrame()->IsOutermostMainFrame()) {
     MainFrame()->ToWebLocalFrame()->SetNotRestoredReasons(nullptr);
   }
 

diff --git a/blink/renderer/core/frame/local_frame.cc b/blink/renderer/core/frame/local_frame.cc
@@ -594,6 +594,8 @@ bool LocalFrame::DetachImpl(FrameDetachType type) {
   if (text_fragment_handler_)
     text_fragment_handler_->DidDetachDocumentOrFrame();
 
+  not_restored_reasons_.reset();
+
   DCHECK(!view_->IsAttached());
   Client()->WillBeDetached();
 
@@ -3314,4 +3316,40 @@ void LocalFrame::MaybeUpdateWindowControlsOverlayWithNewZoomLevel() {
 }
 #endif  // !BUILDFLAG(IS_ANDROID)
 
+void LocalFrame::SetNotRestoredReasons(
+    mojom::blink::BackForwardCacheNotRestoredReasonsPtr not_restored_reasons) {
+  // Back/forward cache is only enabled for outermost main frame.
+  DCHECK(IsOutermostMainFrame());
+  not_restored_reasons_ = mojo::Clone(not_restored_reasons);
+}
+
+const mojom::blink::BackForwardCacheNotRestoredReasonsPtr&
+LocalFrame::GetNotRestoredReasons() {
+  // Back/forward cache is only enabled for the outermost main frames, and the
+  // web exposed API returns non-null values only for the outermost main frames.
+  DCHECK(IsOutermostMainFrame());
+  return not_restored_reasons_;
+}
+
+bool LocalFrame::HasBlockingReasons() {
+  DCHECK(IsOutermostMainFrame());
+  if (!not_restored_reasons_)
+    return false;
+  return HasBlockingReasonsHelper(not_restored_reasons_);
+}
+
+bool LocalFrame::HasBlockingReasonsHelper(
+    const mojom::blink::BackForwardCacheNotRestoredReasonsPtr& not_restored) {
+  if (not_restored->blocked)
+    return true;
+  if (not_restored->same_origin_details) {
+    for (const auto& child : not_restored->same_origin_details->children) {
+      if (HasBlockingReasonsHelper(child))
+        return true;
+    }
+    return false;
+  }
+  return false;
+}
+
 }  // namespace blink
diff --git a/blink/renderer/core/frame/local_frame.h b/blink/renderer/core/frame/local_frame.h
@@ -43,6 +43,7 @@
 #include "third_party/blink/public/common/frame/frame_ad_evidence.h"
 #include "third_party/blink/public/common/frame/transient_allow_fullscreen.h"
 #include "third_party/blink/public/common/tokens/tokens.h"
+#include "third_party/blink/public/mojom/back_forward_cache_not_restored_reasons.mojom-blink.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom-blink-forward.h"
 #include "third_party/blink/public/mojom/devtools/devtools_agent.mojom-blink-forward.h"
 #include "third_party/blink/public/mojom/devtools/inspector_issue.mojom-blink-forward.h"
@@ -604,6 +605,15 @@ class CORE_EXPORT LocalFrame final
   mojom::blink::BackForwardCacheControllerHost&
   GetBackForwardCacheControllerHostRemote();
 
+  // Sets back/forward cache NotRestoredReasons for this frame. Only set for
+  // outermost main frame.
+  void SetNotRestoredReasons(
+      mojom::blink::BackForwardCacheNotRestoredReasonsPtr);
+  const mojom::blink::BackForwardCacheNotRestoredReasonsPtr&
+  GetNotRestoredReasons();
+  // Returns if the saved NotRestoredReasons has any blocking reasons.
+  bool HasBlockingReasons();
+
   const AtomicString& GetReducedAcceptLanguage() const {
     return reduced_accept_language_;
   }
@@ -871,6 +881,10 @@ class CORE_EXPORT LocalFrame final
                                     String& clip_html,
                                     gfx::Rect& clip_rect);
 
+  // Helper function for |HasBlockingReasons()|.
+  bool HasBlockingReasonsHelper(
+      const mojom::blink::BackForwardCacheNotRestoredReasonsPtr&);
+
 #if !BUILDFLAG(IS_ANDROID)
   void SetTitlebarAreaDocumentStyleEnvironmentVariables() const;
   void MaybeUpdateWindowControlsOverlayWithNewZoomLevel();
@@ -943,6 +957,9 @@ class CORE_EXPORT LocalFrame final
 
   mojom::blink::ViewportIntersectionState intersection_state_;
 
+  // Only set for outermost main frame.
+  mojom::blink::BackForwardCacheNotRestoredReasonsPtr not_restored_reasons_;
+
   // Per-frame URLLoader factory.
   std::unique_ptr<WebURLLoaderFactory> url_loader_factory_;
 

diff --git a/blink/renderer/core/frame/web_local_frame_impl.cc b/blink/renderer/core/frame/web_local_frame_impl.cc
@@ -3041,30 +3041,43 @@ void WebLocalFrameImpl::SetSessionStorageArea(
 
 void WebLocalFrameImpl::SetNotRestoredReasons(
     const mojom::BackForwardCacheNotRestoredReasonsPtr& not_restored_reasons) {
-  not_restored_reasons_ =
-      not_restored_reasons.is_null()
-          ? mojom::BackForwardCacheNotRestoredReasonsPtr(nullptr)
-          : not_restored_reasons->Clone();
+  GetFrame()->SetNotRestoredReasons(
+      ConvertNotRestoredReasons(not_restored_reasons));
 }
 
 bool WebLocalFrameImpl::HasBlockingReasons() {
-  if (!not_restored_reasons_)
-    return false;
-  return HasBlockingReasonsHelper(not_restored_reasons_);
-}
-
-bool WebLocalFrameImpl::HasBlockingReasonsHelper(
-    const mojom::BackForwardCacheNotRestoredReasonsPtr& not_restored) {
-  if (not_restored->blocked)
-    return true;
-  if (not_restored->same_origin_details) {
-    for (const auto& child : not_restored->same_origin_details->children) {
-      if (HasBlockingReasonsHelper(child))
-        return true;
+  return GetFrame()->HasBlockingReasons();
+}
+
+const mojom::blink::BackForwardCacheNotRestoredReasonsPtr&
+WebLocalFrameImpl::GetNotRestoredReasons() {
+  return GetFrame()->GetNotRestoredReasons();
+}
+
+mojom::blink::BackForwardCacheNotRestoredReasonsPtr
+WebLocalFrameImpl::ConvertNotRestoredReasons(
+    const mojom::BackForwardCacheNotRestoredReasonsPtr& reasons_to_copy) {
+  mojom::blink::BackForwardCacheNotRestoredReasonsPtr not_restored_reasons;
+  if (!reasons_to_copy.is_null()) {
+    not_restored_reasons =
+        mojom::blink::BackForwardCacheNotRestoredReasons::New();
+    not_restored_reasons->blocked = reasons_to_copy->blocked;
+    auto details = mojom::blink::SameOriginBfcacheNotRestoredDetails::New();
+    if (reasons_to_copy->same_origin_details) {
+      details->id = reasons_to_copy->same_origin_details->id.c_str();
+      details->name = reasons_to_copy->same_origin_details->name.c_str();
+      details->src = reasons_to_copy->same_origin_details->src.c_str();
+      details->url = reasons_to_copy->same_origin_details->url.c_str();
+      for (const auto& reason : reasons_to_copy->same_origin_details->reasons) {
+        details->reasons.push_back(reason.c_str());
+      }
+      for (const auto& child : reasons_to_copy->same_origin_details->children) {
+        details->children.push_back(ConvertNotRestoredReasons(child));
+      }
     }
-    return false;
+    not_restored_reasons->same_origin_details = std::move(details);
   }
-  return not_restored->blocked;
+  return not_restored_reasons;
 }
 
 void WebLocalFrameImpl::AddHitTestOnTouchStartCallback(

diff --git a/blink/renderer/core/frame/web_local_frame_impl.h b/blink/renderer/core/frame/web_local_frame_impl.h
@@ -386,6 +386,9 @@ class CORE_EXPORT WebLocalFrameImpl final
   // Returns if the current frame's NotRestoredReasons has any blocking reasons.
   bool HasBlockingReasons() override;
 
+  const mojom::blink::BackForwardCacheNotRestoredReasonsPtr&
+  GetNotRestoredReasons();
+
   void InitializeCoreFrame(
       Page&,
       FrameOwner*,
@@ -555,10 +558,6 @@ class CORE_EXPORT WebLocalFrameImpl final
   // Sets the local core frame and registers destruction observers.
   void SetCoreFrame(LocalFrame*);
 
-  // Helper function for |HasBlockingReasons()|.
-  bool HasBlockingReasonsHelper(
-      const mojom::BackForwardCacheNotRestoredReasonsPtr&);
-
   // Inherited from WebFrame, but intentionally hidden: it never makes sense
   // to call these on a WebLocalFrameImpl.
   bool IsWebLocalFrame() const override;
@@ -611,6 +610,11 @@ class CORE_EXPORT WebLocalFrameImpl final
       network::mojom::blink::WebSandboxFlags sandbox_flags =
           network::mojom::blink::WebSandboxFlags::kNone);
 
+  // This function converts mojom::BackForwardCacheNotRestoredReasonsPtr to
+  // mojom::blink::BackForwardCacheNotRestoredReasonsPtr.
+  mojom::blink::BackForwardCacheNotRestoredReasonsPtr ConvertNotRestoredReasons(
+      const mojom::BackForwardCacheNotRestoredReasonsPtr& reasons_struct);
+
   WebLocalFrameClient* client_;
 
   // TODO(dcheng): Inline this field directly rather than going through Member.

diff --git a/blink/renderer/core/timing/performance_navigation_timing.cc b/blink/renderer/core/timing/performance_navigation_timing.cc
@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/document_timing.h"
 #include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/loader/document_load_timing.h"
 #include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/performance_entry_names.h"
@@ -310,6 +311,54 @@ DOMHighResTimeStamp PerformanceNavigationTiming::duration() const {
   return loadEventEnd();
 }
 
+ScriptValue PerformanceNavigationTiming::notRestoredReasons(
+    ScriptState* script_state) const {
+  DocumentLoader* loader = GetDocumentLoader();
+  if (!loader || !loader->GetFrame()->IsOutermostMainFrame())
+    return ScriptValue::CreateNull(script_state->GetIsolate());
+
+  // TODO(crbug.com/1370954): Save NotRestoredReasons in Document instead of
+  // Frame.
+  return NotRestoredReasonsBuilder(script_state,
+                                   loader->GetFrame()->GetNotRestoredReasons());
+}
+
+ScriptValue PerformanceNavigationTiming::NotRestoredReasonsBuilder(
+    ScriptState* script_state,
+    const mojom::blink::BackForwardCacheNotRestoredReasonsPtr& reasons) const {
+  if (!reasons)
+    return ScriptValue::CreateNull(script_state->GetIsolate());
+  V8ObjectBuilder builder(script_state);
+  builder.AddBoolean("blocked", reasons->blocked);
+  builder.AddString("url", AtomicString(reasons->same_origin_details
+                                            ? reasons->same_origin_details->url
+                                            : ""));
+  builder.AddString("src", AtomicString(reasons->same_origin_details
+                                            ? reasons->same_origin_details->src
+                                            : ""));
+  builder.AddString("id", AtomicString(reasons->same_origin_details
+                                           ? reasons->same_origin_details->id
+                                           : ""));
+  builder.AddString("name",
+                    AtomicString(reasons->same_origin_details
+                                     ? reasons->same_origin_details->name
+                                     : ""));
+  Vector<AtomicString> reason_strings;
+  Vector<v8::Local<v8::Value>> children_result;
+  if (reasons->same_origin_details) {
+    for (const auto& reason : reasons->same_origin_details->reasons) {
+      reason_strings.push_back(reason);
+    }
+    for (const auto& child : reasons->same_origin_details->children) {
+      children_result.push_back(
+          NotRestoredReasonsBuilder(script_state, child).V8Value());
+    }
+  }
+  builder.Add("reasons", reason_strings);
+  builder.Add("children", children_result);
+  return builder.GetScriptValue();
+}
+
 void PerformanceNavigationTiming::BuildJSONValue(
     V8ObjectBuilder& builder) const {
   PerformanceResourceTiming::BuildJSONValue(builder);
@@ -330,5 +379,11 @@ void PerformanceNavigationTiming::BuildJSONValue(
         "activationStart",
         PerformanceNavigationTimingActivationStart::activationStart(*this));
   }
+
+  if (RuntimeEnabledFeatures::BackForwardCacheNotRestoredReasonsEnabled(
+          ExecutionContext::From(builder.GetScriptState()))) {
+    builder.Add("notRestoredReasons",
+                notRestoredReasons(builder.GetScriptState()));
+  }
 }
 }  // namespace blink
diff --git a/blink/renderer/core/timing/performance_navigation_timing.h b/blink/renderer/core/timing/performance_navigation_timing.h
@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_PERFORMANCE_NAVIGATION_TIMING_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_PERFORMANCE_NAVIGATION_TIMING_H_
 
+#include "third_party/blink/public/mojom/back_forward_cache_not_restored_reasons.mojom-blink.h"
 #include "third_party/blink/public/web/web_navigation_type.h"
 #include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/dom/dom_high_res_time_stamp.h"
@@ -55,6 +56,7 @@ class CORE_EXPORT PerformanceNavigationTiming final
   DOMHighResTimeStamp loadEventEnd() const;
   AtomicString type() const;
   uint16_t redirectCount() const;
+  ScriptValue notRestoredReasons(ScriptState* script_state) const;
 
   // PerformanceResourceTiming overrides:
   DOMHighResTimeStamp fetchStart() const override;
@@ -88,6 +90,10 @@ class CORE_EXPORT PerformanceNavigationTiming final
   AtomicString AlpnNegotiatedProtocol() const override;
   AtomicString ConnectionInfo() const override;
 
+  ScriptValue NotRestoredReasonsBuilder(
+      ScriptState* script_state,
+      const mojom::blink::BackForwardCacheNotRestoredReasonsPtr& reasons) const;
+
   scoped_refptr<ResourceTimingInfo> resource_timing_info_;
 };
 }  // namespace blink

diff --git a/blink/renderer/core/timing/performance_navigation_timing.idl b/blink/renderer/core/timing/performance_navigation_timing.idl
@@ -23,5 +23,6 @@ enum NavigationType {
     readonly attribute DOMHighResTimeStamp loadEventEnd;
     readonly attribute NavigationType      type;
     readonly attribute unsigned short      redirectCount;
+    [RuntimeEnabled=BackForwardCacheNotRestoredReasons, CallWith=ScriptState] readonly attribute object notRestoredReasons;
     [CallWith=ScriptState, ImplementedAs=toJSONForBinding] object toJSON();
 };
diff --git a/blink/web_tests/NeverFixTests b/blink/web_tests/NeverFixTests
@@ -1954,3 +1954,7 @@ crbug.com/1293679 virtual/pending-beacon/external/wpt/pending_beacon/* [ Pass ]
 # Experimental JS shared memory features
 crbug.com/1351118 wpt_internal/js/shared_memory/* [ Skip ]
 crbug.com/1351118 virtual/js-shared-memory/wpt_internal/js/shared_memory/* [ Pass ]
+
+# Feature is not yet launched, so skip the base.
+crbug.com/1326344 external/wpt/performance-timeline/not-restored-reasons/* [ Skip ]
+crbug.com/1326344 virtual/not-restored-reasons/external/wpt/performance-timeline/not-restored-reasons/* [ Pass ]
diff --git a/blink/web_tests/VirtualTestSuites b/blink/web_tests/VirtualTestSuites
@@ -1313,5 +1313,11 @@
     "platforms": ["Linux", "Mac", "Win"],
     "bases": ["external/wpt/custom-elements/scoped-registry"],
     "args": ["--disable-blink-features=ScopedCustomElementRegistry"]
+  },
+  {
+    "prefix": "not-restored-reasons",
+    "platforms": ["Linux", "Mac", "Win"],
+    "bases": ["external/wpt/performance-timeline/not-restored-reasons/"],
+    "args": ["--enable-features=BackForwardCacheSendNotRestoredReasons"]
   }
 ]
diff --git a/...l/wpt/html/browsers/browsing-the-web/remote-context-helper-tests/resources/test-helper.js b/...l/wpt/html/browsers/browsing-the-web/remote-context-helper-tests/resources/test-helper.js
@@ -46,3 +46,40 @@ async function assertHeaderIsAsExpected(
         return res.headers.get(headerName);
       }, [headerName]), 'header is set');
 }
+
+async function  assertNotRestoredReasonsEquals(
+  remoteContextHelper, blocked, url, src, id, name, reasons, children) {
+  let result = await remoteContextHelper.executeScript(() => {
+    return performance.getEntriesByType('navigation')[0].notRestoredReasons;
+  });
+  assertReasonsStructEquals(result, blocked, url, src, id, name, reasons, children);
+}
+
+function assertReasonsStructEquals(result, blocked, url, src, id, name, reasons, children) {
+  assert_equals(result.blocked, blocked);
+  assert_equals(result.url, url);
+  assert_equals(result.src, src);
+  assert_equals(result.id, id);
+  assert_equals(result.name, name);
+  // Reasons should match.
+  assert_equals(result.reasons.length, reasons.length);
+  reasons.sort();
+  result.reasons.sort();
+  for (let i=0; i<reasons.length; i++) {
+    assert_equals(result.reasons[i], reasons[i]);
+  }
+  // Children should match.
+  assert_equals(result.children.length, children.length);
+  children.sort();
+  result.children.sort();
+  for (let j=0; j<children.length; j++) {
+    assertReasonsStructEquals(result.children[0],
+      children[0].blocked,
+      children[0].url,
+      children[0].src,
+      children[0].id,
+      children[0].name,
+      children[0].reasons,
+      children[0].children);
+  }
+}