Skip to content

Commit

Permalink
[bfcache] Add notRestoredReasons field in PerformanceNavigationTiming
Browse files Browse the repository at this point in the history
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}
NOKEYCHECK=True
GitOrigin-RevId: e1af9facf232a11871b0c668b90ec17c5a5d5e16
  • Loading branch information
Yuzu Saijo authored and copybara-github committed Oct 4, 2022
1 parent 86f433d commit 2c85499
Show file tree
Hide file tree
Showing 18 changed files with 446 additions and 24 deletions.
38 changes: 38 additions & 0 deletions blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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
17 changes: 17 additions & 0 deletions blink/renderer/core/frame/local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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_;
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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_;

Expand Down
51 changes: 32 additions & 19 deletions blink/renderer/core/frame/web_local_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 8 additions & 4 deletions blink/renderer/core/frame/web_local_frame_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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*,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
55 changes: 55 additions & 0 deletions blink/renderer/core/timing/performance_navigation_timing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand All @@ -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
6 changes: 6 additions & 0 deletions blink/renderer/core/timing/performance_navigation_timing.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
4 changes: 4 additions & 0 deletions blink/web_tests/NeverFixTests
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
6 changes: 6 additions & 0 deletions blink/web_tests/VirtualTestSuites
Original file line number Diff line number Diff line change
Expand Up @@ -1312,5 +1312,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"]
}
]
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// META: title=RemoteContextHelper navigation using BFCache
// META: script=/common/dispatcher/dispatcher.js
// META: script=/common/get-host-info.sub.js
// META: script=/common/utils.js
// META: script=/resources/testharness.js
// META: script=/resources/testharnessreport.js
// META: script=/html/browsers/browsing-the-web/remote-context-helper/resources/remote-context-helper.js
// META: script=/html/browsers/browsing-the-web/remote-context-helper-tests/resources/test-helper.js

'use strict';

// Ensure that notRestoredReasons is empty for successful BFCache restore.
promise_test(async t => {
const rcHelper = new RemoteContextHelper();

// Open a window with noopener so that BFCache will work.
const rc1 = await rcHelper.addWindow(
/*config=*/ null, /*options=*/ {features: 'noopener'});

// Navigate away.
const rc2 = await rc1.navigateToNew();

// Navigate back.
await rc2.historyBack();

// Verify that no reasons are recorded for successful restore.
assert_true(await rc1.executeScript(() => {
let reasons = performance.getEntriesByType('navigation')[0].notRestoredReasons;
return reasons == null;
}));
});
Loading

0 comments on commit 2c85499

Please sign in to comment.