New issue
Advanced search Search tips

Issue 793028 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Fetching a Resource should also add the requestor as a client

Project Member Reported by japhet@chromium.org, Dec 7 2017

Issue description

Currently, requesting a resource in blink and registering the caller as a client to listen to callbacks for that resource looks like:

SetResource(FooResource::Fetch(params, fetcher));

...where SetResource() is implemented by ResourceClient, sets ResourceClient::resource_ to the given resource, and if it is non-null, calls AddClient(this) on the resource.

After this, fetching should be:

FooResource::Fetch(params, fetcher, this);

...where Fetch handles all of the work of starting the network request, as well as adding the requestor as a client on the subsequent Resource and initializing the client's resource_ pointer.
 
Project Member

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

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

commit 524dc1e22fc49e06b5b37ff75ae3c29f28857d7f
Author: Nate Chapin <japhet@chromium.org>
Date: Fri Dec 08 17:10:26 2017

Have ResourceFetcher call SetResource() for RawResourceClients

Bug:  793028 
Change-Id: I6056322edb0732883395c7711533e5135dde6061
Reviewed-on: https://chromium-review.googlesource.com/814938
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522804}
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/html/imports/HTMLImportLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/html/imports/HTMLImportLoader.h
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/html/imports/HTMLImportsController.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/html/imports/HTMLImportsController.h
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/core/loader/TextTrackLoader.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/RawResource.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/ResourceClient.h
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h
[modify] https://crrev.com/524dc1e22fc49e06b5b37ff75ae3c29f28857d7f/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcherTest.cpp

Project Member

Comment 2 by bugdroid1@chromium.org, Dec 8 2017

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

commit a219af5085fea2b9f0c1546feae2ac9381b028f0
Author: Nate Chapin <japhet@chromium.org>
Date: Fri Dec 08 22:35:15 2017

Have ResourceFetcher call SetResource() for Script-loading ResourceClients

Bug:  793028 
Change-Id: I6675018045f17f9d153cbf86ebd4cfbc4ea2e84c
Reviewed-on: https://chromium-review.googlesource.com/815195
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522901}
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/dom/DocumentWriteIntervention.cpp
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/loader/modulescript/DocumentModuleScriptFetcher.cpp
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/loader/modulescript/DocumentModuleScriptFetcher.h
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/loader/resource/ScriptResource.cpp
[modify] https://crrev.com/a219af5085fea2b9f0c1546feae2ac9381b028f0/third_party/WebKit/Source/core/loader/resource/ScriptResource.h

Project Member

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

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

commit 659c36768b3b4de6a83cbe29bde6cdd81933a60b
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Dec 12 00:58:38 2017

Minimize direct use of ResourceClient::SetResource() and Resource::AddClient() in tests

Bug:  793028 
Change-Id: Icff8fce73e745969e4022329a7cb0374ce43698c
Reviewed-on: https://chromium-review.googlesource.com/818197
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523279}
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/BUILD.gn
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheTest.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcherTest.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/testing/MockResource.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/testing/MockResource.h
[delete] https://crrev.com/eff3260661d0e96a855fa075c32a13bfb2bd296e/third_party/WebKit/Source/platform/loader/testing/MockResourceClient.cpp
[modify] https://crrev.com/659c36768b3b4de6a83cbe29bde6cdd81933a60b/third_party/WebKit/Source/platform/loader/testing/MockResourceClient.h

Project Member

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

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

commit d5b1ba8cb7a39a2e38ccf7375f44771300773a7b
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Dec 12 17:08:42 2017

Have ResourceFetcher call SetResource() for stylesheet Resources

Bug:  793028 
Change-Id: I2c75fa078becfd638afe6072e9e2aa59198af66c
Reviewed-on: https://chromium-review.googlesource.com/815391
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523460}
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/css/StyleRuleImport.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/css/StyleRuleImport.h
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/dom/ProcessingInstruction.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/html/LinkStyle.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/loader/resource/CSSStyleSheetResource.h
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/loader/resource/XSLStyleSheetResource.cpp
[modify] https://crrev.com/d5b1ba8cb7a39a2e38ccf7375f44771300773a7b/third_party/WebKit/Source/core/loader/resource/XSLStyleSheetResource.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 12 2017

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

commit e346670f914f5c32e8784e840c97ecbb1f7957f8
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Dec 12 21:59:22 2017

Have ScriptStreamerTest go through ClassicPendingScript::Fetch()

This lets us get rid of the last non-trival usage of
ResourceClient::SetResource in ClassicPendingScript

