Blink: Throttle progressively loaded images.

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

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
     Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
     License as published by the Free Software Foundation; either
@@ skipping 27 matching lines @@
 #include "public/platform/WebCachePolicy.h"
 #include "wtf/CurrentTime.h"
 #include "wtf/HashCountedSet.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/Vector.h"
 #include <memory>
 #include <v8.h>
 
 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)
         request.mutableResourceRequest().setRequestContext(WebURLRequest::RequestContextImage);
     if (fetcher->context().pageDismissalEventBeingDispatched()) {
         KURL requestURL = request.resourceRequest().url();
         if (requestURL.isValid() && fetcher->context().canRequest(Resource::Image, request.resourceRequest(), requestURL, request.options(), request.forPreload(), request.getOriginRestriction()))
             fetcher->context().sendImagePing(requestURL);
         return nullptr;
     }
 
     return toImageResource(fetcher->requestResource(request, ImageResourceFactory()));
 }
 
 ImageResource::ImageResource(const ResourceRequest& resourceRequest, const ResourceLoaderOptions& options)
     : Resource(resourceRequest, Image, options)
     , m_devicePixelRatioHeaderValue(1.0)
     , m_image(nullptr)
     , m_hasDevicePixelRatioHeaderValue(false)
+    , m_lastFlushTime(0.)
 {
     RESOURCE_LOADING_DVLOG(1) << "new ImageResource(ResourceRequest) " << this;
 }
 
 ImageResource::ImageResource(blink::Image* image, const ResourceLoaderOptions& options)
     : Resource(ResourceRequest(""), Image, options)
     , m_devicePixelRatioHeaderValue(1.0)
     , m_image(image)
     , m_hasDevicePixelRatioHeaderValue(false)
+    , m_lastFlushTime(0.)
 {
     RESOURCE_LOADING_DVLOG(1) << "new ImageResource(Image) " << this;
     setStatus(Cached);
 }
 
 ImageResource::~ImageResource()
 {
     RESOURCE_LOADING_DVLOG(1) << "~ImageResource " << this;
     clearImage();
 }
@@ skipping 154 matching lines @@
     return nullptr;
 }
 
 void ImageResource::appendData(const char* data, size_t length)
 {
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(length);
     if (m_multipartParser) {
         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;
+        }
     }
 }
 
 std::pair<blink::Image*, float> ImageResource::brokenImage(float deviceScaleFactor)
 {
     if (deviceScaleFactor >= 2) {
         DEFINE_STATIC_REF(blink::Image, brokenImageHiRes, (blink::Image::loadPlatformResource("missingImage@2x")));
         return std::make_pair(brokenImageHiRes, 2);
     }
 
@@ skipping 332 matching lines @@
     if (response().wasFetchedViaServiceWorker())
         return response().serviceWorkerResponseType() != WebServiceWorkerResponseTypeOpaque;
     if (!getImage()->currentFrameHasSingleSecurityOrigin())
         return false;
     if (passesAccessControlCheck(securityOrigin))
         return true;
     return !securityOrigin->taintsCanvas(response().url());
 }
 
 } // namespace blink