Revert "Refactor subresource redirect renderer code"

This reverts commit 674a1af2.

Reason for revert: suspected cause of crbug.com/1146687

Original change's description:
> Refactor subresource redirect renderer code
>
> This CL refactors the existing subresource redirect code to make the
> robots rules and login based image compression implementation easy.
>
> * Separate out the url loader throttle that does redirect and timeout
> handling from the public image hints decider logic. The robots decider can
> be subclassed and used.
>
> * Make public image hints agent as renderframeobserver. This reduces
> cluttter in ResourceLoadingHintsAgent. The upcoming robots checker agent
> could also observe renderframeobserver directly.
>
> No change in behavior is expected.
>
> Change-Id: Ic555a4402016a645b8820fa87dd5157c7112fb71
> Bug: 1144836
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514068
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Reviewed-by: Michael Crouse <mcrouse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825115}

TBR=rajendrant@chromium.org,mcrouse@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1144836, 1146687
Change-Id: I483882b29ac342d188137690e7ba9f8cbedcd6ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527859Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825966}

chrome/renderer/BUILD.gn

@@ -95,8 +95,6 @@ static_library("renderer") {
     "plugins/plugin_uma.h",
     "previews/resource_loading_hints_agent.cc",
     "previews/resource_loading_hints_agent.h",
-    "subresource_redirect/public_image_hints_url_loader_throttle.cc",
-    "subresource_redirect/public_image_hints_url_loader_throttle.h",
     "subresource_redirect/subresource_redirect_hints_agent.cc",
     "subresource_redirect/subresource_redirect_hints_agent.h",
     "subresource_redirect/subresource_redirect_params.cc",

chrome/renderer/chrome_content_renderer_client.cc

@@ -60,8 +60,6 @@
 #include "chrome/renderer/plugins/pdf_plugin_placeholder.h"
 #include "chrome/renderer/plugins/plugin_uma.h"
 #include "chrome/renderer/previews/resource_loading_hints_agent.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_params.h"
 #include "chrome/renderer/sync_encryption_keys_extension.h"
 #include "chrome/renderer/url_loader_throttle_provider_impl.h"
 #include "chrome/renderer/v8_unwinder.h"
@@ -603,10 +601,8 @@ void ChromeContentRendererClient::RenderFrameCreated(
   if (lite_video::IsLiteVideoEnabled())
     new lite_video::LiteVideoHintAgent(render_frame);
 
-  new previews::ResourceLoadingHintsAgent(associated_interfaces, render_frame);
-
-  if (subresource_redirect::IsPublicImageHintsBasedCompressionEnabled())
-    new subresource_redirect::SubresourceRedirectHintsAgent(render_frame);
+  new previews::ResourceLoadingHintsAgent(
+      render_frame_observer->associated_interfaces(), render_frame);
 
   if (translate::IsSubFrameTranslationEnabled()) {
     new translate::PerFrameTranslateAgent(

chrome/renderer/previews/resource_loading_hints_agent.cc

@@ -8,7 +8,6 @@
 
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/histogram_macros_local.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
 #include "content/public/renderer/render_frame.h"
 #include "third_party/blink/public/platform/web_loading_hints_provider.h"
 #include "third_party/blink/public/platform/web_string.h"
@@ -36,7 +35,9 @@ const blink::WebVector<blink::WebString> convert_to_web_vector(
 ResourceLoadingHintsAgent::ResourceLoadingHintsAgent(
     blink::AssociatedInterfaceRegistry* associated_interfaces,
     content::RenderFrame* render_frame)
-    : content::RenderFrameObserver(render_frame) {
+    : content::RenderFrameObserver(render_frame),
+      content::RenderFrameObserverTracker<ResourceLoadingHintsAgent>(
+          render_frame) {
   DCHECK(render_frame);
   associated_interfaces->AddInterface(base::BindRepeating(
       &ResourceLoadingHintsAgent::SetReceiver, base::Unretained(this)));
@@ -46,6 +47,23 @@ GURL ResourceLoadingHintsAgent::GetDocumentURL() const {
   return render_frame()->GetWebFrame()->GetDocument().Url();
 }
 
+void ResourceLoadingHintsAgent::DidStartNavigation(
+    const GURL& url,
+    base::Optional<blink::WebNavigationType> navigation_type) {
+  if (!IsMainFrame())
+    return;
+  subresource_redirect_hints_agent_.DidStartNavigation();
+}
+
+void ResourceLoadingHintsAgent::ReadyToCommitNavigation(
+    blink::WebDocumentLoader* document_loader) {
+  if (!IsMainFrame())
+    return;
+
+  subresource_redirect_hints_agent_.ReadyToCommitNavigation(
+      render_frame()->GetRoutingID());
+}
+
 void ResourceLoadingHintsAgent::DidCreateNewDocument() {
   if (!IsMainFrame())
     return;
@@ -126,12 +144,20 @@ void ResourceLoadingHintsAgent::SetResourceLoadingHints(
 
 void ResourceLoadingHintsAgent::SetCompressPublicImagesHints(
     blink::mojom::CompressPublicImagesHintsPtr images_hints) {
-  if (auto* subresource_redirect_hints_agent =
-          subresource_redirect::SubresourceRedirectHintsAgent::Get(
-              render_frame())) {
-    subresource_redirect_hints_agent->SetCompressPublicImagesHints(
-        std::move(images_hints));
+  if (!IsMainFrame())
+    return;
+  subresource_redirect_hints_agent_.SetCompressPublicImagesHints(
+      std::move(images_hints));
+}
+
+void ResourceLoadingHintsAgent::NotifyHttpsImageCompressionFetchFailed(
+    base::TimeDelta retry_after) {
+  if (!subresource_redirect_service_remote_) {
+    render_frame()->GetRemoteAssociatedInterfaces()->GetInterface(
+        &subresource_redirect_service_remote_);
   }
+  subresource_redirect_service_remote_->NotifyCompressedImageFetchFailed(
+      retry_after);
 }
 
 void ResourceLoadingHintsAgent::SetLiteVideoHint(

chrome/renderer/previews/resource_loading_hints_agent.h

@@ -10,8 +10,11 @@
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/optional.h"
+#include "chrome/common/subresource_redirect_service.mojom.h"
 #include "chrome/renderer/lite_video/lite_video_hint_agent.h"
+#include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
 #include "content/public/renderer/render_frame_observer.h"
+#include "content/public/renderer/render_frame_observer_tracker.h"
 #include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
@@ -29,15 +32,29 @@ namespace previews {
 class ResourceLoadingHintsAgent
     : public content::RenderFrameObserver,
       public blink::mojom::PreviewsResourceLoadingHintsReceiver,
-      public base::SupportsWeakPtr<ResourceLoadingHintsAgent> {
+      public base::SupportsWeakPtr<ResourceLoadingHintsAgent>,
+      public content::RenderFrameObserverTracker<ResourceLoadingHintsAgent> {
  public:
   ResourceLoadingHintsAgent(
       blink::AssociatedInterfaceRegistry* associated_interfaces,
       content::RenderFrame* render_frame);
   ~ResourceLoadingHintsAgent() override;
 
+  subresource_redirect::SubresourceRedirectHintsAgent&
+  subresource_redirect_hints_agent() {
+    return subresource_redirect_hints_agent_;
+  }
+
+  // Notifies the browser process that https image compression fetch had failed.
+  void NotifyHttpsImageCompressionFetchFailed(base::TimeDelta retry_after);
+
  private:
   // content::RenderFrameObserver:
+  void DidStartNavigation(
+      const GURL& url,
+      base::Optional<blink::WebNavigationType> navigation_type) override;
+  void ReadyToCommitNavigation(
+      blink::WebDocumentLoader* document_loader) override;
   void DidCreateNewDocument() override;
   void OnDestruct() override;
 
@@ -66,6 +83,13 @@ class ResourceLoadingHintsAgent
   mojo::AssociatedReceiver<blink::mojom::PreviewsResourceLoadingHintsReceiver>
       receiver_{this};
 
+  mojo::AssociatedRemote<
+      subresource_redirect::mojom::SubresourceRedirectService>
+      subresource_redirect_service_remote_;
+
+  subresource_redirect::SubresourceRedirectHintsAgent
+      subresource_redirect_hints_agent_;
+
   blink::mojom::BlinkOptimizationGuideHintsPtr blink_optimization_guide_hints_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceLoadingHintsAgent);

chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle.cc

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle.h"
-
-#include "base/memory/ptr_util.h"
-#include "base/metrics/field_trial_params.h"
-#include "base/metrics/histogram_macros.h"
-#include "base/time/time.h"
-#include "chrome/renderer/previews/resource_loading_hints_agent.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_params.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_util.h"
-#include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
-#include "content/public/renderer/render_frame.h"
-#include "net/base/escape.h"
-#include "net/base/load_flags.h"
-#include "net/http/http_status_code.h"
-#include "net/http/http_util.h"
-#include "services/network/public/mojom/fetch_api.mojom-shared.h"
-#include "services/network/public/mojom/url_response_head.mojom.h"
-#include "third_party/blink/public/common/features.h"
-#include "third_party/blink/public/common/loader/previews_state.h"
-#include "third_party/blink/public/platform/web_network_state_notifier.h"
-#include "third_party/blink/public/platform/web_url.h"
-#include "third_party/blink/public/platform/web_url_request.h"
-
-namespace subresource_redirect {
-
-PublicImageHintsURLLoaderThrottle::PublicImageHintsURLLoaderThrottle(
-    int render_frame_id,
-    bool allowed_to_redirect)
-    : SubresourceRedirectURLLoaderThrottle(render_frame_id) {
-  redirect_result_ =
-      allowed_to_redirect
-          ? SubresourceRedirectHintsAgent::RedirectResult::kRedirectable
-          : SubresourceRedirectHintsAgent::RedirectResult::
-                kIneligibleOtherImage;
-}
-
-PublicImageHintsURLLoaderThrottle::~PublicImageHintsURLLoaderThrottle() =
-    default;
-
-SubresourceRedirectHintsAgent*
-PublicImageHintsURLLoaderThrottle::GetSubresourceRedirectHintsAgent() {
-  return subresource_redirect::SubresourceRedirectHintsAgent::Get(
-      GetRenderFrame());
-}
-
-bool PublicImageHintsURLLoaderThrottle::ShouldRedirectImage(const GURL& url) {
-  if (redirect_result_ !=
-      SubresourceRedirectHintsAgent::RedirectResult::kRedirectable) {
-    return false;
-  }
-
-  auto* subresource_redirect_hints_agent = GetSubresourceRedirectHintsAgent();
-  if (!subresource_redirect_hints_agent)
-    return false;
-
-  redirect_result_ = subresource_redirect_hints_agent->ShouldRedirectImage(url);
-  if (redirect_result_ !=
-      SubresourceRedirectHintsAgent::RedirectResult::kRedirectable) {
-    return false;
-  }
-  return true;
-}
-
-void PublicImageHintsURLLoaderThrottle::OnRedirectedLoadCompleteWithError() {
-  redirect_result_ =
-      SubresourceRedirectHintsAgent::RedirectResult::kIneligibleOtherImage;
-}
-
-void PublicImageHintsURLLoaderThrottle::RecordMetricsOnLoadFinished(
-    const GURL& url,
-    int64_t content_length) {
-  auto* subresource_redirect_hints_agent = GetSubresourceRedirectHintsAgent();
-  if (subresource_redirect_hints_agent) {
-    subresource_redirect_hints_agent->RecordMetricsOnLoadFinished(
-        url, content_length, redirect_result_);
-  }
-}
-
-}  // namespace subresource_redirect

chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle.h

-// Copyright 2020 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef CHROME_RENDERER_SUBRESOURCE_REDIRECT_PUBLIC_IMAGE_HINTS_URL_LOADER_THROTTLE_H_
-#define CHROME_RENDERER_SUBRESOURCE_REDIRECT_PUBLIC_IMAGE_HINTS_URL_LOADER_THROTTLE_H_
-
-#include "base/macros.h"
-#include "base/timer/timer.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle.h"
-#include "third_party/blink/public/common/loader/url_loader_throttle.h"
-
-namespace subresource_redirect {
-
-// This class handles internal redirects for public subresouces on HTTPS sites
-// to compressed versions of subresources.
-class PublicImageHintsURLLoaderThrottle
-    : public SubresourceRedirectURLLoaderThrottle {
- public:
-  PublicImageHintsURLLoaderThrottle(int render_frame_id,
-                                    bool allowed_to_redirect);
-
-  ~PublicImageHintsURLLoaderThrottle() override;
-
-  // SubresourceRedirectURLLoaderThrottle:
-  bool ShouldRedirectImage(const GURL& url) override;
-  void OnRedirectedLoadCompleteWithError() override;
-  void RecordMetricsOnLoadFinished(const GURL& url,
-                                   int64_t content_length) override;
-
- private:
-  friend class TestPublicImageHintsURLLoaderThrottle;
-
-  SubresourceRedirectHintsAgent* GetSubresourceRedirectHintsAgent();
-
-  // Whether the subresource can be redirected or not and what was the reason if
-  // its not eligible.
-  SubresourceRedirectHintsAgent::RedirectResult redirect_result_;
-};
-
-}  // namespace subresource_redirect
-
-#endif  // CHROME_RENDERER_SUBRESOURCE_REDIRECT_PUBLIC_IMAGE_HINTS_URL_LOADER_THROTTLE_H_

chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.cc

@@ -3,39 +3,36 @@
 // found in the LICENSE file.
 
 #include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
-
 #include "base/metrics/field_trial_params.h"
-#include "chrome/renderer/subresource_redirect/subresource_redirect_params.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "services/metrics/public/cpp/metrics_utils.h"
 #include "services/metrics/public/cpp/mojo_ukm_recorder.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 
 namespace subresource_redirect {
 
-SubresourceRedirectHintsAgent::SubresourceRedirectHintsAgent(
-    content::RenderFrame* render_frame)
-    : content::RenderFrameObserver(render_frame),
-      content::RenderFrameObserverTracker<SubresourceRedirectHintsAgent>(
-          render_frame) {
-  DCHECK(render_frame);
-  DCHECK(IsPublicImageHintsBasedCompressionEnabled());
-}
+namespace {
 
-SubresourceRedirectHintsAgent::~SubresourceRedirectHintsAgent() = default;
+// Default timeout for the hints to be received from the time navigation starts.
+const int64_t kHintsReceiveDefaultTimeoutSeconds = 5;
 
-bool SubresourceRedirectHintsAgent::IsMainFrame() const {
-  return render_frame()->IsMainFrame();
+// Returns the hinte receive timeout value from field trial.
+int64_t GetHintsReceiveTimeout() {
+  return base::GetFieldTrialParamByFeatureAsInt(
+      blink::features::kSubresourceRedirect, "hints_receive_timeout",
+      kHintsReceiveDefaultTimeoutSeconds);
 }
 
-void SubresourceRedirectHintsAgent::DidStartNavigation(
-    const GURL& url,
-    base::Optional<blink::WebNavigationType> navigation_type) {
-  if (!IsMainFrame())
-    return;
+}  // namespace
+
+SubresourceRedirectHintsAgent::SubresourceRedirectHintsAgent() = default;
+SubresourceRedirectHintsAgent::~SubresourceRedirectHintsAgent() = default;
+
+void SubresourceRedirectHintsAgent::DidStartNavigation() {
   // Clear the hints when a navigation starts, so that hints from previous
   // navigation do not apply in case the same renderframe is reused.
   public_image_urls_.clear();
@@ -43,9 +40,7 @@ void SubresourceRedirectHintsAgent::DidStartNavigation(
 }
 
 void SubresourceRedirectHintsAgent::ReadyToCommitNavigation(
-    blink::WebDocumentLoader* document_loader) {
-  if (!IsMainFrame())
-    return;
+    int render_frame_id) {
   // Its ok to use base::Unretained(this) here since the timer object is owned
   // by |this|, and the timer and its callback will get deleted when |this| is
   // destroyed.
@@ -53,16 +48,11 @@ void SubresourceRedirectHintsAgent::ReadyToCommitNavigation(
       FROM_HERE, base::TimeDelta::FromSeconds(GetHintsReceiveTimeout()),
       base::BindOnce(&SubresourceRedirectHintsAgent::OnHintsReceiveTimeout,
                      base::Unretained(this)));
-}
-
-void SubresourceRedirectHintsAgent::OnDestruct() {
-  delete this;
+  render_frame_id_ = render_frame_id;
 }
 
 void SubresourceRedirectHintsAgent::SetCompressPublicImagesHints(
     blink::mojom::CompressPublicImagesHintsPtr images_hints) {
-  if (!IsMainFrame())
-    return;
   DCHECK(public_image_urls_.empty());
   DCHECK(!public_image_urls_received_);
   public_image_urls_ = images_hints->image_urls;
@@ -109,12 +99,14 @@ void SubresourceRedirectHintsAgent::ClearImageHints() {
 void SubresourceRedirectHintsAgent::RecordMetrics(
     int64_t content_length,
     RedirectResult redirect_result) const {
-  if (!render_frame() || !render_frame()->GetWebFrame())
+  content::RenderFrame* render_frame =
+      content::RenderFrame::FromRoutingID(render_frame_id_);
+  if (!render_frame || !render_frame->GetWebFrame())
     return;
 
   ukm::builders::PublicImageCompressionDataUse
       public_image_compression_data_use(
-          render_frame()->GetWebFrame()->GetDocument().GetUkmSourceId());
+          render_frame->GetWebFrame()->GetDocument().GetUkmSourceId());
   content_length = ukm::GetExponentialBucketMin(content_length, 1.3);
 
   switch (redirect_result) {
@@ -174,15 +166,4 @@ void SubresourceRedirectHintsAgent::RecordImageHintsUnavailableMetrics() {
   unavailable_image_hints_urls_.clear();
 }
 
-void SubresourceRedirectHintsAgent::NotifyHttpsImageCompressionFetchFailed(
-    base::TimeDelta retry_after) {
-  if (!subresource_redirect_service_remote_) {
-    render_frame()->GetRemoteAssociatedInterfaces()->GetInterface(
-        &subresource_redirect_service_remote_);
-  }
-  subresource_redirect_service_remote_->NotifyCompressedImageFetchFailed(
-      retry_after);
-  ClearImageHints();
-}
-
 }  // namespace subresource_redirect

chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h

@@ -7,24 +7,16 @@
 
 #include "base/containers/flat_set.h"
 #include "base/macros.h"
-#include "base/optional.h"
 #include "base/timer/timer.h"
 #include "chrome/common/subresource_redirect_service.mojom.h"
-#include "content/public/renderer/render_frame_observer.h"
-#include "content/public/renderer/render_frame_observer_tracker.h"
-#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
-#include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h"
 #include "third_party/blink/public/mojom/loader/previews_resource_loading_hints.mojom.h"
 #include "url/gurl.h"
 
 namespace subresource_redirect {
 
 // Holds the public image URL hints to be queried by URL loader throttles. Only
-// redirects public images for mainframes.
-class SubresourceRedirectHintsAgent
-    : public content::RenderFrameObserver,
-      public content::RenderFrameObserverTracker<
-          SubresourceRedirectHintsAgent> {
+// created for mainframes.
+class SubresourceRedirectHintsAgent {
  public:
   enum class RedirectResult {
     // The image was found in the image hints and is eligible to be redirected
@@ -55,13 +47,21 @@ class SubresourceRedirectHintsAgent
     kIneligibleOtherImage
   };
 
-  explicit SubresourceRedirectHintsAgent(content::RenderFrame* render_frame);
-  ~SubresourceRedirectHintsAgent() override;
+  SubresourceRedirectHintsAgent();
+  ~SubresourceRedirectHintsAgent();
 
   SubresourceRedirectHintsAgent(const SubresourceRedirectHintsAgent&) = delete;
   SubresourceRedirectHintsAgent& operator=(
       const SubresourceRedirectHintsAgent&) = delete;
 
+  // Called when a navigation starts to clear the state from previous
+  // navigation.
+  void DidStartNavigation();
+  void ReadyToCommitNavigation(int render_frame_id);
+
+  void SetCompressPublicImagesHints(
+      blink::mojom::CompressPublicImagesHintsPtr images_hints);
+
   RedirectResult ShouldRedirectImage(const GURL& url) const;
 
   // Record metrics when the resource load is finished.
@@ -69,28 +69,11 @@ class SubresourceRedirectHintsAgent
                                    int64_t content_length,
                                    RedirectResult redirect_result);
 
-  void SetCompressPublicImagesHints(
-      blink::mojom::CompressPublicImagesHintsPtr images_hints);
-
-  // Notifies the browser process that https image compression fetch had failed.
-  void NotifyHttpsImageCompressionFetchFailed(base::TimeDelta retry_after);
-
   // Clears the image hint urls.
   void ClearImageHints();
 
  private:
-  // content::RenderFrameObserver:
-  void DidStartNavigation(
-      const GURL& url,
-      base::Optional<blink::WebNavigationType> navigation_type) override;
-  void ReadyToCommitNavigation(
-      blink::WebDocumentLoader* document_loader) override;
-  void OnDestruct() override;
-
-  bool IsMainFrame() const;
-
   void OnHintsReceiveTimeout();
-
   void RecordMetrics(int64_t content_length,
                      RedirectResult redirect_result) const;
 
@@ -111,9 +94,9 @@ class SubresourceRedirectHintsAgent
   // until hints are received or it times out and used to record metrics.
   base::flat_set<std::pair<std::string, int64_t>> unavailable_image_hints_urls_;
 
-  mojo::AssociatedRemote<
-      subresource_redirect::mojom::SubresourceRedirectService>
-      subresource_redirect_service_remote_;
+  // ID of the current render frame (will be main frame). Populated when the
+  // navigation commits. Used to record ukm against.
+  int render_frame_id_;
 };
 
 }  // namespace subresource_redirect

chrome/renderer/subresource_redirect/subresource_redirect_params.cc

@@ -12,16 +12,6 @@
 
 namespace subresource_redirect {
 
-namespace {
-
-// Default timeout for the hints to be received from the time navigation starts.
-const int64_t kHintsReceiveDefaultTimeoutSeconds = 5;
-
-bool IsSubresourceRedirectEnabled() {
-  return base::FeatureList::IsEnabled(blink::features::kSubresourceRedirect);
-}
-}  // namespace
-
 url::Origin GetSubresourceRedirectOrigin() {
   auto lite_page_subresource_origin = base::GetFieldTrialParamValueByFeature(
       blink::features::kSubresourceRedirect, "lite_page_subresource_origin");
@@ -30,30 +20,4 @@ url::Origin GetSubresourceRedirectOrigin() {
   return url::Origin::Create(GURL(lite_page_subresource_origin));
 }
 
-bool IsPublicImageHintsBasedCompressionEnabled() {
-  return IsSubresourceRedirectEnabled() &&
-         base::GetFieldTrialParamByFeatureAsBool(
-             blink::features::kSubresourceRedirect,
-             "enable_public_image_hints_based_compression", true);
-}
-
-bool ShouldCompressionServerRedirectSubresource() {
-  return base::GetFieldTrialParamByFeatureAsBool(
-      blink::features::kSubresourceRedirect,
-      "enable_subresource_server_redirect", true);
-}
-
-base::TimeDelta GetCompressionRedirectTimeout() {
-  return base::TimeDelta::FromMilliseconds(
-      base::GetFieldTrialParamByFeatureAsInt(
-          blink::features::kSubresourceRedirect, "subresource_redirect_timeout",
-          5000));
-}
-
-int64_t GetHintsReceiveTimeout() {
-  return base::GetFieldTrialParamByFeatureAsInt(
-      blink::features::kSubresourceRedirect, "hints_receive_timeout",
-      kHintsReceiveDefaultTimeoutSeconds);
-}
-
 }  // namespace subresource_redirect

chrome/renderer/subresource_redirect/subresource_redirect_params.h

@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "base/time/time.h"
 #include "url/origin.h"
 
 namespace subresource_redirect {
@@ -16,21 +15,6 @@ namespace subresource_redirect {
 // default.
 url::Origin GetSubresourceRedirectOrigin();
 
-// Returns if the public image hints based subresource compression is enabled.
-bool IsPublicImageHintsBasedCompressionEnabled();
-
-// Should the subresource be redirected to its compressed version. This returns
-// false if only coverage metrics need to be recorded and actual redirection
-// should not happen.
-bool ShouldCompressionServerRedirectSubresource();
-
-// Returns the timeout for the compressed subresource redirect, after which the
-// subresource should be fetched directly from the origin.
-base::TimeDelta GetCompressionRedirectTimeout();
-
-// Returns the public image hinte receive timeout value from field trial.
-int64_t GetHintsReceiveTimeout();
-
 }  // namespace subresource_redirect
 
 #endif  // CHROME_RENDERER_SUBRESOURCE_REDIRECT_SUBRESOURCE_REDIRECT_PARAMS_H_

chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle.cc

@@ -9,7 +9,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "chrome/renderer/previews/resource_loading_hints_agent.h"
-#include "chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_params.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_util.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h"
@@ -20,6 +19,7 @@
 #include "net/http/http_util.h"
 #include "services/network/public/mojom/fetch_api.mojom-shared.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
+#include "third_party/blink/public/common/features.h"
 #include "third_party/blink/public/common/loader/previews_state.h"
 #include "third_party/blink/public/platform/web_network_state_notifier.h"
 #include "third_party/blink/public/platform/web_url.h"
@@ -37,6 +37,22 @@ bool IsCompressionServerOrigin(const GURL& url) {
          (url.scheme() == compression_server.scheme());
 }
 
+// Should the subresource be redirected to its compressed version. This returns
+// false if only coverage metrics need to be recorded and actual redirection
+// should not happen.
+bool ShouldCompressionServerRedirectSubresource() {
+  return base::GetFieldTrialParamByFeatureAsBool(
+      blink::features::kSubresourceRedirect,
+      "enable_subresource_server_redirect", true);
+}
+
+base::TimeDelta GetCompressionRedirectTimeout() {
+  return base::TimeDelta::FromMilliseconds(
+      base::GetFieldTrialParamByFeatureAsInt(
+          blink::features::kSubresourceRedirect, "subresource_redirect_timeout",
+          5000));
+}
+
 }  // namespace
 
 // static
@@ -44,7 +60,7 @@ std::unique_ptr<SubresourceRedirectURLLoaderThrottle>
 SubresourceRedirectURLLoaderThrottle::MaybeCreateThrottle(
     const blink::WebURLRequest& request,
     int render_frame_id) {
-  if (IsPublicImageHintsBasedCompressionEnabled() &&
+  if (base::FeatureList::IsEnabled(blink::features::kSubresourceRedirect) &&
       request.GetRequestDestination() ==
           network::mojom::RequestDestination::kImage &&
       request.Url().ProtocolIs(url::kHttpsScheme) &&
@@ -52,7 +68,7 @@ SubresourceRedirectURLLoaderThrottle::MaybeCreateThrottle(
       request.GetRequestContext() !=
           blink::mojom::RequestContextType::FAVICON) {
     return base::WrapUnique<SubresourceRedirectURLLoaderThrottle>(
-        new PublicImageHintsURLLoaderThrottle(
+        new SubresourceRedirectURLLoaderThrottle(
             render_frame_id, request.GetPreviewsState() &
                                  blink::PreviewsTypes::kSubresourceRedirectOn));
   }
@@ -60,8 +76,15 @@ SubresourceRedirectURLLoaderThrottle::MaybeCreateThrottle(
 }
 
 SubresourceRedirectURLLoaderThrottle::SubresourceRedirectURLLoaderThrottle(
-    int render_frame_id)
-    : render_frame_id_(render_frame_id) {}
+    int render_frame_id,
+    bool allowed_to_redirect)
+    : render_frame_id_(render_frame_id) {
+  redirect_result_ =
+      allowed_to_redirect
+          ? SubresourceRedirectHintsAgent::RedirectResult::kRedirectable
+          : SubresourceRedirectHintsAgent::RedirectResult::
+                kIneligibleOtherImage;
+}
 
 SubresourceRedirectURLLoaderThrottle::~SubresourceRedirectURLLoaderThrottle() =
     default;
@@ -69,17 +92,33 @@ SubresourceRedirectURLLoaderThrottle::~SubresourceRedirectURLLoaderThrottle() =
 void SubresourceRedirectURLLoaderThrottle::WillStartRequest(
     network::ResourceRequest* request,
     bool* defer) {
-  DCHECK(IsPublicImageHintsBasedCompressionEnabled());
+  DCHECK(base::FeatureList::IsEnabled(blink::features::kSubresourceRedirect));
   DCHECK_EQ(request->destination, network::mojom::RequestDestination::kImage);
+  DCHECK(
+      request->previews_state & blink::PreviewsTypes::SUBRESOURCE_REDIRECT_ON ||
+      redirect_result_ ==
+          SubresourceRedirectHintsAgent::RedirectResult::kIneligibleOtherImage);
   DCHECK(request->url.SchemeIs(url::kHttpsScheme));
 
   // Do not redirect if its already a litepage subresource.
   if (IsCompressionServerOrigin(request->url))
     return;
 
-  if (!ShouldRedirectImage(request->url))
+  if (redirect_result_ !=
+      SubresourceRedirectHintsAgent::RedirectResult::kRedirectable) {
+    return;
+  }
+
+  auto* subresource_redirect_hints_agent = GetSubresourceRedirectHintsAgent();
+  if (!subresource_redirect_hints_agent)
     return;
 
+  redirect_result_ =
+      subresource_redirect_hints_agent->ShouldRedirectImage(request->url);
+  if (redirect_result_ !=
+      SubresourceRedirectHintsAgent::RedirectResult::kRedirectable) {
+    return;
+  }
   if (!ShouldCompressionServerRedirectSubresource())
     return;
 
@@ -95,6 +134,24 @@ void SubresourceRedirectURLLoaderThrottle::WillStartRequest(
                      base::Unretained(this)));
 }
 
+previews::ResourceLoadingHintsAgent*
+SubresourceRedirectURLLoaderThrottle::GetResourceLoadingHintsAgent() {
+  // The ResourceLoadingHintsAgent is main-frame only.
+  if (content::RenderFrame* render_frame =
+          content::RenderFrame::FromRoutingID(render_frame_id_)) {
+    return previews::ResourceLoadingHintsAgent::Get(render_frame);
+  }
+  return nullptr;
+}
+
+SubresourceRedirectHintsAgent*
+SubresourceRedirectURLLoaderThrottle::GetSubresourceRedirectHintsAgent() {
+  if (auto* resource_loading_hints_agent = GetResourceLoadingHintsAgent()) {
+    return &resource_loading_hints_agent->subresource_redirect_hints_agent();
+  }
+  return nullptr;
+}
+
 void SubresourceRedirectURLLoaderThrottle::WillRedirectRequest(
     net::RedirectInfo* redirect_info,
     const network::mojom::URLResponseHead& response_head,
@@ -139,7 +196,8 @@ void SubresourceRedirectURLLoaderThrottle::BeforeWillProcessResponse(
       response_head.headers->response_code() == 304) {
     return;
   }
-  OnRedirectedLoadCompleteWithError();
+  redirect_result_ =
+      SubresourceRedirectHintsAgent::RedirectResult::kIneligibleOtherImage;
 
   // 503 response code indicates loadshed from the compression server. Notify
   // the browser process which will bypass subresource redirect for subsequent
@@ -153,9 +211,8 @@ void SubresourceRedirectURLLoaderThrottle::BeforeWillProcessResponse(
       net::HttpUtil::ParseRetryAfterHeader(retry_after_string,
                                            base::Time::Now(), &retry_after);
     }
-    if (auto* subresource_redirect_hints_agent =
-            SubresourceRedirectHintsAgent::Get(GetRenderFrame())) {
-      subresource_redirect_hints_agent->NotifyHttpsImageCompressionFetchFailed(
+    if (auto* resource_loading_hints_agent = GetResourceLoadingHintsAgent()) {
+      resource_loading_hints_agent->NotifyHttpsImageCompressionFetchFailed(
           retry_after);
     }
   }
@@ -179,7 +236,11 @@ void SubresourceRedirectURLLoaderThrottle::WillProcessResponse(
   if (content_length < 0)
     return;
 
-  RecordMetricsOnLoadFinished(response_url, content_length);
+  auto* subresource_redirect_hints_agent = GetSubresourceRedirectHintsAgent();
+  if (subresource_redirect_hints_agent) {
+    subresource_redirect_hints_agent->RecordMetricsOnLoadFinished(
+        response_url, content_length, redirect_result_);
+  }
 
   if (!did_redirect_compressed_origin_)
     return;
@@ -215,7 +276,8 @@ void SubresourceRedirectURLLoaderThrottle::WillOnCompleteWithError(
   if (!did_redirect_compressed_origin_)
     return;
   DCHECK(ShouldCompressionServerRedirectSubresource());
-  OnRedirectedLoadCompleteWithError();
+  redirect_result_ =
+      SubresourceRedirectHintsAgent::RedirectResult::kIneligibleOtherImage;
 
   // If the server fails, restart the request to the original resource, and
   // record it.
@@ -230,10 +292,11 @@ void SubresourceRedirectURLLoaderThrottle::OnRedirectTimeout() {
   DCHECK(did_redirect_compressed_origin_);
   did_redirect_compressed_origin_ = false;
   delegate_->RestartWithURLResetAndFlagsNow(net::LOAD_NORMAL);
-  if (auto* subresource_redirect_hints_agent =
-          SubresourceRedirectHintsAgent::Get(GetRenderFrame())) {
-    subresource_redirect_hints_agent->NotifyHttpsImageCompressionFetchFailed(
+  if (auto* resource_loading_hints_agent = GetResourceLoadingHintsAgent()) {
+    resource_loading_hints_agent->NotifyHttpsImageCompressionFetchFailed(
         base::TimeDelta());
+    resource_loading_hints_agent->subresource_redirect_hints_agent()
+        .ClearImageHints();
   }
   UMA_HISTOGRAM_BOOLEAN("SubresourceRedirect.CompressionFetchTimeout", true);
 }

chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle.h

@@ -7,27 +7,36 @@
 
 #include "base/macros.h"
 #include "base/timer/timer.h"
-#include "content/public/renderer/render_frame.h"
+#include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
 #include "third_party/blink/public/common/loader/url_loader_throttle.h"
 
 namespace blink {
 class WebURLRequest;
 }  // namespace blink
 
+namespace previews {
+class ResourceLoadingHintsAgent;
+}  // namespace previews
+
 namespace subresource_redirect {
 
-// This class handles internal redirects for HTTPS public subresources
-// (currently only for images) compressed versions of subresources. When the
-// redirect fails/timesout the original image is fetched directly. Subclasses
-// should implement the decider logic if an URL should be compressed.
+class SubresourceRedirectHintsAgent;
+
+// This class handles internal redirects for subresouces on HTTPS sites to
+// compressed versions of subresources.
 class SubresourceRedirectURLLoaderThrottle : public blink::URLLoaderThrottle {
  public:
   static std::unique_ptr<SubresourceRedirectURLLoaderThrottle>
-  MaybeCreateThrottle(const blink::WebURLRequest& request, int render_frame_id);
+  MaybeCreateThrottle(const blink::WebURLRequest& request,
+                      int render_frame_id);
 
-  explicit SubresourceRedirectURLLoaderThrottle(int render_frame_id);
   ~SubresourceRedirectURLLoaderThrottle() override;
 
+  previews::ResourceLoadingHintsAgent* GetResourceLoadingHintsAgent();
+
+  // virtual for testing.
+  virtual SubresourceRedirectHintsAgent* GetSubresourceRedirectHintsAgent();
+
   // blink::URLLoaderThrottle:
   void WillStartRequest(network::ResourceRequest* request,
                         bool* defer) override;
@@ -50,24 +59,11 @@ class SubresourceRedirectURLLoaderThrottle : public blink::URLLoaderThrottle {
   // Overridden to do nothing as the default implementation is NOT_REACHED()
   void DetachFromCurrentSequence() override;
 
-  // Return whether the image url should be redirected.
-  virtual bool ShouldRedirectImage(const GURL& url) = 0;
-
-  // Indicates the subresource redirect failed, and the image will be fetched
-  // directly from the  origin instead. The failures can be due to non-2xx http
-  // responses or other net errors
-  virtual void OnRedirectedLoadCompleteWithError() = 0;
-
-  // Notifies the image load finished.
-  virtual void RecordMetricsOnLoadFinished(const GURL& url,
-                                           int64_t content_length) = 0;
-
-  content::RenderFrame* GetRenderFrame() const {
-    return content::RenderFrame::FromRoutingID(render_frame_id_);
-  }
-
  private:
-  friend class TestPublicImageHintsURLLoaderThrottle;
+  friend class TestSubresourceRedirectURLLoaderThrottle;
+
+  SubresourceRedirectURLLoaderThrottle(int render_frame_id,
+                                       bool allowed_to_redirect);
 
   // Callback invoked when the redirect fetch times out.
   void OnRedirectTimeout();
@@ -75,6 +71,10 @@ class SubresourceRedirectURLLoaderThrottle : public blink::URLLoaderThrottle {
   // Render frame id to get the hints agent of the render frame.
   const int render_frame_id_;
 
+  // Whether the subresource can be redirected or not and what was the reason if
+  // its not eligible.
+  SubresourceRedirectHintsAgent::RedirectResult redirect_result_;
+
   // Whether this resource was actually redirected to compressed server origin.
   // This will be true when the redirect was attempted. Will be false when
   // redirect failed due to neterrors, or redirect was not attempted (but

chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle_browsertest.cc → chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle_unittest.cc

@@ -5,13 +5,9 @@
 #include "chrome/renderer/subresource_redirect/subresource_redirect_url_loader_throttle.h"
 
 #include "base/test/scoped_feature_list.h"
-#include "chrome/renderer/subresource_redirect/public_image_hints_url_loader_throttle.h"
+#include "base/test/task_environment.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h"
 #include "chrome/renderer/subresource_redirect/subresource_redirect_util.h"
-#include "chrome/test/base/chrome_render_view_test.h"
-#include "chrome/test/base/ui_test_utils.h"
-#include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_view.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/mojom/fetch_api.mojom-shared.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -25,59 +21,49 @@ namespace subresource_redirect {
 
 int kRenderFrameID = 1;
 
-namespace {
-
-class SubresourceRedirectPublicImageHintsURLLoaderThrottleTest
-    : public ChromeRenderViewTest {
+class TestSubresourceRedirectURLLoaderThrottle
+    : public SubresourceRedirectURLLoaderThrottle {
  public:
-  void DisableSubresourceRedirectFeature() {
-    scoped_feature_list_.Reset();
-    scoped_feature_list_.InitAndDisableFeature(
-        blink::features::kSubresourceRedirect);
-  }
-
-  void SetCompressPublicImagesHints(
-      const std::vector<std::string>& public_image_urls) {
-    subresource_redirect_hints_agent_->SetCompressPublicImagesHints(
+  TestSubresourceRedirectURLLoaderThrottle(
+      std::vector<std::string> public_image_urls,
+      bool allowed_to_redirect)
+      : SubresourceRedirectURLLoaderThrottle(kRenderFrameID,
+                                             allowed_to_redirect) {
+    subresource_redirect_hints_agent_.SetCompressPublicImagesHints(
         blink::mojom::CompressPublicImagesHints::New(public_image_urls));
   }
 
-  std::unique_ptr<PublicImageHintsURLLoaderThrottle>
-  CreatePublicImageHintsURLLoaderThrottle(
-      const GURL& url,
-      network::mojom::RequestDestination request_destination,
-      int previews_state) {
-    blink::WebURLRequest request;
-    request.SetUrl(url);
-    request.SetPreviewsState(previews_state);
-    request.SetRequestDestination(request_destination);
-    DCHECK(SubresourceRedirectURLLoaderThrottle::MaybeCreateThrottle(
-               request, view_->GetMainRenderFrame()->GetRoutingID())
-               .get() != nullptr);
-
-    return std::make_unique<PublicImageHintsURLLoaderThrottle>(
-        view_->GetMainRenderFrame()->GetRoutingID(),
-        previews_state & blink::PreviewsTypes::SUBRESOURCE_REDIRECT_ON);
-  }
-
- protected:
-  void SetUp() override {
-    ChromeRenderViewTest::SetUp();
-    scoped_feature_list_.InitWithFeaturesAndParameters(
-        {{blink::features::kSubresourceRedirect,
-          {{"enable_subresource_server_redirect", "true"}}}},
-        {});
-    subresource_redirect_hints_agent_ =
-        new SubresourceRedirectHintsAgent(view_->GetMainRenderFrame());
+  SubresourceRedirectHintsAgent* GetSubresourceRedirectHintsAgent() override {
+    return &subresource_redirect_hints_agent_;
   }
 
  private:
-  SubresourceRedirectHintsAgent* subresource_redirect_hints_agent_;
-  base::test::ScopedFeatureList scoped_feature_list_;
+  base::test::SingleThreadTaskEnvironment task_environment_;
+  SubresourceRedirectHintsAgent subresource_redirect_hints_agent_;
 };
 
-TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
-       TestMaybeCreateThrottle) {
+namespace {
+
+std::unique_ptr<SubresourceRedirectURLLoaderThrottle>
+CreateSubresourceRedirectURLLoaderThrottle(
+    const GURL& url,
+    network::mojom::RequestDestination request_destination,
+    int previews_state,
+    const std::vector<std::string>& public_image_urls) {
+  blink::WebURLRequest request;
+  request.SetUrl(url);
+  request.SetPreviewsState(previews_state);
+  request.SetRequestDestination(request_destination);
+  DCHECK(SubresourceRedirectURLLoaderThrottle::MaybeCreateThrottle(
+             request, kRenderFrameID)
+             .get() != nullptr);
+
+  return std::make_unique<TestSubresourceRedirectURLLoaderThrottle>(
+      public_image_urls,
+      previews_state & blink::PreviewsTypes::SUBRESOURCE_REDIRECT_ON);
+}
+
+TEST(SubresourceRedirectURLLoaderThrottleTest, TestMaybeCreateThrottle) {
   struct TestCase {
     bool data_saver_enabled;
     bool is_subresource_redirect_feature_enabled;
@@ -110,8 +96,15 @@ TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
   for (const TestCase& test_case : kTestCases) {
     blink::WebNetworkStateNotifier::SetSaveDataEnabled(
         test_case.data_saver_enabled);
-    if (!test_case.is_subresource_redirect_feature_enabled) {
-      DisableSubresourceRedirectFeature();
+    base::test::ScopedFeatureList scoped_feature_list;
+    if (test_case.is_subresource_redirect_feature_enabled) {
+      scoped_feature_list.InitWithFeaturesAndParameters(
+          {{blink::features::kSubresourceRedirect,
+            {{"enable_subresource_server_redirect", "true"}}}},
+          {});
+    } else {
+      scoped_feature_list.InitAndDisableFeature(
+          blink::features::kSubresourceRedirect);
     }
 
     blink::WebURLRequest request;
@@ -124,8 +117,7 @@ TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
   }
 }
 
-TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
-       TestGetSubresourceURL) {
+TEST(SubresourceRedirectURLLoaderThrottleTest, TestGetSubresourceURL) {
   struct TestCase {
     int previews_state;
     GURL original_url;
@@ -166,16 +158,19 @@ TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
       },
   };
   blink::WebNetworkStateNotifier::SetSaveDataEnabled(true);
-
-  SetCompressPublicImagesHints(
-      {"https://www.test.com/public_img.jpg",
-       "https://www.test.com/public_img.jpg#anchor",
-       "https://www.test.com/public_img.jpg?public_arg1=bar&public_arg2"});
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeaturesAndParameters(
+      {{blink::features::kSubresourceRedirect,
+        {{"enable_subresource_server_redirect", "true"}}}},
+      {});
 
   for (const TestCase& test_case : kTestCases) {
-    auto throttle = CreatePublicImageHintsURLLoaderThrottle(
+    auto throttle = CreateSubresourceRedirectURLLoaderThrottle(
         test_case.original_url, network::mojom::RequestDestination::kImage,
-        test_case.previews_state);
+        test_case.previews_state,
+        {"https://www.test.com/public_img.jpg",
+         "https://www.test.com/public_img.jpg#anchor",
+         "https://www.test.com/public_img.jpg?public_arg1=bar&public_arg2"});
     network::ResourceRequest request;
     request.url = test_case.original_url;
     request.destination = network::mojom::RequestDestination::kImage;
@@ -192,16 +187,19 @@ TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
   }
 }
 
-TEST_F(SubresourceRedirectPublicImageHintsURLLoaderThrottleTest,
-       DeferOverridenToFalse) {
+TEST(SubresourceRedirectURLLoaderThrottleTest, DeferOverridenToFalse) {
   blink::WebNetworkStateNotifier::SetSaveDataEnabled(true);
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitWithFeaturesAndParameters(
+      {{blink::features::kSubresourceRedirect,
+        {{"enable_subresource_server_redirect", "true"}}}},
+      {});
 
-  SetCompressPublicImagesHints({"https://www.test.com/test.jpg"});
-
-  auto throttle = CreatePublicImageHintsURLLoaderThrottle(
+  auto throttle = CreateSubresourceRedirectURLLoaderThrottle(
       GURL("https://www.test.com/test.jpg"),
       network::mojom::RequestDestination::kImage,
-      blink::PreviewsTypes::SUBRESOURCE_REDIRECT_ON);
+      blink::PreviewsTypes::SUBRESOURCE_REDIRECT_ON,
+      {"https://www.test.com/test.jpg"});
   network::ResourceRequest request;
   request.url = GURL("https://www.test.com/test.jpg");
   request.destination = network::mojom::RequestDestination::kImage;

chrome/test/BUILD.gn

@@ -1551,7 +1551,6 @@ if (!is_android) {
       "../renderer/chrome_content_settings_agent_delegate_browsertest.cc",
       "../renderer/chrome_render_frame_observer_browsertest.cc",
       "../renderer/lite_video/lite_video_hint_agent_browsertest.cc",
-      "../renderer/subresource_redirect/public_image_hints_url_loader_throttle_browsertest.cc",
       "../renderer/translate/per_frame_translate_agent_browsertest.cc",
       "../renderer/translate/translate_agent_browsertest.cc",
       "../renderer/translate/translate_script_browsertest.cc",
@@ -3779,6 +3778,7 @@ test("unit_tests") {
     "../renderer/media/flash_embed_rewrite_unittest.cc",
     "../renderer/net/net_error_helper_core_unittest.cc",
     "../renderer/plugins/plugin_uma_unittest.cc",
+    "../renderer/subresource_redirect/subresource_redirect_url_loader_throttle_unittest.cc",
     "../renderer/subresource_redirect/subresource_redirect_util_unittest.cc",
     "../renderer/v8_unwinder_unittest.cc",
     "../test/base/chrome_render_view_test.cc",