Don't remove CSSPreloaderResourceClient for unused speculative markup preloads

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 8968aeca01cac2fd5c0e9d819f85d23ddcf7da6d..26e48ddf2b1ade1e1883640f537f085a45e92cd0 100644
--- a/third_party/WebKit/Source/core/fetch/Resource.cpp
+++ b/third_party/WebKit/Source/core/fetch/Resource.cpp
@@ -754,20 +754,26 @@ void Resource::removeClient(ResourceClient* client) {
 }
 
 void Resource::didRemoveClientOrObserver() {
-  if (!hasClientsOrObservers() && m_isAlive) {
-    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);
-  }
+  if (hasClientsOrObservers() || !m_isAlive)
+    return;
+
+  // 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() {