Allow cached responses for LoFi placeholder reloading

Update the placeholder image replacement mechanism to not forcibly bypass the
cache. This was implemented for server LoFi feature, and no longer needed.

The design doc has more explanation as well.
https://docs.google.com/document/d/1jF1eSOhqTEt0L1WBCccGwH9chxLd9d1Ez0zo11obj14/edit#heading=h.v3bc3q4jzwm4

Bug: 846170
Change-Id: I6cbbdc81528984bbd9c15da3e498e277a8c2cfca
Reviewed-on: https://chromium-review.googlesource.com/1166600Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583136}

third_party/blink/renderer/core/loader/resource/image_resource.cc

@@ -573,8 +573,6 @@ void ImageResource::ReloadIfLoFiOrPlaceholderImage(
   DCHECK(!is_scheduling_reload_);
   is_scheduling_reload_ = true;
 
-  SetCachePolicyBypassingCache();
-
   // The reloaded image should not use any previews transformations.
   WebURLRequest::PreviewsState previews_state_for_reload =
       WebURLRequest::kPreviewsNoTransform;

third_party/blink/renderer/core/loader/resource/image_resource_test.cc

@@ -195,7 +195,6 @@ void TestThatReloadIsStartedThenServeReload(
     ImageResource* image_resource,
     ImageResourceContent* content,
     MockImageResourceObserver* observer,
-    mojom::FetchCacheMode cache_mode_for_reload,
     bool placeholder_before_reload) {
   const char* data = reinterpret_cast<const char*>(kJpegImage2);
   constexpr size_t kDataLength = sizeof(kJpegImage2);
@@ -209,8 +208,6 @@ void TestThatReloadIsStartedThenServeReload(
   EXPECT_EQ(placeholder_before_reload, image_resource->ShouldShowPlaceholder());
   EXPECT_EQ(g_null_atom,
             image_resource->GetResourceRequest().HttpHeaderField("range"));
-  EXPECT_EQ(cache_mode_for_reload,
-            image_resource->GetResourceRequest().GetCacheMode());
   EXPECT_EQ(content, image_resource->GetContent());
   EXPECT_FALSE(content->HasImage());
 
@@ -672,9 +669,9 @@ TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderAfterFinished) {
                                                  Resource::kReloadAlways);
 
   EXPECT_EQ(3, observer->ImageChangedCount());
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST_P(ImageResourceReloadTest,
@@ -718,9 +715,9 @@ TEST_P(ImageResourceReloadTest,
                                                  Resource::kReloadAlways);
 
   EXPECT_EQ(3, observer->ImageChangedCount());
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST_P(ImageResourceReloadTest,
@@ -802,9 +799,8 @@ TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderViaResourceFetcher) {
 
   EXPECT_EQ(3, observer->ImageChangedCount());
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, content, observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource, content,
+                                         observer.get(), false);
 
   GetMemoryCache()->Remove(image_resource);
 }
@@ -836,9 +832,9 @@ TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderBeforeResponse) {
   // image is still loading.
   EXPECT_FALSE(observer->ImageNotifyFinishedCalled());
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderDuringResponse) {
@@ -887,9 +883,9 @@ TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderDuringResponse) {
   // image is still loading.
   EXPECT_FALSE(observer->ImageNotifyFinishedCalled());
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
@@ -911,9 +907,9 @@ TEST_P(ImageResourceReloadTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
   image_resource->ReloadIfLoFiOrPlaceholderImage(fetcher,
                                                  Resource::kReloadAlways);
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST_P(ImageResourceReloadTest, ReloadLoFiImagesWithDuplicateURLs) {
@@ -1461,9 +1457,9 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessful) {
   EXPECT_EQ(2, observer->ImageChangedCount());
   EXPECT_FALSE(image_resource->ShouldShowPlaceholder());
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, false);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), false);
 }
 
 TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessfulClientLoFi) {
@@ -1502,9 +1498,9 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessfulClientLoFi) {
   EXPECT_FALSE(observer->ImageNotifyFinishedCalled());
   EXPECT_EQ(2, observer->ImageChangedCount());
 
-  TestThatReloadIsStartedThenServeReload(
-      test_url, image_resource, image_resource->GetContent(), observer.get(),
-      mojom::FetchCacheMode::kBypassCache, true);
+  TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                         image_resource->GetContent(),
+                                         observer.get(), true);
 
   EXPECT_FALSE(image_resource->GetContent()->GetImage()->IsBitmapImage());
   EXPECT_TRUE(image_resource->ShouldShowPlaceholder());
@@ -1572,7 +1568,7 @@ TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {
 
     TestThatReloadIsStartedThenServeReload(
         test_url, image_resource, image_resource->GetContent(), observer.get(),
-        mojom::FetchCacheMode::kBypassCache, test.placeholder_before_reload);
+        test.placeholder_before_reload);
 
     EXPECT_EQ(test.expected_reload_previews_state,
               image_resource->GetResourceRequest().GetPreviewsState());
@@ -1798,9 +1794,9 @@ TEST(ImageResourceTest,
 
     // The dimensions could not be extracted, and the response code was a 4xx
     // error, so the full original image should be loading.
-    TestThatReloadIsStartedThenServeReload(
-        test_url, image_resource, image_resource->GetContent(), observer.get(),
-        mojom::FetchCacheMode::kBypassCache, false);
+    TestThatReloadIsStartedThenServeReload(test_url, image_resource,
+                                           image_resource->GetContent(),
+                                           observer.get(), false);
   }
 }
 

third_party/blink/renderer/platform/loader/fetch/resource.cc

@@ -977,10 +977,6 @@ String Resource::GetMemoryDumpName() const {
       identifier_);
 }
 
-void Resource::SetCachePolicyBypassingCache() {
-  resource_request_.SetCacheMode(mojom::FetchCacheMode::kBypassCache);
-}
-
 void Resource::SetPreviewsState(WebURLRequest::PreviewsState previews_state) {
   resource_request_.SetPreviewsState(previews_state);
 }

third_party/blink/renderer/platform/loader/fetch/resource.h

@@ -436,7 +436,6 @@ class PLATFORM_EXPORT Resource : public GarbageCollectedFinalized<Resource>,
     return clients_;
   }
 
-  void SetCachePolicyBypassingCache();
   void SetPreviewsState(WebURLRequest::PreviewsState);
   void ClearRangeRequestHeader();