New issue
Advanced search Search tips

Issue 692544 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Extend BackgroundFetch to work for multiple files in a logical batch

Project Member Reported by harkness@chromium.org, Feb 15 2017

Issue description

The BackgroundFetch API provides developers with the ability to make multiple requests as part of a single logical batch. The implementation will be sending individual files to the DownloadManager for fetch, but it may not be able to send all of them simultaneously.

There needs to be a component which keeps track of which files are associated with which batches and dispatches new files to the DownloadManager when previous ones complete. 

This component will also be responsible for displaying notifications for the batched fetch in progress. If possible, that will use a user-specified size estimate (and this component will be responsible for tracking that estimate against actual sizes received) but as a fall back, it may be as a percentage of total files received.

This may live in /background_fetch or possibly in /downloads.
 
Cc: mlamouri@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 28 2017

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

commit a247d9bf7b2082393dd172c0d78c33b044d646e2
Author: harkness <harkness@chromium.org>
Date: Tue Feb 28 17:01:53 2017

Create the BackgroundFetchBatchManager and initiate a download.

The BatchManager will be the component in the BackgroundFetch service
that takes care of tracking which particular downloads in batch have
completed and which ones are still in progress.

This CL adds the framework for the BatchManager as well as the new
BatchRequest skeleton which collects multiple FetchRequests into a
single batch.

Currently the BatchManager just starts a download, it's not doing any
observation of the download system. That will be in the next CL.

BUG= 692621 , 692544 

Review-Url: https://codereview.chromium.org/2708943002
Cr-Commit-Position: refs/heads/master@{#453627}

[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/BUILD.gn
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_context.cc
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_context.h
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_data_manager.cc
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_data_manager.h
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_controller.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_controller.h
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_controller_unittest.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_data.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_data.h
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_info.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_job_info.h
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_request_info.cc
[add] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/browser/background_fetch/background_fetch_request_info.h
[delete] https://crrev.com/5945b6a2a3a58705e9fc58a0ca12e157c1ae1f7f/content/browser/background_fetch/fetch_request.cc
[delete] https://crrev.com/5945b6a2a3a58705e9fc58a0ca12e157c1ae1f7f/content/browser/background_fetch/fetch_request.h
[modify] https://crrev.com/a247d9bf7b2082393dd172c0d78c33b044d646e2/content/test/BUILD.gn

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 9 2017

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

commit 35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a
Author: harkness <harkness@chromium.org>
Date: Thu Mar 09 21:36:53 2017

Make the BackgroundFetchJobController a per-job object

In previous code, the BackgroundFetchJobController had information about
all jobs in progress. In this CL, multiple job controllers are instantiated,
and each controller monitors a single job. This allows the controller to
be later used as an Observer for the particular files in its job.

Also changed in this CL is the ownership model for the JobData. Instead of
the DataManager keeping ownership of the JobData, the ownership is passed
to the JobController. The Context then keeps ownership of all created
JobControllers and calls Shutdown on the controllers before the DataManager
is destroyed.

There is still temporary storage for the JobInfo and RequestInfos which will
eventually be written to permanent storage, but these temporary data
structures have been more explicitly called out as temporary.

BUG= 692544 

Review-Url: https://codereview.chromium.org/2724783002
Cr-Commit-Position: refs/heads/master@{#455869}

[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_context.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_context.h
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_data_manager.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_data_manager.h
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_controller.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_controller.h
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_controller_unittest.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_data.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_data.h
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_info.cc
[modify] https://crrev.com/35b3ce0d6309b6cc8ad83084aa0e0365ae1e2b5a/content/browser/background_fetch/background_fetch_job_info.h

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 21 2017

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

commit 85aa3feee7879a190e2a9a9563e1e3daf59676f1
Author: harkness <harkness@chromium.org>
Date: Tue Mar 21 14:18:58 2017

Moved tests from DataManager to JobData.

Originally, the request management was in the BackgroundFetchDataManager,
but it was moved into the JobData and the tests weren't moved at the same
time. This CL moves those tests, which avoids a lot of setup for the
unused DataManager functionality.

This also expands the testing for the JobData, adding tests for paused,
cancelled, and out of order request completion.

BUG= 692544 

Review-Url: https://codereview.chromium.org/2749043003
Cr-Commit-Position: refs/heads/master@{#458401}

[delete] https://crrev.com/9252579e4ab9a83ff8a863288bf554aa82877585/content/browser/background_fetch/background_fetch_data_manager_unittest.cc
[add] https://crrev.com/85aa3feee7879a190e2a9a9563e1e3daf59676f1/content/browser/background_fetch/background_fetch_job_data_unittest.cc
[modify] https://crrev.com/85aa3feee7879a190e2a9a9563e1e3daf59676f1/content/test/BUILD.gn

Comment 5 by peter@chromium.org, Mar 29 2017

Status: Fixed (was: Available)
I'm going to mark this as fixed since our iteration code allows for this. (Despite us only making a single request in parallel for now, we'll have to measure and fine-tune the constant.)

Sign in to add a comment