New issue
Advanced search Search tips

Issue 789198 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Flatten the ResourceClient hierachy

Project Member Reported by japhet@chromium.org, Nov 28 2017

Issue description

Currently, blink has a ResourceClient interface that is used to listen to various steps in the resource loading process. The base class has a single NotifyFinished() callback, and subclasses provide others. The most full-featured of these is RawResourceClient, which provides callbacks for most (all?) of the IPCs the renderer process receives related to network requests. There are a couple weird details with the current hierachy:

* StyleSheetResourceClients never get the NotifyFinished() callback from the base class, and instead get custom SetCSSStyleSheet() or SetXSLStyleSheet() callbacks. These could be changed to use NotifyFinished()
* Multiple subclasses have callbacks for getting access to the data as it streams in, rather than waiting for the finish notifications (ScriptResourceClient, StyleSheetResourceClient, and RawResourceClient all do this). We could move this callback down to the base ResourceClient and standardize on that.
* By convention, classes don't implement ResourceClient directly, and always use an intermediate interface. There's no reason this is necessary, so with the first two changes, we can get rid of ScriptResourceClient, StyleSheetResourceClient, and DocumentResourceClient and have their users refer to ResourceClient directly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 29 2017

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

commit 81ff5ac467fa7967cb81a9fd98dbf17804e8cb59
Author: Nate Chapin <japhet@chromium.org>
Date: Wed Nov 29 19:30:48 2017

Use ResourceClient instead of DocumentResourceClient

There's no additional functionality for DocumentResourceClient. Just use the
base class.

Bug:  789198 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ied4815fbba70bb6b7e4e856d654c9d2d6035e446
Reviewed-on: https://chromium-review.googlesource.com/792000
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520198}
[modify] https://crrev.com/81ff5ac467fa7967cb81a9fd98dbf17804e8cb59/third_party/WebKit/Source/core/loader/resource/DocumentResource.h
[modify] https://crrev.com/81ff5ac467fa7967cb81a9fd98dbf17804e8cb59/third_party/WebKit/Source/core/svg/SVGResourceClient.h
[modify] https://crrev.com/81ff5ac467fa7967cb81a9fd98dbf17804e8cb59/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
[modify] https://crrev.com/81ff5ac467fa7967cb81a9fd98dbf17804e8cb59/third_party/WebKit/Source/core/svg/SVGUseElement.h
[modify] https://crrev.com/81ff5ac467fa7967cb81a9fd98dbf17804e8cb59/third_party/WebKit/Source/platform/loader/fetch/ResourceClient.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 30 2017

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

commit 5887f770941260df0da1395c391db5a3bd66525f
Author: Nate Chapin <japhet@chromium.org>
Date: Thu Nov 30 18:28:35 2017

Standardize ResourceClient data streaming callbacks on ResourceClient::DataReceived

This removes the sole purpose of ScriptResourceClient, so change its user to just
be plain ResourceClients.

Bug:  789198 
Change-Id: Id72163ad4c7e81492e9f74fa5f1ee68d2b510215
Reviewed-on: https://chromium-review.googlesource.com/792253
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520614}
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/dom/ClassicPendingScript.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/WorkletScriptLoader.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/WorkletScriptLoader.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/resource/ScriptResource.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/resource/ScriptResource.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/core/loader/resource/StyleSheetResourceClient.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp
[modify] https://crrev.com/5887f770941260df0da1395c391db5a3bd66525f/third_party/WebKit/Source/platform/loader/fetch/ResourceClient.h

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 1 2017

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

commit 818481e558c35b5fa2d5275a3481c94f1e8538d2
Author: Nate Chapin <japhet@chromium.org>
Date: Fri Dec 01 06:30:13 2017

Remove StyleSheetResourceClient and StyleSheetResource

* StyleSheetResourceClient has custom finish notifications for CSS and XSL stylesheets.
  Standardize these on NotifyFinished(), as provided by ResourceClient.
* StyleSheetResourceClient is now an empty wrapper around ResourceClient, switch
  StyleSheetResourceClient subclasses to use ResourceClient.
* StyleSheetResource is now a wrapper around TextResource that just specifies a
  ClientType. Have TextResource expect a ClientType of ResourceClient and remove
  StyleSheetResource.

Bug:  789198 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Iaa7ecdd916bad3f64aa3e35751a91a19ec1eceeb
Reviewed-on: https://chromium-review.googlesource.com/777782
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520875}
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/css/StyleRuleImport.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/css/StyleRuleImport.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/dom/ProcessingInstruction.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/dom/ProcessingInstruction.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/html/LinkStyle.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/html/LinkStyle.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/BUILD.gn
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.h
[delete] https://crrev.com/827b9e9a969c3a9079056dbf3786702b3d2e3681/third_party/WebKit/Source/core/loader/resource/StyleSheetResource.h
[delete] https://crrev.com/827b9e9a969c3a9079056dbf3786702b3d2e3681/third_party/WebKit/Source/core/loader/resource/StyleSheetResourceClient.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/resource/TextResource.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/resource/XSLStyleSheetResource.cpp
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/core/loader/resource/XSLStyleSheetResource.h
[modify] https://crrev.com/818481e558c35b5fa2d5275a3481c94f1e8538d2/third_party/WebKit/Source/platform/loader/fetch/ResourceClient.h

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 6 2017

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

commit 785c19f3bfd7d0953980e606df2514ef60ab0ef3
Author: Nate Chapin <japhet@chromium.org>
Date: Wed Dec 06 00:20:02 2017

Fix infinite recursion in css preload scanning

A self-referencing css stylesheet can attempt to preload itself reentrantly.
Ensure a HTMLResourcePreloader doesn't scan the same Resource twice.

Bug:  789198 ,  790940 ,  790945 
Change-Id: I5a5ca56e3c12978c4a8b7fcbba79ae2a772671f8
Reviewed-on: https://chromium-review.googlesource.com/804496
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521911}
[add] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/LayoutTests/http/tests/preload/css-import-scan-recursive-expected.txt
[add] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/LayoutTests/http/tests/preload/css-import-scan-recursive.html
[add] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/LayoutTests/http/tests/preload/resources/css-link-recurse.css
[modify] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp
[modify] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.cpp
[modify] https://crrev.com/785c19f3bfd7d0953980e606df2514ef60ab0ef3/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.h

Status: Fixed (was: Started)
I think that's all the flattening I have for the moment.

Sign in to add a comment