Cancelling between StartCallback and Client::OnDownloadStarted causes problems |
||||
Issue descriptionWhat steps will reproduce the problem? (1) Call DownloadService->StartDownload (2) Wait for StartCallback to be called (with ACCEPTED). (3) Call DownloadService->Abort before Client::OnDownloadStarted (4) Call DownloadService->StartDownload (5) Wait What is the expected result? StartCallback should run and then Client::OnDownloadStarted should run, followed by the download completing as normal. What happens instead? StartCallback is called with ACCEPTED, but Client::OnDownloadStarted never runs. Aborting this new download and trying again doesn't help. chrome://download-internals lists the download as permanently in-progress but it never progresses.
,
Oct 20 2017
Can you upload the CL? It'd be great to see the repro steps thanks!
,
Oct 20 2017
I think now you can repro with what's in HEAD and on Linux. You need to enable the Experimental Web Platform features flag and then go to https://tests.peter.sh/random/background-fetch-content.html Click Start Background Fetch to start a download and it should complete in 1-2 seconds. To actually trigger the bug, click it several times in quick succession and it should trigger. You actually want to do this an odd number of times because I think clicking it when it's in progress, just aborts it and doesn't start it again. (The octopus icon means it thinks it's aborted).
,
Oct 24 2017
Debugging this further, i have it slightly wrong. It's doesn't go wrong immediately but goes wrong for the 2nd download after the cancel. StartCallback: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd, 0 OnDownloadStarted: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd OnDownloadUpdated: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd : 0 OnDownloadUpdated: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd : 0 OnDownloadUpdated: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd : 17644 OnDownloadUpdated: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd : 17644 OnDownloadUpdated: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd : 17644 OnDownloadSucceeded: 5d2e15f0-5ebe-49f9-b6b4-50cec76966bd StartCallback: 9899e7a6-043b-4a4e-81b6-2e786b1687ed, 0 ControllerImpl::CancelDownload: 9899e7a6-043b-4a4e-81b6-2e786b1687ed: 3 0x17a907e28620 Entry::State: 2 OnDownloadFailed: 9899e7a6-043b-4a4e-81b6-2e786b1687ed : 2 StartCallback: 4745a402-0fa0-4220-9ec0-98e259db3fd8, 0 OnDownloadStarted: 4745a402-0fa0-4220-9ec0-98e259db3fd8 OnDownloadUpdated: 4745a402-0fa0-4220-9ec0-98e259db3fd8 : 0 OnDownloadUpdated: 4745a402-0fa0-4220-9ec0-98e259db3fd8 : 17644 OnDownloadUpdated: 4745a402-0fa0-4220-9ec0-98e259db3fd8 : 17644 OnDownloadUpdated: 4745a402-0fa0-4220-9ec0-98e259db3fd8 : 17644 OnDownloadUpdated: 4745a402-0fa0-4220-9ec0-98e259db3fd8 : 17644 OnDownloadSucceeded: 4745a402-0fa0-4220-9ec0-98e259db3fd8 OnDownloadReceived: f82faf6c-1ddb-41de-90c4-b008f37b534e, 0 ... <Neither OnDownloadStarted nor any other Client method is called> I've logged a little bit of info from download::ControllerImpl::CancelDownload.
,
Oct 25 2017
I think perhaps the bug is at https://cs.chromium.org/chromium/src/components/download/internal/controller_impl.cc?q=internal/controller_impl&sq=package:chromium&dr=C&l=402 OnDownloadCreated is called after DownloadManager calls to say the download started. In this case the download has already been cancelled so entry is nullptr, which means it does: HandleExternalDownload(download.guid, true); Which adds that download to externally_active_downloads rather than removing it. Since nothing ever removes it again, it forever causes ControllerImpl::UpdateDriverState to set blocked_by_downloads to true. Changing true -> false removes the download and means the download queue doesn't get blocked any more. However I still see strange behaviour for the following download actually downloads twice.
,
Oct 25 2017
Thanks for the debugging, I can repro this.
,
Nov 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fee6aea330ee3a9c96873172afb9119ed3e3b8fb commit fee6aea330ee3a9c96873172afb9119ed3e3b8fb Author: Xing Liu <xingliu@chromium.org> Date: Wed Nov 01 18:00:03 2017 Download service: fix an issue when calling CancelDownload. CancelDownload can be called before content layer persist the record in history db, the canceled download will be recognized as external and thus block all background download. This CL filtered the canceled download with an existing cache in download driver. TBR=phajdan.jr@chromium.org Bug: 776716 Change-Id: Ib6476180a4b7db728fa6750264f0893384eb6633 Reviewed-on: https://chromium-review.googlesource.com/738589 Commit-Queue: Xing Liu <xingliu@chromium.org> Reviewed-by: David Trainor <dtrainor@chromium.org> Cr-Commit-Position: refs/heads/master@{#513177} [modify] https://crrev.com/fee6aea330ee3a9c96873172afb9119ed3e3b8fb/components/download/content/internal/download_driver_impl.cc [modify] https://crrev.com/fee6aea330ee3a9c96873172afb9119ed3e3b8fb/components/download/content/internal/download_driver_impl_unittest.cc [modify] https://crrev.com/fee6aea330ee3a9c96873172afb9119ed3e3b8fb/content/public/test/fake_download_item.cc [modify] https://crrev.com/fee6aea330ee3a9c96873172afb9119ed3e3b8fb/content/public/test/fake_download_item.h
,
Nov 3 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by delph...@chromium.org
, Oct 20 2017