Refactor reloading tests in ImageResourceTest.cpp

third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp

Index: third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
diff --git a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
index 9d7b489b81e83aaaee0c3ea9af8b41e0377397be..88dd782d6c4735f4009884e6d0aecf56e3131ade 100644
--- a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
+++ b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
@@ -140,8 +140,6 @@ const unsigned char kJpegImage2[] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
     0x00, 0x00, 0x00, 0x00, 0x03, 0xff, 0xd9};
 
-constexpr int kJpegImage2Width = 50;
-
 const char kSvgImage[] =
     "<svg width=\"200\" height=\"200\" xmlns=\"http://www.w3.org/2000/svg\" "
     "xmlns:xlink=\"http://www.w3.org/1999/xlink\">"
@@ -168,6 +166,67 @@ void receiveResponse(ImageResource* imageResource,
   imageResource->finish();
 }
 
+void testThatReloadIsStartedAndDoReload(

[Nate Chapin, 2017/02/07 22:05:56]

This name is a little weird: I would expect that "Do"ing the reload would
include "Starting" it. Maybe testThatReloadIsStartedThenServeReload()?

[hiroshige, 2017/02/08 00:03:12]

Nice suggestion! Done.

+    const KURL& testURL,
+    ImageResource* imageResource,
+    ImageResourceContent* content,
+    MockImageResourceObserver* observer,
+    bool alreadyNotifiedFinish,
+    WebCachePolicy policyForReload = WebCachePolicy::BypassingCache) {
+  const char* data = reinterpret_cast<const char*>(kJpegImage2);
+  const size_t dataLength = sizeof(kJpegImage2);
+  const int imageWidth = 50;
+  const int imageHeight = 50;
+
+  // Checks that |imageResource| and |content| are ready for non-placeholder
+  // reloading.
+  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
+  EXPECT_FALSE(imageResource->resourceBuffer());
+  EXPECT_FALSE(imageResource->isPlaceholder());
+  EXPECT_EQ(nullAtom,
+            imageResource->resourceRequest().httpHeaderField("range"));
+  EXPECT_EQ(
+      static_cast<int>(policyForReload),
+      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
+  EXPECT_EQ(content, imageResource->getContent());
+  EXPECT_FALSE(content->hasImage());
+
+  // Checks |observer| before reloading.
+  const int originalImageChangedCount = observer->imageChangedCount();
+  EXPECT_EQ(alreadyNotifiedFinish, observer->imageNotifyFinishedCalled());
+  const int imageWidthOnImageNotifyFinished =
+      observer->imageWidthOnImageNotifyFinished();
+
+  // Does Reload.
+  imageResource->loader()->didReceiveResponse(WrappedResourceResponse(
+      ResourceResponse(testURL, "image/jpeg", dataLength, nullAtom, String())));
+  imageResource->loader()->didReceiveData(data, dataLength);
+  imageResource->loader()->didFinishLoading(0.0, dataLength, dataLength);
+
+  // Checks |imageResource|'s status after reloading.
+  EXPECT_EQ(ResourceStatus::Cached, imageResource->getStatus());
+  EXPECT_FALSE(imageResource->errorOccurred());
+  EXPECT_EQ(dataLength, imageResource->encodedSize());
+
+  // Checks |observer| after reloading that it is notified of updates/finish.
+  EXPECT_LT(originalImageChangedCount, observer->imageChangedCount());
+  EXPECT_EQ(imageWidth, observer->imageWidthOnLastImageChanged());
+  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
+  if (alreadyNotifiedFinish) {
+    EXPECT_EQ(imageWidthOnImageNotifyFinished,
+              observer->imageWidthOnImageNotifyFinished());
+  } else {
+    EXPECT_EQ(imageWidth, observer->imageWidthOnImageNotifyFinished());
+  }
+
+  // Checks |content| receives the correct image.
+  EXPECT_TRUE(content->hasImage());
+  EXPECT_FALSE(content->getImage()->isNull());
+  EXPECT_EQ(imageWidth, content->getImage()->width());
+  EXPECT_EQ(imageHeight, content->getImage()->height());
+  EXPECT_TRUE(content->getImage()->isBitmapImage());
+}
+
 class ImageResourceTestMockFetchContext : public FetchContext {
  public:
   static ImageResourceTestMockFetchContext* create() {
@@ -445,30 +504,11 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderAfterFinished) {
   // Call reloadIfLoFiOrPlaceholderImage() after the image has finished loading.
   imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
                                                 Resource::kReloadAlways);
-  EXPECT_FALSE(imageResource->errorOccurred());
-  EXPECT_FALSE(imageResource->resourceBuffer());
-  EXPECT_FALSE(imageResource->getContent()->hasImage());
   EXPECT_EQ(3, observer->imageChangedCount());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
 
-  imageResource->loader()->didReceiveResponse(
-      WrappedResourceResponse(resourceResponse), nullptr);
-  imageResource->loader()->didReceiveData(
-      reinterpret_cast<const char*>(kJpegImage2), sizeof(kJpegImage2));
-  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage2),
-                                            sizeof(kJpegImage2));
-  EXPECT_FALSE(imageResource->errorOccurred());
-  ASSERT_TRUE(imageResource->getContent()->hasImage());
-  EXPECT_FALSE(imageResource->getContent()->getImage()->isNull());
-  EXPECT_EQ(kJpegImage2Width, observer->imageWidthOnLastImageChanged());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-
-  // The observer should not have been notified of completion again.
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
-
-  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->width());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->height());
+  testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                     imageResource->getContent(),
+                                     observer.get(), true);
 }
 
 TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderViaResourceFetcher) {
@@ -504,30 +544,8 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderViaResourceFetcher) {
 
   fetcher->reloadLoFiImages();
 
-  EXPECT_FALSE(imageResource->errorOccurred());
-  EXPECT_FALSE(imageResource->resourceBuffer());
-  EXPECT_FALSE(imageResource->getContent()->hasImage());
-  EXPECT_EQ(3, observer->imageChangedCount());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-
-  imageResource->loader()->didReceiveResponse(
-      WrappedResourceResponse(resourceResponse), nullptr);
-  imageResource->loader()->didReceiveData(
-      reinterpret_cast<const char*>(kJpegImage2), sizeof(kJpegImage2));
-  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage2),
-                                            sizeof(kJpegImage2));
-  EXPECT_FALSE(imageResource->errorOccurred());
-  ASSERT_TRUE(imageResource->getContent()->hasImage());
-  EXPECT_FALSE(imageResource->getContent()->getImage()->isNull());
-  EXPECT_EQ(kJpegImage2Width, observer->imageWidthOnLastImageChanged());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-
-  // The observer should not have been notified of completion again.
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
-
-  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->width());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->height());
+  testThatReloadIsStartedAndDoReload(testURL, imageResource, content,
+                                     observer.get(), true);
 }
 
 TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
