Issue metadata
Sign in to add a comment
|
Cannot download file in Beta or Canary
Reported by
hrlaws...@gmail.com,
Jan 31 2018
|
||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.119 Safari/537.36 Steps to reproduce the problem: 1. Download a file using blob 2. 3. What is the expected behavior? download file What went wrong? The webpage at blob:https://v360demo.xxxxxxxxxxxxxxxxxxxxx might be temporarily down or it may have moved permanently to a new web address. ERR_INVALID_RESPONSE Did this work before? Yes 64 Does this work in other browsers? Yes Chrome version: Version 65.0.3325.31 (Official Build) dev (64-bit) Channel: beta OS Version: 8 Flash Version: When using Chrome version 65 or 66 I cannot download a file using blob. I get the error:
,
Jan 31 2018
,
Jan 31 2018
,
Feb 1 2018
Able to reproduce the issue on Windows 10, mac 10.12.6 and Ubuntu 14.04 using chrome reported version #65.0.3325.31 and latest canary #66.0.3335.0. Issue is not seen in latest stable #64.0.3282.119. Note: Use the url: https://www.google.com/url?q=https://jsfiddle.net/koldev/cW7W5/&sa=D&ust=1517552741539000&usg=AFQjCNHjMtpzyMs0sxyavarFGGQ_MFTqHA to test the issue. Bisect Information: ===================== Good build: 65.0.3309.0 Bad Build : 65.0.3310.0 Change Log URL: https://chromium.googlesource.com/chromium/src/+log/cbce1149c9ae08297b816effbf456c02f97df8db..f2d2fe87028de36a489f7db3f5fb28da2e9d9b2b From the above change log suspecting below change Change-Id: I508d7abe1cba9b75d65132b0688984cdebfc6fd4 Reviewed-on: https://chromium-review.googlesource.com/758236 jochen@ - Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: Adding stable blocker for M-65 as it is a recent regression. Please feel free to remove the same if not appropriate. Thanks...!!
,
Feb 1 2018
thanks for the bug report. The issue is that you revoke the blob URL before the download is ready. Those two operations are racing - I guess that's also the reason why it sometimes works and sometimes doesn't jsbell@ can you chime in on the expected behavior here?
,
Feb 1 2018
i.e. is it expected that revoking an blob url cancels any ongoing async operation?
,
Feb 1 2018
It is expected that this works. But known that it doesn't... Or at least that there are various race conditions around this. We do have at least one browser test for this case even (https://cs.chromium.org/chromium/src/content/test/data/download/download-attribute-blob.html?q=download-attribute-blob.html)
,
Feb 1 2018
And yes, using the navigation path for this probably made it worse, since before at least things were more or less appropriately ordered for the blob URL to be resolved before the revoke IPC gets handled. I'm working on fixing all of this as part of blob URL mojofication, but especially fixing the navigation codepath is rather hard/complicated.
,
Feb 1 2018
(sorry for the repeated replies to myself) What I'm essentially (trying to) do to fix this for real is resolve blob URLs as soon as possible on the blink side, and then passing a resulting URLLoaderFactory along with the blob URL to wherever eventually the blob URL needs to get fetch. This special factory will keep the URL alive (and will be requested over the same mojo pipe as the later URL revoke), and thus no more race conditions possible. This is safe even for navigations since navigations to blob URLs are only allowed to same origin URLs, so even though the renderer can provide whatever content it wants for these URLs the browser can still check that the URL actually is same origin. The overall tracking bug for refactoring blob URL resolving/loading like this is issue 800901. But it's very unlikely that'll finish in time to turn on for M65, so wouldn't help directly for this bug.
,
Feb 1 2018
oh, and one more thing, in https://chromium-review.googlesource.com/c/chromium/src/+/894843 I'm adding tests for some related cases, which at least cover some of the navigation code path. The tests there I'm unfortunately adding as Pass/Failure expectations though, since, like this bug, chrome has flaky race conditions around navigation to blob URLs.
,
Feb 1 2018
Actually, only now noticed that we *had* a test for this case, until jochen@ removed that part of the test in the CL that is blamed for breaking this... Even though the test explicitly mentioned that it was trying to test this case (and still mentions that...)
,
Feb 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a3dd29b25f01eef7bf726aef971d554ed153a4f0 commit a3dd29b25f01eef7bf726aef971d554ed153a4f0 Author: Marijn Kruisselbrink <mek@chromium.org> Date: Fri Feb 02 19:21:44 2018 Bounce Blob URL revoking through the UI thread to give navigation a chance. Navigation IPCs are handled on the UI thread so if a blob URL is revoked immediately after navigating the revocation will almost always happen before the navigation code has had a chance to post a task back to the IO thread to actually perform the request. With this change revocation is hopefully delayed just long enough to make navigation work again. Bug: 807639 Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704 Reviewed-on: https://chromium-review.googlesource.com/899666 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Commit-Position: refs/heads/master@{#534119} [modify] https://crrev.com/a3dd29b25f01eef7bf726aef971d554ed153a4f0/content/browser/blob_storage/blob_dispatcher_host.cc [modify] https://crrev.com/a3dd29b25f01eef7bf726aef971d554ed153a4f0/content/test/data/download/download-attribute-blob.html
,
Feb 2 2018
Thank you!
,
Feb 2 2018
We probably still want to backport the fix to M65, assuming nothing disastrous happens with it in Canary/Dev, right?
,
Feb 2 2018
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
,
Feb 2 2018
Right, I'll request merge next Monday
,
Feb 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/16c3e9aee2a702a8df966f6d6bcb30ffb9e89268 commit 16c3e9aee2a702a8df966f6d6bcb30ffb9e89268 Author: Thomas Anderson <thomasanderson@chromium.org> Date: Sat Feb 03 01:24:12 2018 Revert "Bounce Blob URL revoking through the UI thread to give navigation a chance." This reverts commit a3dd29b25f01eef7bf726aef971d554ed153a4f0. Reason for revert: Causing DownloadContentTest.DownloadAttributeBlobURL to fail on Linux Tests, Linux Tests (dbg)(1), and Linux Tests (dbg)(1)(32). https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests/66991 https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29%2832%29/47823 https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/70044 Original change's description: > Bounce Blob URL revoking through the UI thread to give navigation a chance. > > Navigation IPCs are handled on the UI thread so if a blob URL is revoked > immediately after navigating the revocation will almost always happen before > the navigation code has had a chance to post a task back to the IO thread to > actually perform the request. With this change revocation is hopefully > delayed just long enough to make navigation work again. > > Bug: 807639 > Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704 > Reviewed-on: https://chromium-review.googlesource.com/899666 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> > Cr-Commit-Position: refs/heads/master@{#534119} TBR=mek@chromium.org,jochen@chromium.org Change-Id: Ia460136d52f214d3d2914a8004ebe65c87e90bc3 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 807639 Reviewed-on: https://chromium-review.googlesource.com/900442 Reviewed-by: Thomas Anderson <thomasanderson@chromium.org> Commit-Queue: Thomas Anderson <thomasanderson@chromium.org> Cr-Commit-Position: refs/heads/master@{#534230} [modify] https://crrev.com/16c3e9aee2a702a8df966f6d6bcb30ffb9e89268/content/browser/blob_storage/blob_dispatcher_host.cc [modify] https://crrev.com/16c3e9aee2a702a8df966f6d6bcb30ffb9e89268/content/test/data/download/download-attribute-blob.html
,
Feb 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a5561649b9567c394ef1ff1619745b02ed7759d commit 8a5561649b9567c394ef1ff1619745b02ed7759d Author: Reilly Grant <reillyg@chromium.org> Date: Sat Feb 03 04:37:45 2018 Reland "Bounce Blob URL revoking through the UI thread to give navigation a chance." This is a reland of a3dd29b25f01eef7bf726aef971d554ed153a4f0. Original change's description: > Bounce Blob URL revoking through the UI thread to give navigation a chance. > > Navigation IPCs are handled on the UI thread so if a blob URL is revoked > immediately after navigating the revocation will almost always happen before > the navigation code has had a chance to post a task back to the IO thread to > actually perform the request. With this change revocation is hopefully > delayed just long enough to make navigation work again. > > Bug: 807639 > Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704 > Reviewed-on: https://chromium-review.googlesource.com/899666 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> > Cr-Commit-Position: refs/heads/master@{#534119} TBR=mek@chromium.org,jochen@chromium.org Bug: 807639 , 808759 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I5033422d57ac7672e761045f7871c6aed29368b9 Reviewed-on: https://chromium-review.googlesource.com/900191 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/heads/master@{#534257} [modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/content/browser/blob_storage/blob_dispatcher_host.cc [modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/content/test/data/download/download-attribute-blob.html [modify] https://crrev.com/8a5561649b9567c394ef1ff1619745b02ed7759d/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
,
Feb 5 2018
,
Feb 5 2018
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 5 2018
Verified the fix on Mac 10.12.6, Win-10 and Ubuntu 14.04 using latest chrome version #66.0.3340.0 as per the comment #0 and #4. Attaching screen cast for reference. Observed that file got downloaded as expected. Hence, the fix is working as expected. Adding the verified labels. Thanks...!!
,
Feb 5 2018
Are you requesting a merge for cl listed at #18 to M65?
,
Feb 5 2018
right, the CL was relanded by the sheriff after it turned out not to be the culprit
,
Feb 5 2018
Approving merge to M65 for CL listed at #18 to M65. Please merge ASAP so we can pick it up for tomorrow's last M65 dev release. Thank you.
,
Feb 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b43f737ce7b421ca8717f604db045e7fbb4e7b3 commit 7b43f737ce7b421ca8717f604db045e7fbb4e7b3 Author: Jochen Eisinger <jochen@chromium.org> Date: Mon Feb 05 17:05:22 2018 Reland "Bounce Blob URL revoking through the UI thread to give navigation a chance." This is a reland of a3dd29b25f01eef7bf726aef971d554ed153a4f0. Original change's description: > Bounce Blob URL revoking through the UI thread to give navigation a chance. > > Navigation IPCs are handled on the UI thread so if a blob URL is revoked > immediately after navigating the revocation will almost always happen before > the navigation code has had a chance to post a task back to the IO thread to > actually perform the request. With this change revocation is hopefully > delayed just long enough to make navigation work again. > > Bug: 807639 > Change-Id: I2afde86dd203c61cbb03cc02ac62ec21c7e7d704 > Reviewed-on: https://chromium-review.googlesource.com/899666 > Reviewed-by: Jochen Eisinger <jochen@chromium.org> > Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> > Cr-Commit-Position: refs/heads/master@{#534119} TBR=jochen@chromium.org, mek@chromium.org, reillyg@chromium.org (cherry picked from commit 8a5561649b9567c394ef1ff1619745b02ed7759d) Bug: 807639 , 808759 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I5033422d57ac7672e761045f7871c6aed29368b9 Reviewed-on: https://chromium-review.googlesource.com/900191 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#534257} Reviewed-on: https://chromium-review.googlesource.com/902043 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/branch-heads/3325@{#302} Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369} [modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/content/browser/blob_storage/blob_dispatcher_host.cc [modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/content/test/data/download/download-attribute-blob.html [modify] https://crrev.com/7b43f737ce7b421ca8717f604db045e7fbb4e7b3/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
,
Feb 5 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by hrlaws...@gmail.com
, Jan 31 2018