Blink: Throttle progressively loaded images.

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 be3e5de6e7041ee64971b7cbd566affdb0d64bc1..5701e37b361154aaed590bc83a4dfc835cd64db7 100644
--- a/third_party/WebKit/Source/core/fetch/ImageResource.cpp
+++ b/third_party/WebKit/Source/core/fetch/ImageResource.cpp
@@ -45,6 +45,8 @@
 
 namespace blink {
 
+static double flushDelay = 1.;

[enne (OOO), 2016/09/29 22:49:34]

flushDelayInSeconds?

Mumble mumble wtb base::TimeDelta.

[vmpstr, 2016/10/04 21:18:36]

Done.

+
 ImageResource* ImageResource::fetch(FetchRequest& request, ResourceFetcher* fetcher)
 {
     if (request.resourceRequest().requestContext() == WebURLRequest::RequestContextUnspecified)
@@ -64,6 +66,7 @@ ImageResource::ImageResource(const ResourceRequest& resourceRequest, const Resou
     , m_devicePixelRatioHeaderValue(1.0)
     , m_image(nullptr)
     , m_hasDevicePixelRatioHeaderValue(false)
+    , m_lastFlushTime(0.)
 {
     RESOURCE_LOADING_DVLOG(1) << "new ImageResource(ResourceRequest) " << this;
 }
@@ -73,6 +76,7 @@ ImageResource::ImageResource(blink::Image* image, const ResourceLoaderOptions& o
     , m_devicePixelRatioHeaderValue(1.0)
     , m_image(image)
     , m_hasDevicePixelRatioHeaderValue(false)
+    , m_lastFlushTime(0.)
 {
     RESOURCE_LOADING_DVLOG(1) << "new ImageResource(Image) " << this;
     setStatus(Cached);
@@ -247,7 +251,13 @@ void ImageResource::appendData(const char* data, size_t length)
         m_multipartParser->appendData(data, length);
     } else {
         Resource::appendData(data, length);
-        updateImage(false);
+        double now = WTF::currentTime();
+        if (m_lastFlushTime == 0.) {
+            m_lastFlushTime = now;
+        } else if (m_lastFlushTime + flushDelay <= now) {
+            updateImage(false);

[enne (OOO), 2016/09/29 22:49:34]

Can you leave a big ol' comment here saying this throttles invalidations, but if
outside invalidations occur a new image will still be used? And that this could
be fixed with an extra copy, but it's expected this shouldn't happen in
practice.  I think that'd be super useful for future code archeologists.

I *do* think this approach could cause extra decodes potentially.  It could also
cause a different level of decoding to be used across multiple tiles (but I
think that wouldn't look too weird, as it would eventually fix itself).  It'd
just be a hole.

I think this is 100% ok as a first pass and I don't think it'll cause problems. 
I'd rather land something simple and make it more complicated later than making
it more complicated to start with.

[vmpstr, 2016/10/04 21:18:36]

Actually looking closer at the code, I think even outside invalidations wouldn't
see the new data. There's m_image that we actually grab and data() that lives on
Resource base class. updateImage, other than informing the observers, also puts
data() into m_image. So, if we don't call it, the outside observers should see
the old image.

+            m_lastFlushTime = now;
+        }
     }
 }