Add holdback_status_override to PrerenderAttributes
This allows code in //chrome (e.g., for DSE) to override the holdback
status. In this CL, it is always left as kUnspecified (except in the new
tests). In future CLs, we will set it to kHoldback, when the new DSE
holdback feature is enabled.
Bug: 1464173
Change-Id: I15dbc23da831447ed2fdb6ff2d6abd4b9a64fe97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4682681
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1171527}
diff --git a/content/browser/preloading/prerender/prerender_host_registry.cc b/content/browser/preloading/prerender/prerender_host_registry.cc
index 14e69b1..567ee4a9 100644
--- a/content/browser/preloading/prerender/prerender_host_registry.cc
+++ b/content/browser/preloading/prerender/prerender_host_registry.cc
@@ -31,6 +31,7 @@
#include "content/browser/renderer_host/render_frame_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/common/frame.mojom.h"
+#include "content/public/browser/preloading.h"
#include "content/public/browser/preloading_data.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/visibility.h"
@@ -320,7 +321,7 @@
PrerenderFinalStatus status);
void RejectDueToHoldback();
- void SetHoldback(bool holdback);
+ void SetHoldbackOverride(PreloadingHoldbackStatus status);
bool CheckIfShouldHoldback();
// Public only for exceptional case.
@@ -413,15 +414,12 @@
Drop();
}
-void PrerenderHostBuilder::SetHoldback(bool holdback) {
+void PrerenderHostBuilder::SetHoldbackOverride(
+ PreloadingHoldbackStatus status) {
if (!attempt_) {
return;
}
- if (holdback) {
- attempt_->SetHoldbackStatus(PreloadingHoldbackStatus::kHoldback);
- } else {
- attempt_->SetHoldbackStatus(PreloadingHoldbackStatus::kAllowed);
- }
+ attempt_->SetHoldbackStatus(status);
}
void PrerenderHostBuilder::RejectAsFailure(
@@ -640,17 +638,28 @@
if (attempt)
attempt->SetEligibility(PreloadingEligibility::kEligible);
- // Check for the HoldbackStatus after checking the eligibility. Override
- // preloading holdbacks rules when DevTools is opened to mitigate the cases
- // in which developers are affected by holdbacks.
- bool should_holdbacks_be_overridden =
+ // Normally CheckIfShouldHoldback() computes the holdback status based on
+ // PreloadingConfig. In special cases, we call SetHoldbackOverride() to
+ // override that processing.
+ bool has_devtools_open =
initiator_rfh &&
RenderFrameDevToolsAgentHost::GetFor(initiator_rfh) != nullptr;
- if (should_holdbacks_be_overridden) {
- builder.SetHoldback(false);
+
+ if (has_devtools_open) {
+ // Never holdback when DevTools is opened, to avoid web developer
+ // frustration.
+ builder.SetHoldbackOverride(PreloadingHoldbackStatus::kAllowed);
} else if (base::FeatureList::IsEnabled(features::kPrerender2Holdback)) {
- builder.SetHoldback(true);
+ // The Prerender2Holdback feature overrides the PreloadingConfig logic.
+ // TODO(https://crbug.com/1464173): remove this.
+ builder.SetHoldbackOverride(PreloadingHoldbackStatus::kHoldback);
+ } else if (attributes.holdback_status_override !=
+ PreloadingHoldbackStatus::kUnspecified) {
+ // The caller (e.g. from chrome/) is allowed to specify a holdback that
+ // overrides the default logic.
+ builder.SetHoldbackOverride(attributes.holdback_status_override);
}
+
// Check if the attempt is held back either due to the check above or via
// PreloadingConfig.
if (builder.CheckIfShouldHoldback()) {
