Add initial Controller to DownloadService

Add initial Controller to DownloadService

Add the basic structure for a Controller, which is responsible for
gluing the Model and DownloadDriver in DownloadService.  Initial support
in this patch includes:
  - Successful initialization path of Model and DownloadDriver.
  - All possible paths for supporting StartDownload (minus actually
  sending the request to the DownloadDriver).

BUG=722705
Review-Url: https://codereview.chromium.org/2895953004
Cr-Commit-Position: refs/heads/master@{#475982}
Committed: https://chromium.googlesource.com/chromium/src/+/7f99e4119871345cfbdfb5553b218c7e5464ad42

[David Trainor- moved to gerrit, 2017-05-24 23:54:40]

Description was changed from

==========
WIP

BUG=
==========

to

==========
Add initial Controller to DownloadService

Add the basic structure for a Controller, which is responsible for
gluing the Model and DownloadDriver in DownloadService.  Initial support
in this patch includes:
  - Successful initialization path of Model and DownloadDriver.
  - All possible paths for supporting StartDownload (minus actually
  sending the request to the DownloadDriver).

BUG=722705
==========

[David Trainor- moved to gerrit, 2017-05-24 23:54:40]

dtrainor@chromium.org changed reviewers:
+ shaktisahu@chromium.org, xingliu@chromium.org

[David Trainor- moved to gerrit, 2017-05-24 23:54:50]

ptal thanks!

[xingliu, 2017-05-27 00:25:32]

lgtm with some nits.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/config.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/config.h:42: uint32_t max_concurrent_downloads;
nit: Maybe also change the parsing function in config.cc.
StringToInt ==>StringToUInt

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/download_service_impl.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/download_service_impl.h:24:
DownloadServiceImpl(std::unique_ptr<Controller> scheduler);
nit: (std::unique_ptr<Controller> controller);

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/entry_utils.cc (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/entry_utils.cc:35: return categorized;
optional nit: this is a cool name, but the caller need to reduce themselves.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/model.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/model.h:38: virtual void OnModelDestroyed(bool
success) = 0;
fyi: Destroy call probably will be removed in recent CL.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/startup_status.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/startup_status.h:16: public:
optional nit: public can be removed.

[David Trainor- moved to gerrit, 2017-05-30 16:50:14]

dtrainor@chromium.org changed reviewers:
+ peter@chromium.org

[David Trainor- moved to gerrit, 2017-05-30 16:50:15]

+peter@

[David Trainor- moved to gerrit, 2017-05-30 18:55:02]

The CQ bit was checked by dtrainor@chromium.org to run a CQ dry run

[David Trainor- moved to gerrit, 2017-05-30 18:55:24]

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/config.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/config.h:42: uint32_t max_concurrent_downloads;
On 2017/05/27 00:25:32, xingliu wrote:
> nit: Maybe also change the parsing function in config.cc.
> StringToInt ==>StringToUInt

Done.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/download_service_impl.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/download_service_impl.h:24:
DownloadServiceImpl(std::unique_ptr<Controller> scheduler);
On 2017/05/27 00:25:32, xingliu wrote:
> nit: (std::unique_ptr<Controller> controller);

Done.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/entry_utils.cc (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/entry_utils.cc:35: return categorized;
On 2017/05/27 00:25:32, xingliu wrote:
> optional nit: this is a cool name, but the caller need to reduce themselves.

Done.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/model.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/model.h:38: virtual void OnModelDestroyed(bool
success) = 0;
On 2017/05/27 00:25:32, xingliu wrote:
> fyi: Destroy call probably will be removed in recent CL.

Yeah.  Will remove this.  Will wait for that CL to land and rebase.

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
File components/download/internal/startup_status.h (right):

https://codereview.chromium.org/2895953004/diff/40001/components/download/int...
components/download/internal/startup_status.h:16: public:
On 2017/05/27 00:25:32, xingliu wrote:
> optional nit: public can be removed.

Done.

[commit-bot: I haz the power, 2017-05-30 18:55:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 21:50:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 21:50:29]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[David Trainor- moved to gerrit, 2017-05-31 16:16:28]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-05-31 16:16:29]

The patchset sent to the CQ was uploaded after l-g-t-m from xingliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2895953004/#ps60001
(title: "Addressed comments")

[commit-bot: I haz the power, 2017-05-31 16:17:09]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Trainor- moved to gerrit, 2017-05-31 16:18:31]

submitting to unblock.  happy to address comments in follow up.

[David Trainor- moved to gerrit, 2017-05-31 16:18:41]

The CQ bit was unchecked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-05-31 17:05:05]

The CQ bit was checked by dtrainor@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-31 17:05:24]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Trainor- moved to gerrit, 2017-05-31 18:27:35]

The CQ bit was unchecked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-05-31 18:27:39]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-05-31 18:27:40]

The patchset sent to the CQ was uploaded after l-g-t-m from xingliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2895953004/#ps80001
(title: "Rebased")

[commit-bot: I haz the power, 2017-05-31 18:28:17]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-31 19:34:57]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1496255259620570,
"parent_rev": "c422a96aa3565e4193f3e9288b4b902fc464899a", "commit_rev":
"7f99e4119871345cfbdfb5553b218c7e5464ad42"}

[commit-bot: I haz the power, 2017-05-31 19:35:08]

Description was changed from

==========
Add initial Controller to DownloadService

Add the basic structure for a Controller, which is responsible for
gluing the Model and DownloadDriver in DownloadService.  Initial support
in this patch includes:
  - Successful initialization path of Model and DownloadDriver.
  - All possible paths for supporting StartDownload (minus actually
  sending the request to the DownloadDriver).

BUG=722705
==========

to

==========
Add initial Controller to DownloadService

Add the basic structure for a Controller, which is responsible for
gluing the Model and DownloadDriver in DownloadService.  Initial support
in this patch includes:
  - Successful initialization path of Model and DownloadDriver.
  - All possible paths for supporting StartDownload (minus actually
  sending the request to the DownloadDriver).

BUG=722705

Review-Url: https://codereview.chromium.org/2895953004
Cr-Commit-Position: refs/heads/master@{#475982}
Committed:
https://chromium.googlesource.com/chromium/src/+/7f99e4119871345cfbdfb5553b21...
==========

[commit-bot: I haz the power, 2017-05-31 19:35:10]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/7f99e4119871345cfbdfb5553b21...