Add more unit tests for placeholder reloading

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 7c3e7dbd412112bcdf5ff11d458ec1053e1abb21..108968ff685ce0df714dc18c94ecf4f1e0f176f6 100644
--- a/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
+++ b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
@@ -94,6 +94,7 @@ constexpr int kJpegImageWidth = 1;
 
 constexpr size_t kJpegImageSubrangeWithDimensionsLength =
     sizeof(kJpegImage) - 1;
+constexpr size_t kJpegImageSubrangeWithoutDimensionsLength = 3;
 
 // Ensure that the image decoder can determine the dimensions of kJpegImage from
 // just the first kJpegImageSubrangeWithDimensionsLength bytes. If this test
@@ -470,6 +471,65 @@ TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderAfterFinished) {
   EXPECT_EQ(50, imageResource->getContent()->getImage()->height());
 }
 
+TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderViaResourceFetcher) {
+  ResourceFetcher* fetcher =
+      ResourceFetcher::create(ImageResourceTestMockFetchContext::create());
+
+  KURL testURL(ParsedURLString, "http://www.test.com/cancelTest2.html");
+  ScopedRegisteredURL scopedRegisteredURL(testURL);
+
+  ResourceRequest request = ResourceRequest(testURL);
+  request.setPreviewsState(WebURLRequest::ServerLoFiOn);
+  FetchRequest fetchRequest(request, FetchInitiatorInfo());
+  ImageResource* imageResource = ImageResource::fetch(fetchRequest, fetcher);
+  ImageResourceContent* content = imageResource->getContent();
+
+  std::unique_ptr<MockImageResourceObserver> observer =
+      MockImageResourceObserver::create(content);
+
+  // Send the image response.
+  ResourceResponse resourceResponse(KURL(), "image/jpeg", sizeof(kJpegImage),
+                                    nullAtom, String());
+  resourceResponse.addHTTPHeaderField("chrome-proxy-content-transform",
+                                      "empty-image");
+
+  imageResource->loader()->didReceiveResponse(
+      WrappedResourceResponse(resourceResponse));
+  imageResource->loader()->didReceiveData(
+      reinterpret_cast<const char*>(kJpegImage), sizeof(kJpegImage));
+  imageResource->loader()->didFinishLoading(0.0, sizeof(kJpegImage),
+                                            sizeof(kJpegImage));
+
+  EXPECT_EQ(imageResource, fetcher->cachedResource(testURL));
+
+  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());
+}
+
 TEST(ImageResourceTest, ReloadIfLoFiOrPlaceholderDuringFetch) {
   KURL testURL(ParsedURLString, "http://www.test.com/cancelTest.html");
   ScopedRegisteredURL scopedRegisteredURL(testURL);
@@ -905,6 +965,50 @@ TEST(ImageResourceTest, DecodeErrorWithEmptyBody) {
   EXPECT_FALSE(imageResource->isLoading());
 }
 
+// Testing DecodeError that occurs in didFinishLoading().
+// This is similar to DecodeErrorWithEmptyBody, but with non-empty body.
+TEST(ImageResourceTest, PartialContentWithoutDimensions) {
+  KURL testURL(ParsedURLString, "http://www.test.com/cancelTest.html");
+  ScopedRegisteredURL scopedRegisteredURL(testURL);
+
+  FetchRequest request(testURL, FetchInitiatorInfo());
+  ResourceFetcher* fetcher =
+      ResourceFetcher::create(ImageResourceTestMockFetchContext::create());
+  ImageResource* imageResource = ImageResource::fetch(request, fetcher);
+  std::unique_ptr<MockImageResourceObserver> observer =
+      MockImageResourceObserver::create(imageResource->getContent());
+
+  ResourceResponse partialResponse(testURL, "image/jpeg",
+                                   kJpegImageSubrangeWithoutDimensionsLength,
+                                   nullAtom, String());
+  partialResponse.setHTTPStatusCode(206);

[yhirano, 2017/02/07 09:24:02]

Why do you serve a partial response to a request with no "range" specified?

[hiroshige, 2017/02/07 23:53:48]

Fixed.

+  partialResponse.setHTTPHeaderField(
+      "content-range",
+      buildContentRange(kJpegImageSubrangeWithoutDimensionsLength,
+                        sizeof(kJpegImage)));
+
+  imageResource->loader()->didReceiveResponse(
+      WrappedResourceResponse(partialResponse));
+  imageResource->loader()->didReceiveData(
+      reinterpret_cast<const char*>(kJpegImage),
+      kJpegImageSubrangeWithoutDimensionsLength);
+
+  EXPECT_EQ(ResourceStatus::Pending, imageResource->getStatus());
+  EXPECT_FALSE(observer->imageNotifyFinishedCalled());
+  EXPECT_EQ(0, observer->imageChangedCount());
+
+  imageResource->loader()->didFinishLoading(
+      0.0, kJpegImageSubrangeWithoutDimensionsLength,
+      kJpegImageSubrangeWithoutDimensionsLength);
+
+  EXPECT_EQ(ResourceStatus::DecodeError, imageResource->getStatus());
+  EXPECT_TRUE(observer->imageNotifyFinishedCalled());
+  EXPECT_EQ(ResourceStatus::DecodeError,
+            observer->statusOnImageNotifyFinished());
+  EXPECT_EQ(1, observer->imageChangedCount());
+  EXPECT_FALSE(imageResource->isLoading());
+}
+
 TEST(ImageResourceTest, FetchDisallowPlaceholder) {
   KURL testURL(ParsedURLString, "http://www.test.com/cancelTest.html");
   ScopedRegisteredURL scopedRegisteredURL(testURL);
@@ -1108,6 +1212,78 @@ TEST(ImageResourceTest, FetchAllowPlaceholderUnsuccessful) {
   EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
 }
 
+TEST(ImageResourceTest, FetchAllowPlaceholderPartialContentWithoutDimensions) {

[yhirano, 2017/02/07 09:24:02]

The server sends a response consisting of 3 bytes for a request requesting
0-2047 bytes, and it sends a response consisting of 300+ bytes for a request for
the same URL without a range header. Isn't this behavior strange?

[hiroshige, 2017/02/07 23:53:48]

Yes, strange, but in some of the other unit tests the server responds with
partial response of ~300 bytes for "bytes=0-2047" range requests (see
ReloadIfLoFiOrPlaceholderForPlaceholder, or the other uses of
kJpegImageSubrangeWithDimensionsLength).
How about filing a crbug and doing it later, as I feel it has lower priority and
doesn't affect Issue 667641 directly?
sclittle@, any thoughts?

To make this test consistent, we have to create a JPEG file with >2K that
doesn't contain image dimension information in the first 2KB, and make the
server to serve the first 2KB of that file for the first request, and serve the
whole file for the second request.
(I know there exists such a JPEG file but haven't constructed one yet)
Creating such a strange JPEG and testing it might be worth doing (valid but
extreme cases might be more likely to expose bugs!), but I don't want to have
Issue 667641 blocked by that.

[sclittle, 2017/02/08 00:32:59]

+1 to filing a crbug and doing it later, I agree that it's low priority.

It is strange, but it does work OK for now. This way, we don't have to embed or
run over a 2KB+ image in the tests, and since Blink currently only cares if the
response is decodable even if the response range doesn't correspond to the
requested range, so it works fine.

Also, for this test in particular, the strange 3-byte range response is along
the lines of what a misbehaving server might return, which is one of the reasons
that the full image is loaded when an error occurs.

[yhirano, 2017/02/08 01:18:39]

Please put some comments.

+  KURL testURL(ParsedURLString, "http://www.test.com/cancelTest.html");
+  ScopedRegisteredURL scopedRegisteredURL(testURL);
+
+  FetchRequest request(testURL, FetchInitiatorInfo());
+  request.setAllowImagePlaceholder();
+  ImageResource* imageResource = ImageResource::fetch(
+      request,
+      ResourceFetcher::create(ImageResourceTestMockFetchContext::create()));
+  EXPECT_EQ(FetchRequest::AllowPlaceholder,
+            request.placeholderImageRequestType());
+  EXPECT_EQ("bytes=0-2047",
+            imageResource->resourceRequest().httpHeaderField("range"));
+  EXPECT_TRUE(imageResource->isPlaceholder());
+  std::unique_ptr<MockImageResourceObserver> observer =
+      MockImageResourceObserver::create(imageResource->getContent());
+
+  ResourceResponse partialResponse(testURL, "image/jpeg",
+                                   kJpegImageSubrangeWithoutDimensionsLength,
+                                   nullAtom, String());
+  partialResponse.setHTTPStatusCode(206);
+  partialResponse.setHTTPHeaderField(
+      "content-range",
+      buildContentRange(kJpegImageSubrangeWithoutDimensionsLength,
+                        sizeof(kJpegImage)));
+
+  imageResource->loader()->didReceiveResponse(
+      WrappedResourceResponse(partialResponse));
+  imageResource->loader()->didReceiveData(
+      reinterpret_cast<const char*>(kJpegImage),
+      kJpegImageSubrangeWithoutDimensionsLength);
+
+  EXPECT_EQ(0, observer->imageChangedCount());
+
+  imageResource->loader()->didFinishLoading(
+      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());
+  EXPECT_TRUE(imageResource->getContent()->getImage()->isBitmapImage());
+}
+
 TEST(ImageResourceTest, FetchAllowPlaceholderThenDisallowPlaceholder) {
   KURL testURL(ParsedURLString, "http://www.test.com/cancelTest.html");
   ScopedRegisteredURL scopedRegisteredURL(testURL);