Bug:  793028 
Change-Id: I39a4859cb1b941ff6a399fad88d40e353df4d462
Reviewed-on: https://chromium-review.googlesource.com/822176
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523569}
[modify] https://crrev.com/e346670f914f5c32e8784e840c97ecbb1f7957f8/third_party/WebKit/Source/bindings/core/v8/ScriptStreamerTest.cpp
[modify] https://crrev.com/e346670f914f5c32e8784e840c97ecbb1f7957f8/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/e346670f914f5c32e8784e840c97ecbb1f7957f8/third_party/WebKit/Source/core/dom/ClassicPendingScript.h

Project Member

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

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

commit d0969b07614d1ec9409a827578c0d0211cca0aa9
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Dec 12 23:25:06 2017

Get rid of default ResourceClient* param on ResourceFetcher::RequestResource

* DocumentResource has one easy case to plumb a ResourceClient, and one very
  difficult case that's already friend'ed for ResourceClient::SetResource
* ImageResource and LinkFetchResource never add clients, just always pass nullptr
  for the ResourceClient*.
* FontResource will be correctly plumbed in
  https://chromium-review.googlesource.com/c/chromium/src/+/818304, just use nullptr
  in the meantime.

Bug:  793028 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I49a20de543f4570ca2a8940a57bd22ff5c8f70cc
Reviewed-on: https://chromium-review.googlesource.com/822162
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523599}
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/loader/resource/DocumentResource.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/loader/resource/DocumentResource.h
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/loader/resource/FontResource.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/loader/resource/ImageResource.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/loader/resource/LinkFetchResource.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/svg/SVGElementProxy.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/core/svg/SVGUseElement.cpp
[modify] https://crrev.com/d0969b07614d1ec9409a827578c0d0211cca0aa9/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 12 2017

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

commit d816df282674d182a702ff20d79a09469ab08275
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Dec 12 23:48:50 2017

CSSPreloadResourceClient should register with Resource via ResourceFetcher

Bug:  793028 
Change-Id: Ice4ec0acdd179f2a7e2b17a497ee3bdf6e91fff4
Reviewed-on: https://chromium-review.googlesource.com/822916
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523607}
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/CSSPreloadScanner.h
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/HTMLPreloadScannerTest.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/HTMLResourcePreloader.h
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/PreloadRequest.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/html/parser/PreloadRequest.h
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/d816df282674d182a702ff20d79a09469ab08275/third_party/WebKit/Source/core/loader/LinkLoader.cpp

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18 2017

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

commit a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235
Author: Nate Chapin <japhet@chromium.org>
Date: Mon Dec 18 21:42:32 2017

SetResource() in ResourceFetcher for FontResources

* Take a FontResourceClient* in FontResource::Fetch
* All of the callers of SetResource are now ResourceClient friends,
  so make SetResource private.
* Rework RemoteFontFaceSource to be created before FontResource::Fetch
  is called. FontLoadHistograms needs to be initialized differently.
* Always have CSSFontFaceSrcValue register a FontResource with a
  FontResourceClient.
* The web fonts intervention no longer has sufficient data at the time
  of RemoteFontFaceSource creation (i.e., it does not know what url
  it will load, or whether it will be a cache hit). Initalize
  is_intervention_trigger_ just based on display_ and the connection
  state. When calculating whether to lower priority for the intervention
  in RemoteFontFaceSource::BeginLoadIfNeeded, let FontResource be
  responsible for checking its own state (i.e., whether the url is a
  data: url), and only expect RemoteFontFaceSource to be responsible
  for the state it can calculate at construction time.

Bug:  793028 
Change-Id: I3ebd0605be620c8fa0c798598656be289c719858
Reviewed-on: https://chromium-review.googlesource.com/818304
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524808}
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/css/CSSFontFaceSrcValue.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/css/CSSFontFaceSrcValue.h
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/css/FontFace.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/css/RemoteFontFaceSource.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/css/RemoteFontFaceSource.h
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/frame/FrameSerializer.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/resource/FontResource.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/resource/FontResource.h
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/resource/FontResourceTest.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/resource/MockFontResourceClient.cpp
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/core/loader/resource/MockFontResourceClient.h
[modify] https://crrev.com/a1bfc9f9cb531a1d5dd1e28b51a38b728ece1235/third_party/WebKit/Source/platform/loader/fetch/ResourceClient.h

Comment 9 by japhet@chromium.org, Jan 22 2018

Status: Fixed (was: Started)

Sign in to add a comment