Phase II Step 2: Remove setIsPlaceholder() in updateImage()

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

Index: third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
diff --git a/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp b/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
index 8e51801e45a6450109f1e143fa3274b004f32be1..21528f0493f29d74d1c4704311d078f3267454e0 100644
--- a/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
@@ -23,6 +23,9 @@
 
 #include "core/loader/resource/ImageResource.h"
 
+#include <stdint.h>
+#include <v8.h>
+#include <memory>
 #include "core/loader/resource/ImageResourceContent.h"
 #include "core/loader/resource/ImageResourceInfo.h"
 #include "platform/Histogram.h"
@@ -34,11 +37,10 @@
 #include "platform/loader/fetch/ResourceFetcher.h"
 #include "platform/loader/fetch/ResourceLoader.h"
 #include "platform/loader/fetch/ResourceLoadingLog.h"
+#include "platform/network/HTTPParsers.h"
 #include "public/platform/Platform.h"
 #include "wtf/CurrentTime.h"
 #include "wtf/StdLibExtras.h"
-#include <memory>
-#include <v8.h>
 
 namespace blink {
 namespace {
@@ -77,7 +79,9 @@ class ImageResource::ImageResourceInfoImpl final
     return m_resource->response();
   }
   ResourceStatus getStatus() const override { return m_resource->getStatus(); }
-  bool isPlaceholder() const override { return m_resource->isPlaceholder(); }
+  bool shouldShowPlaceholder() const override {
+    return m_resource->shouldShowPlaceholder();
+  }
   bool isCacheValidator() const override {
     return m_resource->isCacheValidator();
   }
@@ -99,9 +103,6 @@ class ImageResource::ImageResourceInfoImpl final
     return m_resource->resourceError();
   }
 
-  void setIsPlaceholder(bool isPlaceholder) override {
-    m_resource->m_isPlaceholder = isPlaceholder;
-  }
   void setDecodedSize(size_t size) override {
     m_resource->setDecodedSize(size);
   }
@@ -172,7 +173,7 @@ ImageResource* ImageResource::fetch(FetchRequest& request,
       fetcher->requestResource(request, ImageResourceFactory(request)));
   if (resource &&
       request.placeholderImageRequestType() != FetchRequest::AllowPlaceholder &&
-      resource->m_isPlaceholder) {
+      resource->shouldShowPlaceholder()) {
     // If the image is a placeholder, but this fetch doesn't allow a
     // placeholder, then load the original image. Note that the cache is not
     // bypassed here - it should be fine to use a cached copy if possible.
@@ -196,7 +197,8 @@ ImageResource::ImageResource(const ResourceRequest& resourceRequest,
       m_devicePixelRatioHeaderValue(1.0),
       m_hasDevicePixelRatioHeaderValue(false),
       m_isSchedulingReload(false),
-      m_isPlaceholder(isPlaceholder),
+      m_placeholderOption(isPlaceholder ? ShowAndReloadPlaceholderAlways
+                                        : DoNotReloadPlaceholder),
       m_flushTimer(this, &ImageResource::flushImageIfNeeded) {
   DCHECK(getContent());
   RESOURCE_LOADING_DVLOG(1) << "new ImageResource(ResourceRequest) " << this;
@@ -319,10 +321,6 @@ void ImageResource::flushImageIfNeeded(TimerBase*) {
   }
 }
 
-bool ImageResource::willPaintBrokenImage() const {
-  return errorOccurred();
-}
-
 void ImageResource::decodeError(bool allDataReceived) {
   size_t size = encodedSize();
 
@@ -379,6 +377,21 @@ void ImageResource::error(const ResourceError& error) {
               true);
 }
 
+// Determines if |response| likely contains the entire resource for the purposes
+// of determining whether or not to show a placeholder, e.g. if the server
+// responded with a full 200 response or if the full image is smaller than the
+// requested range.
+static bool isEntireResource(const ResourceResponse& response) {
+  if (response.httpStatusCode() != 206)
+    return true;
+
+  int64_t firstBytePosition = -1, lastBytePosition = -1, instanceLength = -1;
+  return parseContentRangeHeaderFor206(
+             response.httpHeaderField("Content-Range"), &firstBytePosition,
+             &lastBytePosition, &instanceLength) &&
+         firstBytePosition == 0 && lastBytePosition + 1 == instanceLength;
+}
+
 void ImageResource::responseReceived(
     const ResourceResponse& response,
     std::unique_ptr<WebDataConsumerHandle> handle) {
@@ -401,6 +414,45 @@ void ImageResource::responseReceived(
       m_hasDevicePixelRatioHeaderValue = false;
     }
   }
+
+  if (isEntireResource(this->response())) {
+    if (this->response().httpStatusCode() < 400 ||
+        this->response().httpStatusCode() >= 600) {
+      // Don't treat a complete and broken image as a placeholder if the
+      // response code is something other than a 4xx or 5xx error.
+      // This is done to prevent reissuing the request in cases like
+      // "204 No Content" responses to tracking requests triggered by <img>
+      // tags, and <img> tags used to preload non-image resources.
+      m_placeholderOption = DoNotReloadPlaceholder;
+    } else {
+      m_placeholderOption = ReloadPlaceholderOnDecodeError;

[yhirano, 2017/02/13 11:36:57]

Is DoNotReloadPlaceholder => ReloadPlaceholderOnDecodeError transition valid?

[hiroshige, 2017/02/23 20:52:38]

Oh, looks invalid. Fixed.

+    }
+  }
+}
+
+bool ImageResource::shouldShowPlaceholder() const {
+  switch (m_placeholderOption) {
+    case ShowAndReloadPlaceholderAlways:
+      return true;
+    case ReloadPlaceholderOnDecodeError:
+    case DoNotReloadPlaceholder:
+      return false;
+  }
+  NOTREACHED();
+  return false;
+}
+
+bool ImageResource::shouldReloadBrokenPlaceholder() const {
+  switch (m_placeholderOption) {
+    case ShowAndReloadPlaceholderAlways:
+      return errorOccurred();
+    case ReloadPlaceholderOnDecodeError:
+      return getStatus() == ResourceStatus::DecodeError;
+    case DoNotReloadPlaceholder:
+      return false;
+  }
+  NOTREACHED();
+  return false;
 }
 
 static bool isLoFiImage(const ImageResource& resource) {
@@ -420,7 +472,7 @@ void ImageResource::reloadIfLoFiOrPlaceholderImage(
   if (policy == kReloadIfNeeded && !shouldReloadBrokenPlaceholder())
     return;
 
-  if (!m_isPlaceholder && !isLoFiImage(*this))
+  if (m_placeholderOption == DoNotReloadPlaceholder && !isLoFiImage(*this))
     return;
 
   // Prevent clients and observers from being notified of completion while the
@@ -435,10 +487,9 @@ void ImageResource::reloadIfLoFiOrPlaceholderImage(
 
   setPreviewsStateNoTransform();
 
-  if (m_isPlaceholder) {
-    m_isPlaceholder = false;
+  if (m_placeholderOption != DoNotReloadPlaceholder)
     clearRangeRequestHeader();
-  }
+  m_placeholderOption = DoNotReloadPlaceholder;
 
   if (isLoading()) {
     loader()->cancel();