Add Blink support for showing image placeholders using range requests.

third_party/WebKit/Source/core/fetch/ImageResource.cpp

Index: third_party/WebKit/Source/core/fetch/ImageResource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/ImageResource.cpp b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
index e4c93592c8e0ac2361f84aab18df2f68f2c03fd8..7422ab34696fbbffb7d74ea639127b1601391569 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -32,7 +32,9 @@
 #include "core/svg/graphics/SVGImage.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/SharedBuffer.h"
+#include "platform/geometry/IntSize.h"
 #include "platform/graphics/BitmapImage.h"
+#include "platform/graphics/PlaceholderImage.h"
 #include "platform/tracing/TraceEvent.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebCachePolicy.h"
@@ -45,6 +47,26 @@
 
 namespace blink {
 
+class ImageResource::ImageResourceFactory : public ResourceFactory {
+  STACK_ALLOCATED();
+
+ public:
+  ImageResourceFactory(const FetchRequest& fetchRequest)
+      : ResourceFactory(Resource::Image), m_fetchRequest(&fetchRequest) {}
+
+  Resource* create(const ResourceRequest& request,
+                   const ResourceLoaderOptions& options,
+                   const String&) const override {
+    return new ImageResource(request, options,
+                             m_fetchRequest->placeholderImageRequestType() ==
+                                 FetchRequest::AllowPlaceholder);
+  }
+
+ private:
+  // Weak, unowned pointer. Must outlive |this|.
+  const FetchRequest* m_fetchRequest;

[Nate Chapin, 2016/10/18 18:23:17]

Instead of stashing the FetchRequest here and adding a parameter to the
constructor, could we just detect the range header on the ResourceRequest in the
constructor?

[sclittle, 2016/10/18 23:53:03]

I'm hesitant to just detect the range header, since there's nothing stopping
some other caller of ImageResource::fetch() from adding their own range header
for an image fetch for some reason. Currently, the code only adds Range header
and starts the placeholder logic if there isn't already a Range header on the
request.

I don't know of any other feature that uses range headers for images, or if it's
possible for JS to add a range header somehow. I can change this to just check
the range header if you think that's better though.

+};
+
 ImageResource* ImageResource::fetch(FetchRequest& request,
                                     ResourceFetcher* fetcher) {
   if (request.resourceRequest().requestContext() ==
@@ -63,17 +85,28 @@ ImageResource* ImageResource::fetch(FetchRequest& request,
     return nullptr;
   }
 
-  return toImageResource(
-      fetcher->requestResource(request, ImageResourceFactory()));
+  ImageResource* resource = toImageResource(
+      fetcher->requestResource(request, ImageResourceFactory(request)));
+  if (resource &&
+      request.placeholderImageRequestType() != FetchRequest::AllowPlaceholder &&
+      resource->m_isPlaceholder) {
+    // 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.
+    resource->reloadIfLoFiOrPlaceholder(fetcher, false);
+  }
+  return resource;
 }
 
 ImageResource::ImageResource(const ResourceRequest& resourceRequest,
-                             const ResourceLoaderOptions& options)
+                             const ResourceLoaderOptions& options,
+                             bool isPlaceholder)
     : Resource(resourceRequest, Image, options),
       m_devicePixelRatioHeaderValue(1.0),
       m_image(nullptr),
       m_hasDevicePixelRatioHeaderValue(false),
-      m_isSchedulingReload(false) {
+      m_isSchedulingReload(false),
+      m_isPlaceholder(isPlaceholder) {
   RESOURCE_LOADING_DVLOG(1) << "new ImageResource(ResourceRequest) " << this;
 }
 
@@ -83,7 +116,8 @@ ImageResource::ImageResource(blink::Image* image,
       m_devicePixelRatioHeaderValue(1.0),
       m_image(image),
       m_hasDevicePixelRatioHeaderValue(false),
-      m_isSchedulingReload(false) {
+      m_isSchedulingReload(false),
+      m_isPlaceholder(false) {
   RESOURCE_LOADING_DVLOG(1) << "new ImageResource(Image) " << this;
   setStatus(Cached);
 }
@@ -103,7 +137,7 @@ DEFINE_TRACE(ImageResource) {
 void ImageResource::checkNotify() {
   // Don't notify observers and clients of completion if this ImageResource is
   // about to be reloaded.
-  if (m_isSchedulingReload)
+  if (m_isSchedulingReload || shouldReloadBrokenPlaceholder())
     return;
 
   notifyObserversInternal(MarkFinishedOption::ShouldMarkFinished);
@@ -136,7 +170,7 @@ void ImageResource::didAddClient(ResourceClient* client) {
 
   // Don't notify observers and clients of completion if this ImageResource is
   // about to be reloaded.
-  if (m_isSchedulingReload)
+  if (m_isSchedulingReload || shouldReloadBrokenPlaceholder())
     return;
 
   Resource::didAddClient(client);
@@ -164,7 +198,7 @@ void ImageResource::addObserver(ImageResourceObserver* observer) {
     observer->imageChanged(this);
   }
 
-  if (isLoaded() && !m_isSchedulingReload) {
+  if (isLoaded() && !m_isSchedulingReload && !shouldReloadBrokenPlaceholder()) {
     markObserverFinished(observer);
     observer->imageNotifyFinished(this);
   }
@@ -402,6 +436,22 @@ void ImageResource::updateImage(bool allDataReceived) {
   // observers to repaint, which will force that chunk to decode.
   if (sizeAvailable == Image::SizeUnavailable && !allDataReceived)
     return;
+
+  if (m_isPlaceholder && allDataReceived && m_image && !m_image->isNull()) {
+    if (sizeAvailable == Image::SizeAvailable) {
+      // TODO(sclittle): Show the original image if the response consists of the
+      // entire image, such as if the entire image response body is smaller than
+      // the requested range.
+      IntSize dimensions = m_image->size();
+      clearImage();
+      m_image = PlaceholderImage::create(this, dimensions);
+    } else {
+      // Clear the image so that it gets treated like a decoding error, since
+      // the attempt to build a placeholder image failed.
+      clearImage();
+    }
+  }
+
   if (!m_image || m_image->isNull()) {
     size_t size = encodedSize();
     clear();
@@ -535,12 +585,14 @@ void ImageResource::updateImageAnimationPolicy() {
   }
 }
 
-void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher) {
-  if (resourceRequest().loFiState() != WebURLRequest::LoFiOn)
-    return;
-  if (isLoaded() &&
-      !response().httpHeaderField("chrome-proxy").contains("q=low"))
+void ImageResource::reloadIfLoFiOrPlaceholder(ResourceFetcher* fetcher,
+                                              bool bypassCache) {

[Nate Chapin, 2016/10/18 18:23:17]

bypassCache should probably be an enum.

[sclittle, 2016/10/18 23:53:03]

Done.

+  if (!m_isPlaceholder &&

[Nate Chapin, 2016/10/18 18:23:17]

This if-statement is sufficiently complex that it might be be time to pull it
out into a helper, even though there's only one callsite.

[sclittle, 2016/10/18 23:53:03]

Done.

+      (resourceRequest().loFiState() != WebURLRequest::LoFiOn ||
+       (isLoaded() &&
+        !response().httpHeaderField("chrome-proxy").contains("q=low")))) {
     return;
+  }
 
   // Prevent clients and observers from being notified of completion while the
   // reload is being scheduled, so that e.g. canceling an existing load in
@@ -549,8 +601,15 @@ void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher) {
   DCHECK(!m_isSchedulingReload);
   m_isSchedulingReload = true;
 
-  setCachePolicyBypassingCache();
+  if (bypassCache)
+    setCachePolicyBypassingCache();
   setLoFiStateOff();
+
+  if (m_isPlaceholder) {
+    m_isPlaceholder = false;
+    clearRangeRequestHeader();
+  }
+
   if (isLoading()) {
     loader()->cancel();
     // Canceling the loader causes error() to be called, which in turn calls