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

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

Index: third_party/WebKit/Source/core/fetch/Resource.cpp
diff --git a/third_party/WebKit/Source/core/fetch/Resource.cpp b/third_party/WebKit/Source/core/fetch/Resource.cpp
index d0de8259a3b53162531e2bb2d529296f5f0cf925..f684f39cc921044fa52872e731db0c99c9d34c1b 100644
--- a/third_party/WebKit/Source/core/fetch/Resource.cpp
+++ b/third_party/WebKit/Source/core/fetch/Resource.cpp
@@ -317,6 +317,7 @@ Resource::Resource(const ResourceRequest& request,
       m_isAlive(false),
       m_integrityDisposition(ResourceIntegrityDisposition::NotChecked),
       m_isAddRemoveClientProhibited(false),
+      m_isNotifyClientsOfCompletionProhibited(false),

[Nate Chapin, 2016/10/10 23:47:49]

I'm suspicious this can be on ImageResource instead of Resource. I think you can
stop all these notifications by hooking ImageResource::checkNotify? Especially
since clients shouldn't get added/removed during cancellation if callbacks are
being surpressed. Is there a case I'm forgetting about?

[sclittle, 2016/10/11 02:56:56]

Only the completion callbacks are being suppressed, not others like
ImageResourceObserver::imageChanged(), so IIUC it's possible that an
imageChanged() or other kind of callback gets called and adds a new
ResourceClient.

We could try to suppress imageChanged callbacks too, but there are other
ImageResourceObserver callbacks that probably shouldn't be suppressed during a
reload and could still possibly add clients or observers, like
ImageResourceObserver::computeResourcePriority().

I could be missing something, but AIUI it seems easier to just handle this in
Resource than to enforce that no callbacks add observers or clients. WDYT?

[Nate Chapin, 2016/10/12 22:56:17]

I'm not certain how much of a risk this is in practice, but assuming it is: I
think supressing in ImageResource::didAddClient and ImageResource::checkNotify
should case all caseS?

[sclittle, 2016/10/12 23:38:16]

Good point, sorry, I forgot about ImageResource::didAddClient. I've moved this
boolean to ImageResource, and renamed it to m_isSchedulingReload to be more
specific.

       m_options(options),
       m_responseTimestamp(currentTime()),
       m_cancelTimer(this, &Resource::cancelTimerFired),
@@ -356,7 +357,7 @@ void Resource::checkNotify() {
 }
 
 void Resource::notifyClientsInternal(MarkFinishedOption markFinishedOption) {
-  if (isLoading())
+  if (isLoading() || m_isNotifyClientsOfCompletionProhibited)
     return;
 
   ResourceClientWalker<ResourceClient> w(m_clients);
@@ -656,7 +657,7 @@ String Resource::reasonNotDeletable() const {
 }
 
 void Resource::didAddClient(ResourceClient* c) {
-  if (isLoaded()) {
+  if (isLoaded() && !m_isNotifyClientsOfCompletionProhibited) {
     c->notifyFinished(this);
     if (m_clients.contains(c)) {
       m_finishedClients.add(c);