Don't remove CSSPreloaderResourceClient for unused speculative markup preloads

third_party/WebKit/Source/core/fetch/Resource.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, 2008, 2009, 2010, 2011 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
@@ skipping 736 matching lines @@
   else
     m_clients.remove(client);
 
   if (m_clientsAwaitingCallback.isEmpty())
     ResourceCallback::callbackHandler().cancel(this);
 
   didRemoveClientOrObserver();
 }
 
 void Resource::didRemoveClientOrObserver() {
-  if (!hasClientsOrObservers() && m_isAlive) {
-    m_isAlive = false;
-    allClientsAndObserversRemoved();
+  if (hasClientsOrObservers() || !m_isAlive)
+    return;
 
-    // RFC2616 14.9.2:
-    // "no-store: ... MUST make a best-effort attempt to remove the information
-    // from volatile storage as promptly as possible"
-    // "... History buffers MAY store such responses as part of their normal
-    // operation."
-    // We allow non-secure content to be reused in history, but we do not allow
-    // secure content to be reused.
-    if (hasCacheControlNoStoreHeader() && url().protocolIs("https"))
-      memoryCache()->remove(this);
-  }
+  // Allow speculative markup preloads to continue even without clients or
+  // observers. Their lifetime is determined by the ResourceFetcher.
+  if (isUnusedPreload() && !isLinkPreload())

[yhirano, 2016/12/02 01:32:25]

Is it possible to register a ResourceClient instead? I would like to keep this
function simple as much as possible.

[Charlie Harrison, 2016/12/02 04:20:22]

Hm... so the CSSPreloaderResourceClient removes itself as soon as it gets the
data it needs, which triggers this callback. We could have that client *not*
remove itself, but that would entail other problematic things like link preloads
that have "as=style" not cancelling if they are dynamically removed from the
page.

[yhirano, 2016/12/02 08:50:29]

Then is it possible to stop removing itself only when isLinkPreload() returns
false?

[Charlie Harrison, 2016/12/02 13:14:15]

I think it might be possible now that CSSPreloaderResourceClient holds weak
references to the resource. Let me think it through to make sure there are no
gotchas.

I'm not totally convinced that that solution is simpler than this one, because
its action is more "at a distance", but I don't feel too strongly and I agree
this method being simple is better for everyone.

+    return;
+
+  m_isAlive = false;
+  allClientsAndObserversRemoved();
+
+  // RFC2616 14.9.2:
+  // "no-store: ... MUST make a best-effort attempt to remove the information
+  // from volatile storage as promptly as possible"
+  // "... History buffers MAY store such responses as part of their normal
+  // operation."
+  // We allow non-secure content to be reused in history, but we do not allow
+  // secure content to be reused.
+  if (hasCacheControlNoStoreHeader() && url().protocolIs("https"))
+    memoryCache()->remove(this);
 }
 
 void Resource::allClientsAndObserversRemoved() {
   if (!m_loader)
     return;
   if (!m_cancelTimer.isActive())
     m_cancelTimer.startOneShot(0, BLINK_FROM_HERE);
 }
 
 void Resource::cancelTimerFired(TimerBase* timer) {
@@ skipping 318 matching lines @@
     case Resource::TextTrack:
     case Resource::Media:
     case Resource::Manifest:
       return false;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace blink