Create fluent Builder for DataReductionProxyTestContext.

Create fluent Builder for DataReductionProxyTestContext.

This simplifies the way in which a DRP TestContext is created, in that
optional parameters are specified with explicit methods on the Builder.

BUG=461914
Committed: https://crrev.com/e9448c81348c4d10000654c29ac2c83a6cd76086
Cr-Commit-Position: refs/heads/master@{#318142}

[jeremyim, 2015-02-25 21:25:08]

jeremyim@chromium.org changed reviewers:
+ bengr@chromium.org, sclittle@chromium.org

[jeremyim, 2015-02-25 21:25:08]

PTAL!

[sclittle, 2015-02-25 21:49:17]

I really like this; much cleaner now. Just some comments about style.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
(right):

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:124:
make_scoped_ptr(new TestingPrefServiceSimple());
This can just be:

scoped_ptr<TestingPrefServiceSimple> pref_service(new
TestingPrefServiceSimple());

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:126:
if (request_context_ != nullptr) {
This can just be "if (request_context_)"

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:134:
make_scoped_ptr(new DataReductionProxyEventStore(task_runner));
You don't need the make_scoped_ptr here, you can just write "scoped_ptr<...>
event_store(new ...)"

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:138:
configurator = make_scoped_ptr(new TestDataReductionProxyConfigurator(
Maybe use configurator.reset instead of make_scoped_ptr? There's really no need
to create and pass a whole new scoped ptr in.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:141:
configurator = make_scoped_ptr(new DataReductionProxyConfigurator(
Same thing with configurator.reset instead of make_scoped_ptr

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:158:
make_scoped_ptr(new DataReductionProxyRequestOptions(
Construct with pointer instead of using make_scoped_ptr

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:162:
make_scoped_ptr(new DataReductionProxySettings());
Construct with pointer instead of using make_scoped_ptr

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:178:
make_scoped_ptr(new DataReductionProxyTestContext(
Construct with pointer instead of using make_scoped_ptr

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:199:
loop_ = loop.Pass();
I think you can initialize all of these in the initializer list (e.g.
"loop_(loop.Pass())") instead of in the body of the constructor.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
(right):

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h:92:
// Specifies a |net::URLRequestContext| to use.
Maybe mention that the net::URLRequestContext is owned by the caller?

[jeremyim, 2015-02-25 22:15:09]

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
(right):

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:124:
make_scoped_ptr(new TestingPrefServiceSimple());
On 2015/02/25 21:49:17, sclittle wrote:
> This can just be:
> 
> scoped_ptr<TestingPrefServiceSimple> pref_service(new
> TestingPrefServiceSimple());

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:126:
if (request_context_ != nullptr) {
On 2015/02/25 21:49:17, sclittle wrote:
> This can just be "if (request_context_)"

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:134:
make_scoped_ptr(new DataReductionProxyEventStore(task_runner));
On 2015/02/25 21:49:17, sclittle wrote:
> You don't need the make_scoped_ptr here, you can just write "scoped_ptr<...>
> event_store(new ...)"

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:138:
configurator = make_scoped_ptr(new TestDataReductionProxyConfigurator(
On 2015/02/25 21:49:17, sclittle wrote:
> Maybe use configurator.reset instead of make_scoped_ptr? There's really no
need
> to create and pass a whole new scoped ptr in.

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:141:
configurator = make_scoped_ptr(new DataReductionProxyConfigurator(
On 2015/02/25 21:49:17, sclittle wrote:
> Same thing with configurator.reset instead of make_scoped_ptr

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:158:
make_scoped_ptr(new DataReductionProxyRequestOptions(
On 2015/02/25 21:49:17, sclittle wrote:
> Construct with pointer instead of using make_scoped_ptr

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:162:
make_scoped_ptr(new DataReductionProxySettings());
On 2015/02/25 21:49:17, sclittle wrote:
> Construct with pointer instead of using make_scoped_ptr

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:178:
make_scoped_ptr(new DataReductionProxyTestContext(
On 2015/02/25 21:49:17, sclittle wrote:
> Construct with pointer instead of using make_scoped_ptr

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc:199:
loop_ = loop.Pass();
On 2015/02/25 21:49:17, sclittle wrote:
> I think you can initialize all of these in the initializer list (e.g.
> "loop_(loop.Pass())") instead of in the body of the constructor.

Done.

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
(right):

https://codereview.chromium.org/959913002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h:92:
// Specifies a |net::URLRequestContext| to use.
On 2015/02/25 21:49:17, sclittle wrote:
> Maybe mention that the net::URLRequestContext is owned by the caller?

Done.

[sclittle, 2015-02-25 22:19:21]

LGTM

[bengr, 2015-02-25 23:01:53]

lgtm

[jeremyim, 2015-02-25 23:02:16]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2015-02-25 23:03:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/959913002/20001

[commit-bot: I haz the power, 2015-02-25 23:28:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-25 23:29:09]

Patchset 2 (id:??) landed as
https://crrev.com/e9448c81348c4d10000654c29ac2c83a6cd76086
Cr-Commit-Position: refs/heads/master@{#318142}