@@ -567,36 +585,16 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
   // Call reloadIfLoFiOrPlaceholderImage() while the image is still loading.
   imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
                                                 Resource::kReloadAlways);
-  EXPECT_FALSE(imageResource->errorOccurred());
-  EXPECT_FALSE(imageResource->resourceBuffer());
-  EXPECT_FALSE(imageResource->getContent()->hasImage());
+
   EXPECT_EQ(2, observer->imageChangedCount());
   EXPECT_EQ(0, observer->imageWidthOnLastImageChanged());
   // The observer should not have been notified of completion yet, since the
-  // image
-  // is still loading.
+  // image is still loading.
   EXPECT_FALSE(observer->imageNotifyFinishedCalled());
 
-  imageResource->loader()->didReceiveResponse(
-      WrappedResourceResponse(ResourceResponse(
-          testURL, "image/jpeg", sizeof(kJpegImage2), nullAtom, String())),
-      nullptr);
-  imageResource->loader()->didReceiveData(
-      reinterpret_cast<const char*>(kJpegImage2), sizeof(kJpegImage2));
-  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage2),
-                                            sizeof(kJpegImage2));
-
-  EXPECT_FALSE(imageResource->errorOccurred());
-  ASSERT_TRUE(imageResource->getContent()->hasImage());
-  EXPECT_FALSE(imageResource->getContent()->getImage()->isNull());
-  EXPECT_EQ(kJpegImage2Width, observer->imageWidthOnLastImageChanged());
-  // The observer should have been notified of completion only after the reload
-  // completed.
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-  EXPECT_EQ(kJpegImage2Width, observer->imageWidthOnImageNotifyFinished());
-  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->width());
-  EXPECT_EQ(50, imageResource->getContent()->getImage()->height());
+  testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                     imageResource->getContent(),
+                                     observer.get(), false);
 }
 
 TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
