Configuration for download service.

Configuration for download service.

Setup the configuration for download service, each service holds a
configuration object.

Tests can modify the Configuration object or use
base::test::ScopedFeatureList to overwrite the configuration.

BUG=717180
Review-Url: https://codereview.chromium.org/2866483002
Cr-Commit-Position: refs/heads/master@{#469872}
Committed: https://chromium.googlesource.com/chromium/src/+/a3b9a27415726553953ea3f0d1c926a51d26a412

[xingliu, 2017-05-04 18:10:49]

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

[commit-bot: I haz the power, 2017-05-04 18:11:26]

Dry run: CQ is trying da patch.

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

[xingliu, 2017-05-04 18:12:29]

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

[xingliu, 2017-05-04 18:12:30]

Hi, PTAL, thanks.

[commit-bot: I haz the power, 2017-05-04 19:19:24]

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

[commit-bot: I haz the power, 2017-05-04 19:19:25]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-05-05 20:10:52]

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
File components/download/internal/config.cc (right):

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
components/download/internal/config.cc:44: void LoadFinchConfigs(Configuration*
config) {
Can we do something like make this static and make it erturn a configuration?

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

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
components/download/internal/download_service_impl.h:34: Configuration config_;
We should pass one in the constructor

[xingliu, 2017-05-05 22:55:03]

Thanks for the review, PTAL.

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
File components/download/internal/config.cc (right):

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
components/download/internal/config.cc:44: void LoadFinchConfigs(Configuration*
config) {
On 2017/05/05 20:10:52, David Trainor-ping if over 24h wrote:
> Can we do something like make this static and make it erturn a configuration?

Done.

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

https://codereview.chromium.org/2866483002/diff/20001/components/download/int...
components/download/internal/download_service_impl.h:34: Configuration config_;
On 2017/05/05 20:10:52, David Trainor-ping if over 24h wrote:
> We should pass one in the constructor

Done.

[David Trainor- moved to gerrit, 2017-05-05 23:36:29]

lgtm

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

https://codereview.chromium.org/2866483002/diff/60001/components/download/int...
components/download/internal/config.h:37: static std::unique_ptr<Configuration>
Create();
CreateFromFinch?

https://codereview.chromium.org/2866483002/diff/60001/components/download/int...
components/download/internal/config.h:59: Configuration();
Leave this public.

[xingliu, 2017-05-06 00:55:02]

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

https://codereview.chromium.org/2866483002/diff/60001/components/download/int...
components/download/internal/config.h:37: static std::unique_ptr<Configuration>
Create();
On 2017/05/05 23:36:28, David Trainor-ping if over 24h wrote:
> CreateFromFinch?

Done.

https://codereview.chromium.org/2866483002/diff/60001/components/download/int...
components/download/internal/config.h:59: Configuration();
On 2017/05/05 23:36:29, David Trainor-ping if over 24h wrote:
> Leave this public.

Done.

[xingliu, 2017-05-06 00:55:14]

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

[commit-bot: I haz the power, 2017-05-06 00:55:49]

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-06 02:12:04]

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

[commit-bot: I haz the power, 2017-05-06 02:12:04]

Dry run: This issue passed the CQ dry run.

[xingliu, 2017-05-06 02:32:19]

The CQ bit was checked by xingliu@chromium.org

[xingliu, 2017-05-06 02:32:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2866483002/#ps80001
(title: "Work on reviews.")

[commit-bot: I haz the power, 2017-05-06 02:32:55]

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-06 19:12:29]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494037939923770,
"parent_rev": "ec24fc6943d5408cf5aba9bc2989a7aea37f6381", "commit_rev":
"a3b9a27415726553953ea3f0d1c926a51d26a412"}

[commit-bot: I haz the power, 2017-05-06 19:12:38]

Description was changed from

==========
Configuration for download service.

Setup the configuration for download service, each service holds a
configuration object.

Tests can modify the Configuration object or use
base::test::ScopedFeatureList to overwrite the configuration.

BUG=717180
==========

to

==========
Configuration for download service.

Setup the configuration for download service, each service holds a
configuration object.

Tests can modify the Configuration object or use
base::test::ScopedFeatureList to overwrite the configuration.

BUG=717180

Review-Url: https://codereview.chromium.org/2866483002
Cr-Commit-Position: refs/heads/master@{#469872}
Committed:
https://chromium.googlesource.com/chromium/src/+/a3b9a27415726553953ea3f0d1c9...
==========

[commit-bot: I haz the power, 2017-05-06 19:12:39]

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