New issue
Advanced search Search tips

Issue 881314 link

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



Sign in to add a comment

Background Fetch resumes properly on browser start-up

Project Member Reported by rayankans@chromium.org, Sep 6 
This is a master bug to track the overall progress of this feature.

Some initial issues to tackle though:
- Browser crashes on start-up if there is a resuming download
- When an interrupted (resuming) download is complete, the header / url chain info is not available

Feature requests to make this completely work:
- Have the Background Fetch client control when a download resumes
- Ability to access the download info via guid after the download is complete (in case the browser crashes in between the download completing and storing the response).
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 12 

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

commit de0b3ae1551dd78731d028af9f8b45cbce617ad3
Author: Rayan Kanso <rayankans@chromium.org>
Date: Wed Sep 12 19:17:59 2018

[Background Fetch] Resume fetch correctly on browser restart.

A few changes were added to make sure that this works end-to-end:
- Remove a DCHECK in download_item_impl.cc that caused the browser to
crash on restart if a download was being resumed.
- Add an appropriate callback to the Controller to handle the completed
download.
- Remove redundant calls to Resume in the delegate.

With this change the fetch will continue on restart, though it will
fail due to missing header / url chain info.

Bug: 881314
Change-Id: I34de75fa7cd8284672ed7de7b63297d5b3edf138
Reviewed-on: https://chromium-review.googlesource.com/1210062
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590778}
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/chrome/browser/background_fetch/background_fetch_delegate_impl.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/chrome/browser/background_fetch/background_fetch_delegate_impl.h
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/chrome/browser/background_fetch/background_fetch_download_client.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/components/download/internal/common/download_item_impl.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_cross_origin_filter.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_job_controller.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_job_controller.h
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_scheduler.cc
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_scheduler.h
[modify] https://crrev.com/de0b3ae1551dd78731d028af9f8b45cbce617ad3/content/browser/background_fetch/background_fetch_scheduler_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 17 

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

commit ee615c54c537ef2f3f58fa761ef4e81384b7548c
Author: Rayan Kanso <rayankans@chromium.org>
Date: Mon Sep 17 12:24:42 2018

Store response headers and url chain in download::Entry

This allows for downloads that resume on startup to still have access to
that information.

Bug: 883359, 881314
Change-Id: I65ff163180f8b834ca59fadba0f45d8536d13be5
Reviewed-on: https://chromium-review.googlesource.com/1222893
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591649}
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/controller_impl.cc
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/entry.cc
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/entry.h
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/entry_utils.cc
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/proto/entry.proto
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/proto_conversions.cc
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/internal/background_service/test/entry_utils.cc
[modify] https://crrev.com/ee615c54c537ef2f3f58fa761ef4e81384b7548c/components/download/public/background_service/download_metadata.h
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 19 

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

commit 44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07
Author: Xing Liu <xingliu@chromium.org>
Date: Wed Sep 19 02:32:04 2018

Background download: Retry when no response header is persisted.

We persist download response headers and url chain when
OnDownloadCreated is called. This CL does the following:

1. If the persist failed during ACTIVE state, delete the driver and the
file to let the download service to retry.

2. Change DownloadDriver::Remove to be able to remove file for completed
download.

3. Add did_receive_response in entry proto, since some protocol or
a socket error may cause the response headers to be empty. We should
validate the entry state with this flag instead of response headers.

TBR=carlosk@chromium.org


Bug: 883359, 881314
Change-Id: I560cc478b5d1f12c33b56b94ee97cbb1d2b76c3d
Reviewed-on: https://chromium-review.googlesource.com/1227290
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592306}
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/content/internal/download_driver_impl.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/content/internal/download_driver_impl.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/content/internal/download_driver_impl_unittest.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/controller_impl.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/controller_impl_unittest.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/download_driver.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/entry.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/entry.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/entry_utils.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/in_memory_download_driver.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/in_memory_download_driver.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/in_memory_download_driver_unittest.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/proto/entry.proto
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/proto_conversions.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/test/entry_utils.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/test/test_download_driver.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/internal/background_service/test/test_download_driver.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/public/background_service/download_metadata.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/public/background_service/download_metadata.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/public/background_service/test/test_download_service.cc
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/download/public/common/download_item.h
[modify] https://crrev.com/44dfb9f4a9439707d5a1e8a8bb92fa8f9fd15a07/components/offline_pages/core/prefetch/test_download_service.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21 

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

