Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(229)

Issue 1757083002: [SVG 1/4] Move Resource::finish() for data URLs from requestResource() to Resource::load()

Created:
4 years, 9 months ago by hiroshige
Modified:
4 years, 8 months ago
Reviewers:
CC:
chromium-reviews, tyoshino+watch_chromium.org, Yoav Weiss, gavinp+loader_chromium.org, blink-reviews, loading-reviews+fetch_chromium.org, Nate Chapin
Base URL:
https://chromium.googlesource.com/chromium/src.git@SVG_new0b
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Move Resource::finish() for data URLs from requestResource() to Resource::load() Previously, for both SubstituteData and data URLs, 1. Resource and ResourceResponse are created and 2. Resource::finish() is always called in ResourceFetcher::resourceForStaticData() called from requestResource(). However, FontResource and ImageResource don't always start loading in requestResource(), and thus Resource::finish() shouldn't be called in such cases. This CL 1. Creates Resource for a data URL by createResourceForLoading() called in the main path of requestResource(), and 2. Creates ResourceResponse and calls Resource::finish() in Resource::load(). To do this, most of ResourceFetcher::resourceForStaticData() is moved into Resource::loadLocallyStaticData(), loadLocallyDataURL() and loadLocallySubstituteData(). Resource for SubstituteData is still created in a separated path in requestResource() and Resource::finish() is still called inside requestResource(), because this is not a problem for SubstituteData. This CL is [1/4] to load FontResource of data: URLs synchronously so that all subresource loading of SVG image completes before loading of the SVG file. BUG=382170

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Temp debug. #

Patch Set 4 : auto-Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -64 lines) Patch
M third_party/WebKit/Source/core/fetch/ImageResource.cpp View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/fetch/Resource.h View 1 2 3 chunks +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/Resource.cpp View 1 2 3 3 chunks +71 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/ResourceFetcher.h View 1 1 chunk +1 line, -3 lines 0 comments Download
M third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp View 1 2 3 5 chunks +15 lines, -60 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 8 (5 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1757083002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1757083002/1
4 years, 9 months ago (2016-03-02 22:36:59 UTC) #3
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/183097)
4 years, 9 months ago (2016-03-02 23:50:01 UTC) #6
commit-bot: I haz the power
4 years, 9 months ago (2016-03-03 00:16:42 UTC) #8

Powered by Google App Engine
This is Rietveld 408576698