Skip to content

Commit

Permalink
[bfcache] Rename reasons to match the spec
Browse files Browse the repository at this point in the history
This CL renames the reporting strings of NotRestoredReasons API so that
they match the spec draft[1].

[1]: whatwg/html#10154

Bug: 331754704
Change-Id: If805ad7e30c14ff64b440c8d9515eaee38370fd8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5392283
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1280059}
  • Loading branch information
rubberyuzu authored and Chromium LUCI CQ committed Mar 29, 2024
1 parent cf85acd commit b0bf2c1
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 47 deletions.
4 changes: 2 additions & 2 deletions components/back_forward_cache/back_forward_cache_disable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ std::string ReasonIdToReportString(DisabledReasonId reason_id) {
case DisabledReasonId::kExtensionSentMessageToCachedFrame:
return "extension-messaging";
case DisabledReasonId::kModalDialog:
return "modal-dialog";
return "modals";
case DisabledReasonId::kPermissionRequestManager:
return "permission-request-manager";
return "pending-permission-request";
default:
return "masked";
}
Expand Down
2 changes: 1 addition & 1 deletion content/browser/back_forward_cache_features_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5013,7 +5013,7 @@ class BackForwardCacheBrowserTestWithSupportedFeatures
protected:
void SetUpCommandLine(base::CommandLine* command_line) override {
EnableFeatureAndSetParams(features::kBackForwardCache, "supported_features",
"broadcastchannel,keyboard-lock");
"broadcastchannel,keyboardlock");
BackForwardCacheBrowserTest::SetUpCommandLine(command_line);
}
};
Expand Down
4 changes: 2 additions & 2 deletions content/browser/back_forward_cache_no_store_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ IN_PROC_BROWSER_TEST_P(
auto subframe_result = MatchesNotRestoredReasons(
/*id=*/"", /*name=*/"", /*src=*/url_a_no_store.spec(),
/*reasons=*/
{MatchesDetailedReason("cache-control-no-store",
{MatchesDetailedReason("response-cache-control-no-store",
/*source=*/std::nullopt)},
MatchesSameOriginDetails(
/*url=*/url_a_no_store,
Expand All @@ -969,7 +969,7 @@ IN_PROC_BROWSER_TEST_P(
MatchesNotRestoredReasons(
/*id=*/std::nullopt, /*name=*/std::nullopt, /*src=*/std::nullopt,
/*reasons=*/
{MatchesDetailedReason("cache-control-no-store",
{MatchesDetailedReason("response-cache-control-no-store",
/*source=*/std::nullopt)},
MatchesSameOriginDetails(
/*url=*/url_a_no_store,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ IN_PROC_BROWSER_TEST_F(BackForwardCacheBrowserTestWithNotRestoredReasons,
/*id=*/std::nullopt,
/*name=*/std::nullopt, /*src=*/std::nullopt,
/*reasons=*/
{MatchesDetailedReason("related-active-contents",
{MatchesDetailedReason("non-trivial-browsing-context-group",
/*source=*/std::nullopt),
MatchesDetailedReason("masked", /*source=*/std::nullopt)},
MatchesSameOriginDetails(
Expand Down
2 changes: 1 addition & 1 deletion content/browser/back_forward_cache_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace content {
namespace {
constexpr auto* kBlockingPagePath =
"/back_forward_cache/page_with_blocking_feature.html";
constexpr auto* kBlockingReasonString = "webxr";
constexpr auto* kBlockingReasonString = "webxrdevice";
constexpr auto kBlockingReasonEnum =
blink::scheduler::WebSchedulerTrackedFeature::kWebXR;
} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,27 +452,27 @@ std::string
BackForwardCacheCanStoreDocumentResult::NotRestoredReasonToReportString(
BackForwardCacheMetrics::NotRestoredReason reason) const {
switch (reason) {
// TODO(crbug.com/1349223): Add string to all reasons. Be sure to mask
// extension related reasons so that its presence would not be visible to
// the API.
// Report strings have to match the ones defined in the spec.
// If you ever add a new one, you have to add it to the spec as well.
// https://html.spec.whatwg.org/#nrr-details-reason
case Reason::kNotPrimaryMainFrame:
return "not-main-frame";
case Reason::kRelatedActiveContentsExist:
return "related-active-contents";
return "non-trivial-browsing-context-group";
case Reason::kSchemeNotHTTPOrHTTPS:
return "not-http-https-scheme";
return "response-scheme-not-http-or-https";
case Reason::kLoading:
return "loading";
return "navigating";
case Reason::kWasGrantedMediaAccess:
return "granted-media-access";
case Reason::kBlocklistedFeatures:
// This should not be reported. Instead actual feature list will be
// reported.
return "Blocklisted feature";
case Reason::kHTTPMethodNotGET:
return "http-not-get";
return "response-method-not-get";
case Reason::kSubframeIsNavigating:
return "subframe-navigating";
return "frame-navigating";
case Reason::kTimeout:
return "timeout";
case Reason::kServiceWorkerVersionActivation:
Expand All @@ -486,15 +486,15 @@ BackForwardCacheCanStoreDocumentResult::NotRestoredReasonToReportString(
case Reason::kServiceWorkerClaim:
return "serviceworker-claim";
case Reason::kNavigationCancelledWhileRestoring:
return "navigation-cancelled";
return "navigation-canceled";
case Reason::kServiceWorkerUnregistration:
return "serviceworker-unregistration";
case Reason::kErrorDocument:
case Reason::kHTTPStatusNotOK:
return "navigation-failure";
return "response-status-not-ok";
case Reason::kUnloadHandlerExistsInMainFrame:
case Reason::kUnloadHandlerExistsInSubFrame:
return "unload-handler";
return "unload-listener";
case Reason::kNetworkRequestRedirected:
case Reason::kNetworkRequestTimeout:
case Reason::kNetworkExceedsBufferLimit:
Expand All @@ -505,13 +505,13 @@ BackForwardCacheCanStoreDocumentResult::NotRestoredReasonToReportString(
case Reason::kCacheControlNoStore:
case Reason::kCacheControlNoStoreCookieModified:
case Reason::kCacheControlNoStoreHTTPOnlyCookieModified:
return "cache-control-no-store";
return "response-cache-control-no-store";
case Reason::kCookieDisabled:
return "cookie-disabled";
case Reason::kHTTPAuthRequired:
return "http-auth-required";
return "response-auth-required";
case Reason::kCookieFlushed:
return "cookie-flushed";
return "cookie-removed";
case Reason::kDisableForRenderFrameHostCalled:
return DisabledReasonsToString(disabled_reasons_,
/*for_not_restored_reasons=*/true);
Expand Down
50 changes: 26 additions & 24 deletions third_party/blink/common/scheduler/web_scheduler_tracked_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,35 @@ FeatureNames FeatureToNames(WebSchedulerTrackedFeature feature) {
case WebSchedulerTrackedFeature::kWebTransportSticky:
return {"webtransport", "WebTransport used"};
case WebSchedulerTrackedFeature::kWebRTC:
return {"webrtc", "WebRTC live connection"};
return {"rtc", "WebRTC live connection"};
case WebSchedulerTrackedFeature::kWebRTCSticky:
return {"webrtc", "WebRTC used"};
return {"rtc", "WebRTC used"};
case WebSchedulerTrackedFeature::kMainResourceHasCacheControlNoCache:
return {"cache-control-no-cache",
return {"response-cache-control-no-cache",
"main resource has Cache-Control: No-Cache"};
case WebSchedulerTrackedFeature::kMainResourceHasCacheControlNoStore:
return {"cache-control-no-store",
return {"response-cache-control-no-store",
"main resource has Cache-Control: No-Store"};
case WebSchedulerTrackedFeature::kSubresourceHasCacheControlNoCache:
return {"cache-control-no-cache",
return {"response-cache-control-no-cache",
"subresource has Cache-Control: No-Cache"};
case WebSchedulerTrackedFeature::kSubresourceHasCacheControlNoStore:
return {"cache-control-no-store",
return {"response-cache-control-no-store",
"subresource has Cache-Control: No-Store"};
case WebSchedulerTrackedFeature::kContainsPlugins:
return {"contains-plugins", "page contains plugins"};
return {"plugins", "page contains plugins"};
case WebSchedulerTrackedFeature::kDocumentLoaded:
return {"document-loaded", "document loaded"};
case WebSchedulerTrackedFeature::kSharedWorker:
return {"shared-worker", "Shared worker present"};
return {"sharedworker", "Shared worker present"};
case WebSchedulerTrackedFeature::kOutstandingNetworkRequestFetch:
return {"fetch", "outstanding network request (fetch)"};
case WebSchedulerTrackedFeature::kOutstandingNetworkRequestXHR:
return {"outstanding-network", "outstanding network request (XHR)"};
return {"outstanding-network-request",
"outstanding network request (XHR)"};
case WebSchedulerTrackedFeature::kOutstandingNetworkRequestOthers:
return {"outstanding-network", "outstanding network request (others)"};
return {"outstanding-network-request",
"outstanding network request (others)"};
case WebSchedulerTrackedFeature::kRequestedMIDIPermission:
return {"midi", "requested midi permission"};
case WebSchedulerTrackedFeature::kRequestedAudioCapturePermission:
Expand All @@ -71,57 +73,57 @@ FeatureNames FeatureToNames(WebSchedulerTrackedFeature feature) {
case WebSchedulerTrackedFeature::kBroadcastChannel:
return {"broadcastchannel", "requested broadcast channel permission"};
case WebSchedulerTrackedFeature::kWebXR:
return {"webxr", "WebXR"};
return {"webxrdevice", "WebXR"};
case WebSchedulerTrackedFeature::kWebLocks:
return {"lock", "WebLocks"};
case WebSchedulerTrackedFeature::kWebHID:
return {"webhid", "WebHID"};
case WebSchedulerTrackedFeature::kWebShare:
return {"webshare", "WebShare"};
case WebSchedulerTrackedFeature::kRequestedStorageAccessGrant:
return {"storage-access", "requested storage access permission"};
return {"storageaccess", "requested storage access permission"};
case WebSchedulerTrackedFeature::kWebNfc:
return {"webnfc", "WebNfc"};
case WebSchedulerTrackedFeature::kPrinting:
return {"printing", "Printing"};
case WebSchedulerTrackedFeature::kWebDatabase:
return {"web-database", "WebDatabase"};
case WebSchedulerTrackedFeature::kPictureInPicture:
return {"picture-in-picture", "PictureInPicture"};
return {"pictureinpicturewindow", "PictureInPicture"};
case WebSchedulerTrackedFeature::kSpeechRecognizer:
return {"speech-recognizer", "SpeechRecognizer"};
return {"speechrecognition", "SpeechRecognizer"};
case WebSchedulerTrackedFeature::kIdleManager:
return {"idle-manager", "IdleManager"};
return {"idledetector", "IdleManager"};
case WebSchedulerTrackedFeature::kPaymentManager:
return {"payment-manager", "PaymentManager"};
return {"paymentrequest", "PaymentManager"};
case WebSchedulerTrackedFeature::kKeyboardLock:
return {"keyboard-lock", "KeyboardLock"};
return {"keyboardlock", "KeyboardLock"};
case WebSchedulerTrackedFeature::kWebOTPService:
return {"sms", "SMSService"};
return {"otpcredential", "SMSService"};
case WebSchedulerTrackedFeature::kOutstandingNetworkRequestDirectSocket:
return {"outstanding-network",
return {"outstanding-network-request",
"outstanding network request (direct socket)"};
case WebSchedulerTrackedFeature::kInjectedJavascript:
return {"injected-javascript", "External javascript injected"};
case WebSchedulerTrackedFeature::kInjectedStyleSheet:
return {"injected-stylesheet", "External systesheet injected"};
case WebSchedulerTrackedFeature::kKeepaliveRequest:
return {"keepalive-request", "requests with keepalive set"};
return {"response-keep-alive", "requests with keepalive set"};
case WebSchedulerTrackedFeature::kDummy:
return {"Dummy", "Dummy for testing"};
case WebSchedulerTrackedFeature::
kJsNetworkRequestReceivedCacheControlNoStoreResource:
return {"cache-control-no-store",
return {"response-cache-control-no-store",
"JavaScript network request received Cache-Control: no-store "
"resource"};
case WebSchedulerTrackedFeature::kIndexedDBEvent:
return {"indexed-db-event", "IndexedDB event is pending"};
return {"idbversionchangeevent", "IndexedDB event is pending"};
case WebSchedulerTrackedFeature::kWebSerial:
return {"webserial", "Serial port open"};
case WebSchedulerTrackedFeature::kSmartCard:
return {"smart-card", "SmartCardContext used"};
return {"smartcardconnection", "SmartCardContext used"};
case WebSchedulerTrackedFeature::kLiveMediaStreamTrack:
return {"media-stream", "page has live MediaStreamTrack"};
return {"mediastream", "page has live MediaStreamTrack"};
case WebSchedulerTrackedFeature::kUnloadHandler:
return {"unload-handler", "page contains unload handler"};
case WebSchedulerTrackedFeature::kParserAborted:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ promise_test(async t => {

// Check the BFCache result and the reported reasons.
await assertBFCacheEligibility(rc1, /*shouldRestoreFromBFCache=*/ false);
await assertNotRestoredFromBFCache(rc1, ['navigation-failure']);
await assertNotRestoredFromBFCache(rc1, ['response-status-not-ok']);
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ promise_test(async t => {
await openWebRTC(rc1);
// The page should not be eligible for BFCache because of open WebRTC connection and live MediaStreamTrack.
await assertBFCacheEligibility(rc1, /*shouldRestoreFromBFCache=*/ false);
await assertNotRestoredFromBFCache(rc1, ['webrtc', 'media-stream']);
await assertNotRestoredFromBFCache(rc1, ['rtc', 'mediastream']);
});

0 comments on commit b0bf2c1

Please sign in to comment.