0

Add holdback feature parameter for measuring the impact of removing

debug strings

Bug: 399783247
Change-Id: I00de7344b3d1b8324dd2c1612daab56867f6e991
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6332182
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Jiacheng Guo <gjc@google.com>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1430115}
This commit is contained in:
Jiacheng Guo
2025-03-10 00:13:00 -07:00
committed by Chromium LUCI CQ
parent c6aabd4931
commit d954fba11f
4 changed files with 37 additions and 2 deletions

@ -4610,6 +4610,7 @@ void NavigationRequest::SelectFrameHostForOnResponseStarted(
<< "`render_frame_host_` should not be set before the "
"`NavigationRequest` starts to select the RFH.";
ScopedCrashKeys crash_keys(*this);
std::string rfh_selected_reason;
// Select an appropriate renderer to commit the navigation.
if (IsServedFromBackForwardCache()) {
@ -4639,12 +4640,19 @@ void NavigationRequest::SelectFrameHostForOnResponseStarted(
prerender_frame_tree_node_id_.value())
->GetSafeRef();
} else if (response_should_be_rendered_) {
std::string* reason_output =
base::FeatureList::IsEnabled(
features::kHoldbackDebugReasonStringRemoval)
? &rfh_selected_reason
: nullptr;
if (auto result =
frame_tree_node_->render_manager()->GetFrameHostForNavigation(
this, &browsing_context_group_swap_,
ProcessAllocationContext::CreateForNavigationRequest(
ProcessAllocationNavigationStage::kAfterResponse,
navigation_id_));
navigation_id_),
reason_output);
result.has_value()) {
render_frame_host_ = result.value()->GetSafeRef();
} else {
@ -4865,6 +4873,16 @@ void NavigationRequest::SelectFrameHostForOnResponseStarted(
return;
}
// TODO(crbug.com/399783247): Remove
if (base::FeatureList::IsEnabled(
features::kHoldbackDebugReasonStringRemoval)) {
SCOPED_CRASH_KEY_STRING256(
"Bug1454273", "base_host_for_data_url",
common_params_->base_url_for_data_url.host_piece());
SCOPED_CRASH_KEY_STRING1024("Bug1454273", "rfh_selected_reason",
rfh_selected_reason);
}
if (HasRenderFrameHost() &&
!CheckPermissionsPoliciesForFencedFrames(GetOriginToCommit().value())) {
OnRequestFailedInternal(

@ -1730,9 +1730,19 @@ RenderFrameHostManager::GetFrameHostForNavigation(
render_frame_host_->IsNavigationSameSite(request->GetUrlInfo());
IsSameSiteGetter is_same_site_getter(is_same_site);
std::string site_instance_reason;
std::string* reason_output =
base::FeatureList::IsEnabled(features::kHoldbackDebugReasonStringRemoval)
? &site_instance_reason
: reason;
scoped_refptr<SiteInstanceImpl> dest_site_instance =
GetSiteInstanceForNavigationRequest(request, is_same_site_getter,
browsing_context_group_swap, reason);
browsing_context_group_swap,
reason_output);
if (base::FeatureList::IsEnabled(
features::kHoldbackDebugReasonStringRemoval)) {
reason->append(site_instance_reason);
}
// A subframe should always be in the same BrowsingInstance as the parent
// (see also https://crbug.com/1107269).

@ -65,6 +65,12 @@ BASE_FEATURE(kHidePastePopupOnGSB,
base::FEATURE_ENABLED_BY_DEFAULT);
#endif
// Holdback the removal of debug reason strings in crrev.com/c/6312375
// to measure the impact.
BASE_FEATURE(kHoldbackDebugReasonStringRemoval,
"HoldbackDebugReasonStringRemoval",
base::FEATURE_DISABLED_BY_DEFAULT);
// If Canvas2D Image Chromium is allowed, this feature controls whether it is
// enabled.
BASE_FEATURE(kCanvas2DImageChromium,

@ -61,6 +61,7 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kGroupNIKByJoiningOrigin);
#if BUILDFLAG(IS_ANDROID)
CONTENT_EXPORT BASE_DECLARE_FEATURE(kHidePastePopupOnGSB);
#endif
CONTENT_EXPORT BASE_DECLARE_FEATURE(kHoldbackDebugReasonStringRemoval);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kInMemoryCodeCache);
CONTENT_EXPORT BASE_DECLARE_FEATURE(kInterestGroupUpdateIfOlderThan);