ImageLoader: Ensure to re-enqueue a request if the referrer policy changes.

This patch ensures that we proceed with re-enqueing a request if the
referrer policy has changed.

R=chrishtr@chromium.org

Bug: 884505
Change-Id: I515f3185e783e356311b2a4a84a6898132fb6baa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1641426Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665986}

third_party/blink/renderer/core/BUILD.gn

@@ -2037,6 +2037,7 @@ jumbo_source_set("unit_tests") {
     "loader/frame_fetch_context_test.cc",
     "loader/frame_resource_fetcher_properties_test.cc",
     "loader/idleness_detector_test.cc",
+    "loader/image_loader_test.cc",
     "loader/interactive_detector_test.cc",
     "loader/link_loader_test.cc",
     "loader/long_task_detector_test.cc",

third_party/blink/renderer/core/html/html_image_element.cc

@@ -448,8 +448,8 @@ Node::InsertionNotificationRequest HTMLImageElement::InsertedInto(
     }
   }
 
-  if (image_was_modified ||
-      GetImageLoader().ShouldUpdateOnInsertedInto(insertion_point)) {
+  if (image_was_modified || GetImageLoader().ShouldUpdateOnInsertedInto(
+                                insertion_point, referrer_policy_)) {
     GetImageLoader().UpdateFromElement(ImageLoader::kUpdateNormal,
                                        referrer_policy_);
   }

third_party/blink/renderer/core/loader/image_loader.cc

@@ -343,7 +343,8 @@ void ImageLoader::SetImageForTest(ImageResourceContent* new_image) {
 }
 
 bool ImageLoader::ShouldUpdateOnInsertedInto(
-    ContainerNode& insertion_point) const {
+    ContainerNode& insertion_point,
+    network::mojom::ReferrerPolicy referrer_policy) const {
   // If we're being inserted into a disconnected tree, we don't need to update.
   if (!insertion_point.isConnected())
     return false;
@@ -355,10 +356,15 @@ bool ImageLoader::ShouldUpdateOnInsertedInto(
   if (element_->GetDocument().ValidBaseElementURL() != last_base_element_url_)
     return true;
 
-  // Finally, try to update if we're idle (that is, we have neither the image
-  // contents nor any activity). This could be an indication that we skipped a
-  // previous load when inserted into an inactive document.
-  return !image_content_ && !HasPendingActivity();
+  // If we already have image content, then we don't need an update.
+  if (image_content_)
+    return false;
+
+  // Finally, try to update if we're idle. This could be an indication that we
+  // skipped a previous load when inserted into an inactive document. Note that
+  // if we're not idle, we should also update our referrer policy if it has
+  // changed.
+  return !HasPendingActivity() || referrer_policy != last_referrer_policy_;
 }
 
 void ImageLoader::ClearImage() {
@@ -659,6 +665,7 @@ void ImageLoader::UpdateFromElement(
   suppress_error_events_ = (update_behavior == kUpdateSizeChanged);
   last_base_element_url_ =
       element_->GetDocument().ValidBaseElementURL().GetString();
+  last_referrer_policy_ = referrer_policy;
 
   if (update_behavior == kUpdateIgnorePreviousError)
     ClearFailedLoadURL();

third_party/blink/renderer/core/loader/image_loader.h

@@ -90,7 +90,10 @@ class CORE_EXPORT ImageLoader : public GarbageCollectedFinalized<ImageLoader>,
 
   // Returns true if this loader should be updated via UpdateFromElement() when
   // being inserted into a new parent; returns false otherwise.
-  bool ShouldUpdateOnInsertedInto(ContainerNode& insertion_point) const;
+  bool ShouldUpdateOnInsertedInto(
+      ContainerNode& insertion_point,
+      network::mojom::ReferrerPolicy referrer_policy =
+          network::mojom::ReferrerPolicy::kDefault) const;
 
   // Cancels pending load events, and doesn't dispatch new ones.
   // Note: ClearImage/SetImage.*() are not a simple setter.
@@ -208,6 +211,8 @@ class CORE_EXPORT ImageLoader : public GarbageCollectedFinalized<ImageLoader>,
   Member<ImageResource> image_resource_for_image_document_;
 
   String last_base_element_url_;
+  network::mojom::ReferrerPolicy last_referrer_policy_ =
+      network::mojom::ReferrerPolicy::kDefault;
   AtomicString failed_load_url_;
   base::WeakPtr<Task> pending_task_;  // owned by Microtask
   std::unique_ptr<IncrementLoadEventDelayCount>

third_party/blink/renderer/core/loader/image_loader_test.cc

+// Copyright 2019 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 "third_party/blink/renderer/core/loader/image_loader.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/core/html/html_image_loader.h"
+#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
+
+namespace blink {
+
+class ImageLoaderTest : public RenderingTest {};
+
+TEST_F(ImageLoaderTest, ReferrerPolicyChangeCausesUpdateOnInsert) {
+  SetHtmlInnerHTML(R"HTML(
+    <img id="test" src="test.png">
+  )HTML");
+
+  auto* element = GetDocument().getElementById("test");
+  ASSERT_TRUE(element);
+
+  auto* loader = MakeGarbageCollected<HTMLImageLoader>(element);
+  ASSERT_TRUE(loader);
+
+  // We should already be collected, so UpdateFromElement() would cause some
+  // pending activity.
+  loader->UpdateFromElement();
+  ASSERT_TRUE(loader->HasPendingActivity());
+
+  // We don't need an update, since we're already loading an image.
+  EXPECT_FALSE(loader->ShouldUpdateOnInsertedInto(*element));
+
+  // However, if the referrer policy changes, then we should need an update.
+  EXPECT_TRUE(loader->ShouldUpdateOnInsertedInto(
+      *element, network::mojom::ReferrerPolicy::kNever));
+
+  // Changing referrer policy.
+  loader->UpdateFromElement(ImageLoader::kUpdateNormal,
+                            network::mojom::ReferrerPolicy::kNever);
+
+  // Now, we don't need an update with the latest referrer policy.
+  EXPECT_FALSE(loader->ShouldUpdateOnInsertedInto(
+      *element, network::mojom::ReferrerPolicy::kNever));
+
+  // But we do want an update if the referrer policy changes back to what it was
+  // before.
+  EXPECT_TRUE(loader->ShouldUpdateOnInsertedInto(
+      *element, network::mojom::ReferrerPolicy::kDefault));
+}
+
+}  // namespace blink