commit 66aa370d4ea68d4958940c604a642f67459cfa27
Author: Rayan Kanso <rayankans@chromium.org>
Date: Fri Sep 21 11:37:41 2018

Provide URL chain and response headers on download completion

Pass a CompletionInfo to the OnDownloadFailed client observer,
and populate the CompletionInfo with the URL chain and response in both
cases.

Bug: 881314
Change-Id: I4a3609aabd5c7a34429414c43fb0744ba678c070
Reviewed-on: https://chromium-review.googlesource.com/1228914
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593149}
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/chrome/browser/background_fetch/background_fetch_download_client.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/chrome/browser/background_fetch/background_fetch_download_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/chrome/browser/offline_pages/prefetch/offline_prefetch_download_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/internal/background_service/controller_impl.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/internal/background_service/controller_impl.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/internal/background_service/controller_impl_unittest.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/internal/background_service/debugging_client.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/internal/background_service/debugging_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/download_metadata.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/download_metadata.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/test/empty_client.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/test/empty_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/test/mock_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/test/test_download_service.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/download/public/background_service/test/test_download_service.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/offline_pages/core/prefetch/test_download_client.cc
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/components/offline_pages/core/prefetch/test_download_client.h
[modify] https://crrev.com/66aa370d4ea68d4958940c604a642f67459cfa27/content/shell/browser/layout_test/layout_test_background_fetch_delegate.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21 

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

commit 8a0eece29a41a3b19fa11c5e37b43284a705c5b3
Author: Rayan Kanso <rayankans@chromium.org>
Date: Fri Sep 21 13:33:21 2018

[Background Fetch] Handle start-up resumption cases

Update the BackgroundFetchDownloadClient to handle the new
DownloadService contract. The Background Fetch scheduler will attempt to
start all active downloads on start-up. The ones that are actually resuming will
be ignored by the client.

Since the response metadata and the actual response are propagated in
different calls, the response metadata will be propagated twice to
account for restarts. This will be fixed in a follow-up CL where all of the information
associated with the response will be sent once to the Background Fetch scheduler.

Bug: 881314
Change-Id: I4d806c0eec2cb9c35a42da568613fb2d5ede856e
Reviewed-on: https://chromium-review.googlesource.com/1227995
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593164}
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/chrome/browser/background_fetch/background_fetch_delegate_impl.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/chrome/browser/background_fetch/background_fetch_delegate_impl.h
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/chrome/browser/background_fetch/background_fetch_download_client.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/chrome/browser/background_fetch/background_fetch_download_client.h
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/browser/background_fetch/background_fetch_job_controller.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/browser/background_fetch/background_fetch_job_controller.h
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/browser/background_fetch/background_fetch_scheduler.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/browser/background_fetch/background_fetch_scheduler.h
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/browser/background_fetch/background_fetch_scheduler_unittest.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/public/browser/background_fetch_description.cc
[modify] https://crrev.com/8a0eece29a41a3b19fa11c5e37b43284a705c5b3/content/public/browser/background_fetch_description.h
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 21 

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

commit d52eaaf17dcc98e43ba44edf768cc2a745c8c504
Author: Rayan Kanso <rayankans@chromium.org>
Date: Fri Sep 21 17:04:36 2018

[Background Fetch] Propagate headers and url chain on completion

With the recent changes in the Download Service, we should use the
response headers and request url chain provided when the download is
complete.

Bug: 881314
Change-Id: I3291186c9b867130683b3b09ef9c1d226d0089f1
Reviewed-on: https://chromium-review.googlesource.com/1236274
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593234}
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/chrome/browser/background_fetch/background_fetch_delegate_impl.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/chrome/browser/background_fetch/background_fetch_delegate_impl.h
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/chrome/browser/background_fetch/background_fetch_download_client.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_cross_origin_filter.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_delegate_proxy.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_delegate_proxy_unittest.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_job_controller.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_request_info.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/background_fetch_request_info.h
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/mock_background_fetch_delegate.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/browser/background_fetch/storage/mark_request_complete_task.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/public/browser/background_fetch_response.cc
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/public/browser/background_fetch_response.h
[modify] https://crrev.com/d52eaaf17dcc98e43ba44edf768cc2a745c8c504/content/shell/browser/layout_test/layout_test_background_fetch_delegate.cc
►

Sign in to add a comment