Commit dff0384e authored by csharrison's avatar csharrison Committed by Commit bot

Don't remove CSSPreloaderResourceClient for unused speculative markup preloads

The CSS @import scanner attaches a passive resource client to a css
preload request. This passive client should not affect the policy
decisions of the preload and should just observe notifications passively.

This patch fixes a bug where removing a passive client from an otherwise
unused preload ends up cancelling it, which removes the preload from
memory cache. This is very wrong behavior, and causes the optimization
to be less effective, and report bad metrics.

Simply not removing the client will not cause the resource to live
longer than necessary, because the client holds only weak references
to the resource.

BUG=670295,662999

Review-Url: https://codereview.chromium.org/2542183002
Cr-Commit-Position: refs/heads/master@{#436312}
parent 5e03cd34
......@@ -505,8 +505,6 @@ crbug.com/248938 virtual/threaded/animations/fill-mode-multiple-keyframes.html [
crbug.com/659123 [ Mac ] fast/css/text-overflow-ellipsis-button.html [ Pass Failure ]
crbug.com/670295 virtual/mojo-loading/http/tests/preload/external_css_import_preload.html [ Pass Failure ]
# TODO(oshima): Mac Android are currently not supported.
crbug.com/567837 [ Mac Android ] virtual/scalefactor200withzoom/fast/hidpi/static [ Skip ]
......
......@@ -320,6 +320,18 @@ void CSSPreloaderResourceClient::fetchPreloads(PreloadRequestStream& preloads) {
}
void CSSPreloaderResourceClient::clearResource() {
// Do not remove the client for unused, speculative markup preloads. This will
// trigger cancellation of the request and potential removal from memory
// cache. Link preloads are an exception because they support dynamic removal
// cancelling the request (and have their own passive resource client).
// Note: Speculative preloads which remain unused for their lifetime will
// never have this client removed. This should be fine because we only hold
// weak references to the resource.
if (m_resource && m_resource->isUnusedPreload() &&
!m_resource->isLinkPreload()) {
return;
}
if (m_resource)
m_resource->removeClient(this);
m_resource.clear();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment