[sheriff] Revert "Clear the subresource redirect hints when navigation is started"

This reverts commit 653f075d.

Reason for revert: Consistent failures on Linux:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests
The first failure:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Tests/87279

Original change's description:
> Clear the subresource redirect hints when navigation is started
> 
> RenderFrames could be reused for same-origin navigations. This causes
> the hints from previous navigations to be used in next navigation
> until the new hints fetch finishes and updates the hints. This CL clears
> the hints.
> 
> Unfortunately this causes race conditions between the image fetch and
> the hints update, which causes images to not redirected to compressed
> versions. So the image fetches are delayed a bit to circumvent this.
> 
> Bug: 1051283
> Change-Id: Ie9e5351e68c0081ab9dbfe23faed2c3c5e8e4d42
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2056207
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Reviewed-by: Michael Crouse <mcrouse@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#742326}

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

Change-Id: I74441c0600ab5fe5de3016c813590a5d3d21e447
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1051283
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063170Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742550}

chrome/browser/subresource_redirect/subresource_redirect_observer.cc

@@ -114,7 +114,7 @@ SubresourceRedirectObserver::SubresourceRedirectObserver(
 
 SubresourceRedirectObserver::~SubresourceRedirectObserver() = default;
 
-void SubresourceRedirectObserver::DidFinishNavigation(
+void SubresourceRedirectObserver::ReadyToCommitNavigation(
     content::NavigationHandle* navigation_handle) {
   DCHECK(navigation_handle);
   if (!navigation_handle->IsInMainFrame() ||

chrome/browser/subresource_redirect/subresource_redirect_observer.h

@@ -34,7 +34,7 @@ class SubresourceRedirectObserver
   explicit SubresourceRedirectObserver(content::WebContents* web_contents);
 
   // content::WebContentsObserver.
-  void DidFinishNavigation(
+  void ReadyToCommitNavigation(
       content::NavigationHandle* navigation_handle) override;
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();

chrome/renderer/previews/resource_loading_hints_agent.cc

@@ -48,16 +48,15 @@ GURL ResourceLoadingHintsAgent::GetDocumentURL() const {
   return render_frame()->GetWebFrame()->GetDocument().Url();
 }
 
-void ResourceLoadingHintsAgent::DidStartNavigation(
-    const GURL& url,
-    base::Optional<blink::WebNavigationType> navigation_type) {
-  subresource_redirect_hints_agent_.DidStartNavigation();
-}
-
 void ResourceLoadingHintsAgent::DidCreateNewDocument() {
   DCHECK(IsMainFrame());
+  did_create_new_document_ = true;
   if (!GetDocumentURL().SchemeIsHTTPOrHTTPS())
     return;
+  if (images_hints_) {
+    subresource_redirect_hints_agent_.SetCompressPublicImagesHints(
+        std::move(images_hints_));
+  }
 
   if (subresource_patterns_to_block_.empty())
     return;
@@ -111,8 +110,13 @@ void ResourceLoadingHintsAgent::SetResourceLoadingHints(
 
 void ResourceLoadingHintsAgent::SetCompressPublicImagesHints(
     blink::mojom::CompressPublicImagesHintsPtr images_hints) {
-  subresource_redirect_hints_agent_.SetCompressPublicImagesHints(
-      std::move(images_hints));
+  if (did_create_new_document_) {
+    subresource_redirect_hints_agent_.SetCompressPublicImagesHints(
+        std::move(images_hints));
+  } else {
+    // Let the hints be sent in DidCreateNewDocument.
+    images_hints_ = std::move(images_hints);
+  }
 }
 
 }  // namespace previews

chrome/renderer/previews/resource_loading_hints_agent.h

@@ -46,9 +46,6 @@ class ResourceLoadingHintsAgent
 
  private:
   // content::RenderFrameObserver:
-  void DidStartNavigation(
-      const GURL& url,
-      base::Optional<blink::WebNavigationType> navigation_type) override;
   void DidCreateNewDocument() override;
   void OnDestruct() override;
 
@@ -66,6 +63,8 @@ class ResourceLoadingHintsAgent
 
   bool IsMainFrame() const;
 
+  bool did_create_new_document_ = false;
+
   std::vector<std::string> subresource_patterns_to_block_;
   base::Optional<int64_t> ukm_source_id_;
 
@@ -74,6 +73,7 @@ class ResourceLoadingHintsAgent
 
   subresource_redirect::SubresourceRedirectHintsAgent
       subresource_redirect_hints_agent_;
+  blink::mojom::CompressPublicImagesHintsPtr images_hints_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceLoadingHintsAgent);
 };

chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.cc

@@ -16,17 +16,8 @@ namespace subresource_redirect {
 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();
-  public_image_urls_received_ = false;
-}
-
 void SubresourceRedirectHintsAgent::SetCompressPublicImagesHints(
     blink::mojom::CompressPublicImagesHintsPtr images_hints) {
-  DCHECK(public_image_urls_.empty());
-  DCHECK(!public_image_urls_received_);
   public_image_urls_ = images_hints->image_urls;
   public_image_urls_received_ = true;
 }

chrome/renderer/subresource_redirect/subresource_redirect_hints_agent.h

@@ -40,10 +40,6 @@ class SubresourceRedirectHintsAgent {
   SubresourceRedirectHintsAgent& operator=(
       const SubresourceRedirectHintsAgent&) = delete;
 
-  // Called when a navigation starts to clear the state from previous
-  // navigation.
-  void DidStartNavigation();
-
   void SetCompressPublicImagesHints(
       blink::mojom::CompressPublicImagesHintsPtr images_hints);
 

chrome/test/data/load_image/common.js

-// Copyright (c) 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.
-
-function checkImage() {
-  return imageLoadedPromise(document.images[0]);
-}
-
-function imageLoadedPromise(img_element) {
-  return new Promise((resolve, reject) => {
-    if (img_element.complete && img_element.src) {
-      console.log("image loaded callback", img_element.src);
-      resolve(true);
-    } else {
-      img_element.addEventListener('load', () => {
-        console.log("image loaded callback", img_element.src);
-        resolve(true);
-      });
-    }
-  });
-}
-
-function imageSrc() {
-  sendValueToTest(document.images[0].src);
-}
-
-function sendValueToTest(value) {
-  window.domAutomationController.send(value);
-}

chrome/test/data/load_image/fail_image.html

 <html>
 <head></head>
-<img alt="long_placeholder_text"/>
-<script src="common.js"></script>
+<img alt="long_placeholder_text" src="fail_image.png" />
 <script>
-window.onload = () => {
-  setTimeout(() => {
-    document.images[0].src = "fail_image.png"
-  }, 1000);
+function checkImage() {
+	sendValueToTest(document.images[0].complete);
+}
+
+function imageSrc() {
+	sendValueToTest(document.images[0].src);
+}
+
+function sendValueToTest(value) {
+  window.domAutomationController.send(value);
 }
 </script>
 </html>

chrome/test/data/load_image/image_delayed_load.html

-<html>
-<head></head>
-<img alt="long_placeholder_text"/>
-<script src="common.js"></script>
-<script>
-window.onload = () => {
-  setTimeout(() => {
-    document.images[0].src = "image.png"
-  }, 1000);
-}
-</script>
-</html>

chrome/test/data/load_image/image_js.html

 <html>
 <body>
 <div> div tag </div>
-<script src="common.js"></script>
 <script>
 	var img = document.createElement("img");
 	img.src="image.png";
 	document.getElementsByTagName('div')[0].appendChild(img);
+
+	function checkImage() {
+		sendValueToTest(document.images[0].complete);
+	}
+
+	function imageSrc() {
+		sendValueToTest(document.images[0].src);
+	}
+
+	function sendValueToTest(value) {
+	  window.domAutomationController.send(value);
+	}
 </script>
 </body>
-</html>
+</html>
\ No newline at end of file

chrome/test/data/load_image/image_with_fragment.html

 <html>
 <head></head>
-<img alt="long_placeholder_text"/>
-<script src="common.js"></script>
+<img alt="long_placeholder_text" src="image.png#fragment" />
 <script>
-window.onload = () => {
-  setTimeout(() => {
-    document.images[0].src = "image.png#fragment"
-  }, 1000)
+function checkImage() {
+	sendValueToTest(document.images[0].complete);
+}
+
+function imageSrc() {
+	sendValueToTest(document.images[0].src);
+}
+
+function sendValueToTest(value) {
+  window.domAutomationController.send(value);
 }
 </script>
 </html>

chrome/test/data/load_image/private_url_image.html

 <html>
 <body>
-<img alt="long_placeholder_text"/>
-<script src="common.js"></script>
+<img alt="long_placeholder_text" src="private_url_image.png" />
 <script>
-window.onload = () => {
-  setTimeout(() => {
-    document.images[0].src = "private_url_image.png"
-  }, 1000)
+function checkImage() {
+	sendValueToTest((document.images[0].complete && (document.images[0].naturalWidth > 0)));
+}
+
+function imageSrc() {
+	sendValueToTest(document.images[0].src);
+}
+
+function sendValueToTest(value) {
+  window.domAutomationController.send(value);
 }
 </script>
 </body>
-</html>
+</html>
\ No newline at end of file

chrome/test/data/load_image/two_images.html

 <html>
 <head></head>
-<img/>
-<img/>
-<script src="common.js"></script>
+<img src="image.png" />
+<img src="image.png?foo" />
 <script>
 function checkBothImagesLoaded() {
-  return new Promise((resolve, reject) => {
-    Promise.all([imageLoadedPromise(document.images[0]),
-      imageLoadedPromise(document.images[1])]).then(() => {
-        resolve(true);
-      });
-   });
+	sendValueToTest(document.images[0].complete && document.images[1].complete);
 }
 
-window.onload = () => {
-  setTimeout(() => {
-    document.images[0].src = "image.png"
-    document.images[1].src = "image.png?foo"
-  }, 1000)
+function imageSrc() {
+	sendValueToTest(document.images[0].src);
+}
+
+function sendValueToTest(value) {
+  window.domAutomationController.send(value);
 }
 </script>
 </html>