@@ -633,19 +631,14 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderForPlaceholder) {
 
   EXPECT_EQ(ResourceStatus::Cached, imageResource->getStatus());
   EXPECT_TRUE(imageResource->isPlaceholder());
+  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
 
   imageResource->reloadIfLoFiOrPlaceholderImage(fetcher,
                                                 Resource::kReloadAlways);
 
-  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_EQ(nullAtom,
-            imageResource->resourceRequest().httpHeaderField("range"));
-  EXPECT_EQ(
-      static_cast<int>(WebCachePolicy::BypassingCache),
-      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
-
-  imageResource->loader()->cancel();
+  testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                     imageResource->getContent(),
+                                     observer.get(), true);
 }
 
 TEST(ImageResourceTest, SVGImage) {
@@ -1185,36 +1178,12 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessful) {
 
   // The dimensions could not be extracted, so the full original image should be
   // loading.
-  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_EQ(nullAtom,
-            imageResource->resourceRequest().httpHeaderField("range"));
-  EXPECT_EQ(
-      static_cast<int>(WebCachePolicy::BypassingCache),
-      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
   EXPECT_FALSE(observer->imageNotifyFinishedCalled());
   EXPECT_EQ(3, observer->imageChangedCount());
 
-  imageResource->loader()->didReceiveResponse(
-      WrappedResourceResponse(ResourceResponse(
-          testURL, "image/jpeg", sizeof(kJpegImage), nullAtom, String())));
-  imageResource->loader()->didReceiveData(
-      reinterpret_cast<const char*>(kJpegImage), sizeof(kJpegImage));
-  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage),
-                                            sizeof(kJpegImage));
-
-  EXPECT_EQ(ResourceStatus::Cached, imageResource->getStatus());
-  EXPECT_EQ(sizeof(kJpegImage), imageResource->encodedSize());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_LT(3, observer->imageChangedCount());
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnLastImageChanged());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
-
-  ASSERT_TRUE(imageResource->getContent()->hasImage());
-  EXPECT_EQ(1, imageResource->getContent()->getImage()->width());
-  EXPECT_EQ(1, imageResource->getContent()->getImage()->height());

[hiroshige, 2017/02/06 23:10:34]

This test used kJpegImage as the reloaded image (and thus its width is 1 here),
while this CL changes this to kJpegImage2 (as
testThatReloadIsStartedAndDoReload() always uses kJpegImage2).
This is OK because this part of the test confirms that correct image is
(re)loaded, not depending on what image it is.

-  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
+  testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                     imageResource->getContent(),
+                                     observer.get(), false);
 }
 
 TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {
@@ -1255,38 +1224,11 @@ TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {
       0.0, kJpegImageSubrangeWithoutDimensionsLength,
       kJpegImageSubrangeWithoutDimensionsLength);
 
-  // Decode error didn't occur but the dimensions could not be extracted,
-  // so the full original image should be loading.
-  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_EQ(nullAtom,
-            imageResource->resourceRequest().httpHeaderField("range"));
-  EXPECT_EQ(
-      static_cast<int>(WebCachePolicy::BypassingCache),
-      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
-  EXPECT_FALSE(observer->imageNotifyFinishedCalled());
   EXPECT_EQ(2, observer->imageChangedCount());
 
-  imageResource->loader()->didReceiveResponse(
-      WrappedResourceResponse(ResourceResponse(
-          testURL, "image/jpeg", sizeof(kJpegImage), nullAtom, String())));
-  imageResource->loader()->didReceiveData(
-      reinterpret_cast<const char*>(kJpegImage), sizeof(kJpegImage));
-  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage),
-                                            sizeof(kJpegImage));
-
-  EXPECT_EQ(ResourceStatus::Cached, imageResource->getStatus());
-  EXPECT_EQ(sizeof(kJpegImage), imageResource->encodedSize());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_LT(0, observer->imageChangedCount());
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnLastImageChanged());
-  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-  EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
-
-  ASSERT_TRUE(imageResource->getContent()->hasImage());
-  EXPECT_EQ(1, imageResource->getContent()->getImage()->width());
-  EXPECT_EQ(1, imageResource->getContent()->getImage()->height());

