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

Issue 2173003002: Load data URI images in an async way according to spec (Closed)

Created:
4 years, 5 months ago by Yoav Weiss
Modified:
4 years, 4 months ago
CC:
chromium-reviews, tyoshino+watch_chromium.org, blink-reviews-html_chromium.org, loading-reviews_chromium.org, dglazkov+blink, gavinp+loader_chromium.org, blink-reviews, kinuko+watch, Nate Chapin, kouhei (in TOK), mdjones, nyquist
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Load data URI images in an async way according to spec According to https://github.com/ResponsiveImagesCG/picture-element/issues/223#issuecomment-69772765 we seem to be the only browser that loads data URIs immediately (as if they were cached). It was introduced as part of the move to async image loading: https://codereview.chromium.org/200923002 This is now causing issues: https://code.google.com/p/chromium/issues/detail?id=469131 This CL changes this behavior to be spec compliant and load data URIs asynchronously, like other resources. This is relanding https://codereview.chromium.org/1492303003 BUG=514206 Committed: https://crrev.com/d0c9ac3bf5bb4843c2c189a72e7b6c39e6743831 Cr-Commit-Position: refs/heads/master@{#407616}

Patch Set 1 #

Patch Set 2 : new expected result #

Patch Set 3 : Really added expected result #

Patch Set 4 : perf test config #

Patch Set 5 : removed test config #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -84 lines) Patch
M third_party/WebKit/LayoutTests/fast/canvas/canvas-drawImage-out-of-bounds-src.html View 2 chunks +12 lines, -10 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/HTMLImageElement/image-async-loading-data-uris.html View 1 chunk +10 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/forms/basic-buttons.html View 1 chunk +4 lines, -2 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/security/contentSecurityPolicy/report-blocked-data-uri-expected.txt View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
A + third_party/WebKit/LayoutTests/loader/data-uri-images-load-asynchronously.html View 2 chunks +3 lines, -3 lines 0 comments Download
A third_party/WebKit/LayoutTests/loader/data-uri-images-load-asynchronously-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/loader/data-uri-images-load-synchronously.html View 1 chunk +0 lines, -18 lines 0 comments Download
D third_party/WebKit/LayoutTests/loader/data-uri-images-load-synchronously-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
A + third_party/WebKit/LayoutTests/loader/data-uri-images-reload-asynchronously.html View 2 chunks +12 lines, -6 lines 0 comments Download
A third_party/WebKit/LayoutTests/loader/data-uri-images-reload-asynchronously-expected.txt View 1 chunk +2 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/loader/data-uri-images-reload-synchronously.html View 1 chunk +0 lines, -28 lines 0 comments Download
D third_party/WebKit/LayoutTests/loader/data-uri-images-reload-synchronously-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/svg/canvas/canvas-draw-image-globalalpha.html View 1 chunk +5 lines, -3 lines 0 comments Download
M third_party/WebKit/LayoutTests/svg/canvas/image-svg-intrinsic-size.html View 1 chunk +7 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/parser/HTMLSrcsetParser.cpp View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/ImageLoader.cpp View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/web/tests/ActivityLoggerTest.cpp View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 37 (15 generated)
Yoav Weiss
Hey! This is an attempt to reland https://codereview.chromium.org/1492303003 which got reverted for (apparently unrelated) perf ...
4 years, 5 months ago (2016-07-22 08:18:25 UTC) #2
Charlie Harrison
Do you mind running the new page_cyclers with this change, just in case? kouhei@ has ...
4 years, 5 months ago (2016-07-22 12:05:40 UTC) #3
Yoav Weiss
On 2016/07/22 12:05:40, csharrison wrote: > Do you mind running the new page_cyclers with this ...
4 years, 5 months ago (2016-07-22 12:19:51 UTC) #4
Yoav Weiss
On 2016/07/22 12:19:51, Yoav Weiss wrote: > On 2016/07/22 12:05:40, csharrison wrote: > > Do ...
4 years, 5 months ago (2016-07-22 12:20:38 UTC) #5
Charlie Harrison
On 2016/07/22 12:19:51, Yoav Weiss wrote: > On 2016/07/22 12:05:40, csharrison wrote: > > Do ...
4 years, 5 months ago (2016-07-22 12:24:03 UTC) #6
Charlie Harrison
On 2016/07/22 12:24:03, csharrison wrote: > On 2016/07/22 12:19:51, Yoav Weiss wrote: > > On ...
4 years, 5 months ago (2016-07-22 12:24:32 UTC) #7
Yoav Weiss
CCing mdjones@ and nyquist@ as git blame points their way. Could you perhaps take a ...
4 years, 5 months ago (2016-07-22 14:34:35 UTC) #8
nyquist
Adding wychen as well.
4 years, 5 months ago (2016-07-22 15:52:51 UTC) #10
Yoav Weiss
On 2016/07/22 15:52:51, nyquist wrote: > Adding wychen as well. Thanks to nyquist@ who pointed ...
4 years, 5 months ago (2016-07-22 16:13:49 UTC) #11
pdr.
Still LGTM for the code change.
4 years, 5 months ago (2016-07-22 16:28:02 UTC) #12
wychen
On 2016/07/22 16:13:49, Yoav Weiss wrote: > wychen@ - can you advise on the best ...
4 years, 5 months ago (2016-07-22 17:47:28 UTC) #13
wychen
On 2016/07/22 17:47:28, wychen wrote: > On 2016/07/22 16:13:49, Yoav Weiss wrote: > > wychen@ ...
4 years, 5 months ago (2016-07-22 18:26:37 UTC) #14
wychen
On 2016/07/22 18:26:37, wychen wrote: > On 2016/07/22 17:47:28, wychen wrote: > > On 2016/07/22 ...
4 years, 5 months ago (2016-07-22 22:41:37 UTC) #15
Yoav Weiss
On 2016/07/22 22:41:37, wychen wrote: > On 2016/07/22 18:26:37, wychen wrote: > > On 2016/07/22 ...
4 years, 4 months ago (2016-07-25 12:37:50 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2173003002/60001
4 years, 4 months ago (2016-07-25 12:38:07 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/224191)
4 years, 4 months ago (2016-07-25 12:44:24 UTC) #24
Charlie Harrison
Don't commit the perf config. That's just for trybots. The system is pretty confusing :(
4 years, 4 months ago (2016-07-25 13:22:13 UTC) #25
Yoav Weiss
On 2016/07/25 13:22:13, csharrison wrote: > Don't commit the perf config. That's just for trybots. ...
4 years, 4 months ago (2016-07-25 14:05:54 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2173003002/80001
4 years, 4 months ago (2016-07-25 22:20:22 UTC) #33
commit-bot: I haz the power
Committed patchset #5 (id:80001)
4 years, 4 months ago (2016-07-25 22:24:37 UTC) #34
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/d0c9ac3bf5bb4843c2c189a72e7b6c39e6743831 Cr-Commit-Position: refs/heads/master@{#407616}
4 years, 4 months ago (2016-07-25 22:27:02 UTC) #36
Yoav Weiss
4 years, 4 months ago (2016-08-03 04:11:27 UTC) #37
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2201313002/ by yoav@yoav.ws.

The reason for reverting is: Reverting due to
https://bugs.chromium.org/p/chromium/issues/detail?id=632277

Seems like the dev tools' screen capture functionality relies on the sync
bahavior of data URIs (issue reproduces with the patch, and does not when
reverted)

R=pdr, csharrison.

Powered by Google App Engine
This is Rietveld 408576698