New issue
Advanced search Search tips

Issue 804409 link

Starred by 1 user
Status: Started
Owner:
japhet@chromium.org
Cc:
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ResourceFetcher::RequestResource should always return non-null

Project Member Reported by japhet@chromium.org, Jan 22 2018 
Currently, we can fail a resource request by either returning a nullptr Resource from ResourceFetcher::RequestResource, or by populating a Resource with an error. 

https://chromium-review.googlesource.com/c/chromium/src/+/858257 will standardize all failures on a non-null Resource with an error.

This will likely enable some additional cleanup opportunities, once we can assume that there is only one failure codepath.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 23 2018 

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

commit 266b4fe6f8a70199444b98207a32cffb8e8fd39a
Author: Nate Chapin <japhet@chromium.org>
Date: Tue Jan 23 00:02:52 2018

ResourceFetcher::RequestResource() should never return nullptr

This means FooResource::Fetch callers that provide a ResourceClient can guarantee
that they will receive a NotifyFinished callback (though that NotifyFinished may
still be called synchronously). This simplifies many Fetch() callers.

Bug: 804409
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I9bee394d7837a8ee65fe23e98f923149a522e78d
Reviewed-on: https://chromium-review.googlesource.com/858257
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531069}
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/LayoutTests/fast/workers/chromium/worker-crash-with-invalid-location-expected.txt
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/LayoutTests/http/tests/navigation/ping-cross-origin-from-https-expected.txt
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/LayoutTests/mhtml/resource_not_found_in_archive-expected.html
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/LayoutTests/mhtml/resource_not_found_in_archive.mht
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/CSSFontFaceSrcValue.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/CSSFontFaceSrcValue.h
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/CSSImageSetValue.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/CSSImageValue.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/FontFace.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/css/StyleRuleImport.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/dom/ProcessingInstruction.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/frame/FrameSerializer.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/frame/FrameSerializer.h
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/html/imports/HTMLImportsController.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/html/parser/CSSPreloadScannerTest.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/inspector/InspectorResourceContentLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/DocumentThreadableLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/ImageLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/PingLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/modulescript/DocumentModuleScriptFetcher.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/resource/FontResourceTest.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/loader/resource/XSLStyleSheetResource.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/script/ClassicPendingScript.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/script/ScriptLoader.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/script/ScriptLoader.h
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/svg/SVGFEImageElement.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/xml/XSLStyleSheetLibxslt.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/xml/XSLTProcessorLibxslt.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/platform/loader/fetch/MemoryCache.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h
[modify] https://crrev.com/266b4fe6f8a70199444b98207a32cffb8e8fd39a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcherTest.cpp
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 1 2018 

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

commit ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d
Author: Nate Chapin <japhet@chromium.org>
Date: Thu Feb 01 22:22:23 2018

Always assume ImageResourceConent::Fetch() will return non-null in core/css/

This removes the last callers of StyleInvalidImage, so delete it.

Bug: 804409
Change-Id: I0b7cbfd9c764c7255ad7b22212add3c125902b39
Reviewed-on: https://chromium-review.googlesource.com/879409
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533837}
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/css/CSSImageSetValue.cpp
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/css/CSSImageValue.cpp
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/style/BUILD.gn
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/style/StyleFetchedImage.cpp
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/style/StyleFetchedImage.h
[modify] https://crrev.com/ffb7e2a2d2b82525360be8cb96e5bb0ce45a238d/third_party/WebKit/Source/core/style/StyleImage.h
[delete] https://crrev.com/74b3d06052ba093cf7ca45afb4a5ede5d7b10e73/third_party/WebKit/Source/core/style/StyleInvalidImage.h

Comment 3 by ricea@chromium.org, Feb 2 2018

Components: Blink>Loader

Comment 4 by woxxom@gmail.com, Mar 12 2018 

japhet@ PTAL at the attached mhtml - its CSS is not applied since your r522861 according to my bisect:
https://chromium.googlesource.com/chromium/src/+log/ce3cea47..06d36e78?pretty=fuller
Works correctly in Chrome 64, but not in 65+.
test.zip
170 KB Download
►

Sign in to add a comment