[hiroshige, 2017/02/06 23:10:34]

ditto.

-  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
+  testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                     imageResource->getContent(),
+                                     observer.get(), false);
 }
 
 TEST(ImageResourceTest, FetchAllowPlaceholderThenDisallowPlaceholder) {
@@ -1306,16 +1248,10 @@ TEST(ImageResourceTest, FetchAllowPlaceholderThenDisallowPlaceholder) {
   ImageResource* secondImageResource =
       ImageResource::fetch(nonPlaceholderRequest, fetcher);
   EXPECT_EQ(imageResource, secondImageResource);
-  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_EQ(nullAtom,
-            imageResource->resourceRequest().httpHeaderField("range"));
-  EXPECT_EQ(
-      static_cast<int>(WebCachePolicy::UseProtocolCachePolicy),
-      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
-  EXPECT_FALSE(observer->imageNotifyFinishedCalled());
 
-  imageResource->loader()->cancel();
+  testThatReloadIsStartedAndDoReload(
+      testURL, imageResource, imageResource->getContent(), observer.get(),
+      false, WebCachePolicy::UseProtocolCachePolicy);
 }
 
 TEST(ImageResourceTest,
@@ -1359,15 +1295,10 @@ TEST(ImageResourceTest,
   ImageResource* secondImageResource =
       ImageResource::fetch(nonPlaceholderRequest, fetcher);
   EXPECT_EQ(imageResource, secondImageResource);
-  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-  EXPECT_FALSE(imageResource->isPlaceholder());
-  EXPECT_EQ(nullAtom,
-            imageResource->resourceRequest().httpHeaderField("range"));
-  EXPECT_EQ(
-      static_cast<int>(WebCachePolicy::UseProtocolCachePolicy),
-      static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
 
-  imageResource->loader()->cancel();
+  testThatReloadIsStartedAndDoReload(
+      testURL, imageResource, imageResource->getContent(), observer.get(), true,
+      WebCachePolicy::UseProtocolCachePolicy);
 }
 
 TEST(ImageResourceTest, FetchAllowPlaceholderFullResponseDecodeSuccess) {
@@ -1500,36 +1431,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.
-    EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
-    EXPECT_FALSE(imageResource->isPlaceholder());
-    EXPECT_EQ(nullAtom,
-              imageResource->resourceRequest().httpHeaderField("range"));
-    EXPECT_EQ(
-        static_cast<int>(WebCachePolicy::BypassingCache),
-        static_cast<int>(imageResource->resourceRequest().getCachePolicy()));
-    EXPECT_FALSE(observer->imageNotifyFinishedCalled());
-
-    imageResource->loader()->didReceiveResponse(
-
-        WrappedResourceResponse(ResourceResponse(
-            testURL, "image/jpeg", sizeof(kJpegImage), nullAtom, String())));
-    imageResource->loader()->didReceiveData(
-        reinterpret_cast<const char*>(kJpegImage), sizeof(kJpegImage));
-    imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage),
-                                              sizeof(kJpegImage));
-
-    EXPECT_EQ(ResourceStatus::Cached, imageResource->getStatus());
-    EXPECT_EQ(sizeof(kJpegImage), imageResource->encodedSize());
-    EXPECT_FALSE(imageResource->isPlaceholder());
-    EXPECT_LT(0, observer->imageChangedCount());
-    EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnLastImageChanged());
-    EXPECT_TRUE(observer->imageNotifyFinishedCalled());
-    EXPECT_EQ(kJpegImageWidth, observer->imageWidthOnImageNotifyFinished());
-
-    ASSERT_TRUE(imageResource->getContent()->hasImage());
-    EXPECT_EQ(1, imageResource->getContent()->getImage()->width());
-    EXPECT_EQ(1, imageResource->getContent()->getImage()->height());

[hiroshige, 2017/02/06 23:10:34]

ditto.

-    EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
+    testThatReloadIsStartedAndDoReload(testURL, imageResource,
+                                       imageResource->getContent(),
+                                       observer.get(), false);
   }
 }