Fetch Responses automatically read blobs, but they shouldn't |
||
Issue description
Consider the following script:
caches.match("http://example.com/")
.then(function() {
console.log("tada!");
});
Since the resolving method doesn't use the response object, you'd expect that it wouldn't read the response blob, but it does. All Responses start reading their blobs. Presumably this also means that respondWith(caches.match("http://example.com/")) also causes unnecessary blob reads.
Response object creation includes the creation of a FetchBlobDataConsumerHandle, which creates a ReaderContext and when the context is created a NotifyOnReaderCreationHelper calls BodyStreamBuffer::didGetReadable() which starts reading the blob.
,
Sep 1 2016
Do we need to create all of these objects on Response creation? Can we just hang onto a blob handle and then create the reader machinery when the blob is actually read?
,
Sep 1 2016
Assigning to yhirano@ as I don't have much expertise in this area. Please reassign to someone else (or me) as appropriate. I'm happy to take it on if necessary.
,
Sep 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f1ca96c413912ff1768e4274b8b7ac78eb41de64 commit f1ca96c413912ff1768e4274b8b7ac78eb41de64 Author: yhirano <yhirano@chromium.org> Date: Wed Sep 07 13:08:07 2016 [Fetch API] Start reading blob only when the reader wants to read contents. BUG= 643174 Review-Url: https://codereview.chromium.org/2303753003 Cr-Commit-Position: refs/heads/master@{#416923} [modify] https://crrev.com/f1ca96c413912ff1768e4274b8b7ac78eb41de64/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandle.cpp [modify] https://crrev.com/f1ca96c413912ff1768e4274b8b7ac78eb41de64/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandleTest.cpp [modify] https://crrev.com/f1ca96c413912ff1768e4274b8b7ac78eb41de64/third_party/WebKit/Source/modules/fetch/FetchFormDataConsumerHandleTest.cpp
,
Sep 7 2016
Thanks yhirano!
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cbd2420c74783570442264a3e9a1c76cc42152e commit 2cbd2420c74783570442264a3e9a1c76cc42152e Author: lushnikov <lushnikov@chromium.org> Date: Thu Sep 08 17:58:51 2016 Revert of [Fetch API] Start reading blob only when the reader wants to read contents. (patchset #3 id:40001 of https://codereview.chromium.org/2303753003/ ) Reason for revert: Speculative revert: this patch is a suspect to cause Linux MSAN failures: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20MSAN/builds/12091 The other suspected patch, the only one related to canvas, was reverted and didn't help. Original issue's description: > [Fetch API] Start reading blob only when the reader wants to read contents. > > BUG= 643174 > > Committed: https://crrev.com/f1ca96c413912ff1768e4274b8b7ac78eb41de64 > Cr-Commit-Position: refs/heads/master@{#416923} TBR=hiroshige@chromium.org,jkarlin@chromium.org,yhirano@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 643174 Review-Url: https://codereview.chromium.org/2319093004 Cr-Commit-Position: refs/heads/master@{#417338} [modify] https://crrev.com/2cbd2420c74783570442264a3e9a1c76cc42152e/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandle.cpp [modify] https://crrev.com/2cbd2420c74783570442264a3e9a1c76cc42152e/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandleTest.cpp [modify] https://crrev.com/2cbd2420c74783570442264a3e9a1c76cc42152e/third_party/WebKit/Source/modules/fetch/FetchFormDataConsumerHandleTest.cpp
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a043096ac664aecc87fc5eb207c2ca84e892422e commit a043096ac664aecc87fc5eb207c2ca84e892422e Author: lushnikov <lushnikov@chromium.org> Date: Thu Sep 08 20:51:19 2016 Reland of [Fetch API] Start reading blob only when the reader wants to read contents. (patchset #1 id:1 of https://codereview.chromium.org/2319093004/ ) Reason for revert: With the revert in place, the Linux MSAN is still failing: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20MSAN/builds/12113 This proofs this patch to be innocent. Original issue's description: > Revert of [Fetch API] Start reading blob only when the reader wants to read contents. (patchset #3 id:40001 of https://codereview.chromium.org/2303753003/ ) > > Reason for revert: > Speculative revert: this patch is a suspect to cause Linux MSAN failures: > > https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20MSAN/builds/12091 > > The other suspected patch, the only one related to canvas, was reverted and didn't help. > > Original issue's description: > > [Fetch API] Start reading blob only when the reader wants to read contents. > > > > BUG= 643174 > > > > Committed: https://crrev.com/f1ca96c413912ff1768e4274b8b7ac78eb41de64 > > Cr-Commit-Position: refs/heads/master@{#416923} > > TBR=hiroshige@chromium.org,jkarlin@chromium.org,yhirano@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 643174 > > Committed: https://crrev.com/2cbd2420c74783570442264a3e9a1c76cc42152e > Cr-Commit-Position: refs/heads/master@{#417338} TBR=hiroshige@chromium.org,jkarlin@chromium.org,yhirano@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 643174 Review-Url: https://codereview.chromium.org/2327583002 Cr-Commit-Position: refs/heads/master@{#417391} [modify] https://crrev.com/a043096ac664aecc87fc5eb207c2ca84e892422e/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandle.cpp [modify] https://crrev.com/a043096ac664aecc87fc5eb207c2ca84e892422e/third_party/WebKit/Source/modules/fetch/FetchBlobDataConsumerHandleTest.cpp [modify] https://crrev.com/a043096ac664aecc87fc5eb207c2ca84e892422e/third_party/WebKit/Source/modules/fetch/FetchFormDataConsumerHandleTest.cpp |
||
►
Sign in to add a comment |
||
Comment 1 Deleted