New issue
Advanced search Search tips

Issue 610437 link

Starred by 1 user
Status: Fixed
Owner:
csharrison@chromium.org
Closed: Sep 2016
Cc:
japhet@chromium.org
csharrison@chromium.org
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Add support for adding weak ResourceClients

Project Member Reported by csharrison@chromium.org, May 9 2016 
The current state of detecting whether preloads are "referenced" only looks at whether or not they have active ResourceClients:

See: void Resource::willAddClientOrObserver

For some reasons (e.g. CSS), we might want to do pre-processing of a preload request before it is properly used. One example is preload scanning these preloads.

It would be nice to have a way to attach a ResourceClient to a preload without it affected any of the current logic for Preloads (i.e. anything touching Resource::PreloadResult).

Comment 1 by clamy@chromium.org, May 11 2016

Owner: hirosh...@chromium.org
Status: Assigned (was: Untriaged)
Assigning to hiroshige@ as part of the loading bug triage rotation. Please reassign if mistaken.

Comment 2 by csharrison@chromium.org, May 11 2016

Owner: csharrison@chromium.org
I could probably own this. There may be a better solution for this though.

I'm not sure if WeakResourceClient is too general. We may just want a PreloadObserver or something, separate from the ResourceClient stack.

Comment 3 by csharrison@chromium.org, May 12 2016 

hiroshige@, I think the easiest way to go about doing this is to add a separate method on Resource, addPassiveResourceClient. This method would keep a preload in PreloadNotReferenced if it has no other clients.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 26 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c016ef022ac17d029d61bc79d8a8996c053556f3

commit c016ef022ac17d029d61bc79d8a8996c053556f3
Author: csharrison <csharrison@chromium.org>
Date: Fri Aug 26 18:30:08 2016

Preload scan external CSS for @import

This change adds a StyleSheetResourceClient on all CSS
preloads. The client holds a CSSPreloadScanner which receives
the stream of CSS as it comes in.

This is guarded by two Blink settings,
cssExternalScannerPreload and cssExternalScannerNoPreload.
The former allows for scanning and preloading and the latter just
allows for scanning, as a baseline for the overhead of the feature.

This is partially a reland of
https://codereview.chromium.org/1819593002, which was
reverted manually here:
https://codereview.chromium.org/1947053002/

The revert was due to a crash and some faulty logic with
regard to all css preload requests being considered
"referenced". This CL attempts to fix that by adding an enum
to mark a ResourceClient as "passive".

BUG= 610437 , 596676 

Review-Url: https://codereview.chromium.org/1976463003
Cr-Commit-Position: refs/heads/master@{#414756}

[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/chrome/browser/chrome_content_browser_client.cc
[add] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/LayoutTests/http/tests/preload/external_css_import_no_scanning.html
[add] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/LayoutTests/http/tests/preload/external_css_import_preload.html
[add] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/LayoutTests/http/tests/preload/external_css_import_scan_only.html
[add] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/LayoutTests/http/tests/resources/css_with_import.css
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/core.gypi
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/CSSStyleSheetResource.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/ImageResource.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/Resource.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/Resource.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/ResourceOwner.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/fetch/StyleSheetResourceClient.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/frame/Settings.in
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.h
[add] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.h
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/c016ef022ac17d029d61bc79d8a8996c053556f3/third_party/WebKit/public/platform/WebLoadingBehaviorFlag.h

Comment 5 by csharrison@chromium.org, Sep 7 2016

Status: Fixed (was: Assigned)
►

Sign in to add a comment