Wait to notify completion until after a Lo-Fi image is reloaded.

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 53 matching lines @@
 
   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_hasDevicePixelRatioHeaderValue(false),
+      m_isSchedulingReload(false) {
   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_hasDevicePixelRatioHeaderValue(false),
+      m_isSchedulingReload(false) {
   RESOURCE_LOADING_DVLOG(1) << "new ImageResource(Image) " << this;
   setStatus(Cached);
 }
 
 ImageResource::~ImageResource() {
   RESOURCE_LOADING_DVLOG(1) << "~ImageResource " << this;
   clearImage();
 }
 
 DEFINE_TRACE(ImageResource) {
   visitor->trace(m_multipartParser);
   Resource::trace(visitor);
   ImageObserver::trace(visitor);
   MultipartImageResourceParser::Client::trace(visitor);
 }
 
 void ImageResource::checkNotify() {
+  // Don't notify observers and clients of completion if this ImageResource is
+  // about to be reloaded.
+  if (m_isSchedulingReload)
+    return;
+
   notifyObserversInternal(MarkFinishedOption::ShouldMarkFinished);
   Resource::checkNotify();
 }
 
 void ImageResource::notifyObserversInternal(
     MarkFinishedOption markFinishedOption) {
-  if (isLoading())
+  if (isLoading() || m_isSchedulingReload)

[Nate Chapin, 2016/10/12 23:45:23]

This should be redundant. checkNotify() is already covered, and the only other
caller, onePartInMultipartReceived(), is only done in response to an
appendData() callback, which shouldn't be possible inside reloadIfLofi().

[sclittle, 2016/10/12 23:49:05]

Done.

     return;
 
   for (auto* observer : m_observers.asVector()) {
     if (m_observers.contains(observer)) {
       if (markFinishedOption == MarkFinishedOption::ShouldMarkFinished)
         markObserverFinished(observer);
       observer->imageNotifyFinished(this);
     }
   }
 }
 
 void ImageResource::markObserverFinished(ImageResourceObserver* observer) {
   if (m_observers.contains(observer)) {
     m_finishedObservers.add(observer);
     m_observers.remove(observer);
   }
 }
 
 void ImageResource::didAddClient(ResourceClient* client) {
   DCHECK((m_multipartParser && isLoading()) || !data() || m_image);
+
+  // Don't notify observers and clients of completion if this ImageResource is
+  // about to be reloaded.
+  if (m_isSchedulingReload)
+    return;
+
   Resource::didAddClient(client);
 }
 
 void ImageResource::addObserver(ImageResourceObserver* observer) {
   willAddClientOrObserver(MarkAsReferenced);
 
   m_observers.add(observer);
 
   if (isCacheValidator())
     return;
 
   // When the response is not multipart, if |data()| exists, |m_image| must be
   // created. This is assured that |updateImage()| is called when |appendData()|
   // is called.
   //
   // On the other hand, when the response is multipart, |updateImage()| is not
   // called in |appendData()|, which means |m_image| might not be created even
   // when |data()| exists. This is intentional since creating a |m_image| on
   // receiving data might destroy an existing image in a previous part.
   DCHECK((m_multipartParser && isLoading()) || !data() || m_image);
 
   if (m_image && !m_image->isNull()) {
     observer->imageChanged(this);
   }
 
-  if (isLoaded()) {
+  if (isLoaded() && !m_isSchedulingReload) {
     markObserverFinished(observer);
     observer->imageNotifyFinished(this);
   }
 }
 
 void ImageResource::removeObserver(ImageResourceObserver* observer) {
   DCHECK(observer);
 
   if (m_observers.contains(observer))
     m_observers.remove(observer);
@@ skipping 361 matching lines @@
     m_image->setAnimationPolicy(newPolicy);
   }
 }
 
 void ImageResource::reloadIfLoFi(ResourceFetcher* fetcher) {
   if (resourceRequest().loFiState() != WebURLRequest::LoFiOn)
     return;
   if (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
+  // progress doesn't cause clients and observers to be notified of completion
+  // prematurely.
+  DCHECK(!m_isSchedulingReload);
+  m_isSchedulingReload = true;
+
   setCachePolicyBypassingCache();
   setLoFiStateOff();
-  if (isLoading())
+  if (isLoading()) {
     loader()->cancel();
-  clear();
-  notifyObservers();
+    // Canceling the loader causes error() to be called, which in turn calls
+    // clear() and notifyObservers(), so there's no need to call these again
+    // here.
+  } else {
+    clear();
+    notifyObservers();
+  }
 
   setStatus(NotStarted);
+
+  DCHECK(m_isSchedulingReload);
+  m_isSchedulingReload = false;
+
   fetcher->startLoad(this);
 }
 
 void ImageResource::changedInRect(const blink::Image* image,
                                   const IntRect& rect) {
   if (!image || image != m_image)
     return;
   notifyObservers(&rect);
 }
 
@@ skipping 33 matching lines @@
     return response().serviceWorkerResponseType() !=
            WebServiceWorkerResponseTypeOpaque;
   if (!getImage()->currentFrameHasSingleSecurityOrigin())
     return false;
   if (passesAccessControlCheck(securityOrigin))
     return true;
   return !securityOrigin->taintsCanvas(response().url());
 }
 
 }  // namespace blink