Convert FieldTrialList to Accept a std::unique_ptr

Convert FieldTrialList to Accept a std::unique_ptr

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Committed: https://crrev.com/79393ffbe9d9a1615146b2de220a893dec26dd7b
Cr-Commit-Position: refs/heads/master@{#420118}

[robliao, 2016-09-21 00:03:27]

Description was changed from

==========
Convert FieldTrialList to Accept a std::unique_ptr

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

to

==========
Convert FieldTrialList to Accept a std::unique_ptr

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

[robliao, 2016-09-21 00:04:36]

robliao@chromium.org changed reviewers:
+ asvitkine@chromium.org, brettw@chromium.org, isherman@chromium.org

[robliao, 2016-09-21 00:04:37]

brettw@chromium.org: Please review this CL. Thanks!

asvitkine@chromium.org and isherman@chromium.org: FYI on the signature change.
Feel free to comment.

[robliao, 2016-09-21 00:04:45]

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

[commit-bot: I haz the power, 2016-09-21 00:05:14]

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, 2016-09-21 01:33:31]

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

[commit-bot: I haz the power, 2016-09-21 01:33:32]

Dry run: This issue passed the CQ dry run.

[Alexei Svitkine (slow), 2016-09-21 15:52:01]

Thanks for doing this!

https://codereview.chromium.org/2358723002/diff/1/base/metrics/field_trial.h
File base/metrics/field_trial.h (right):

https://codereview.chromium.org/2358723002/diff/1/base/metrics/field_trial.h#...
base/metrics/field_trial.h:344: // entropy values. If one time randomization is
not desired, pass in nullptr
Nit: I think in comments just using "null" rather than "nullptr" is preferred -
since it's talking about the concept of null. I remember that was the conclusion
from the chromium-dev@ thread where we first discussed using nullptr in Chromium
instead of NULL.

https://codereview.chromium.org/2358723002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/2358723002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/search_provider_unittest.cc:511:
base::MakeUnique<metrics::SHA1EntropyProvider>("foo")));
Nit: Actually all the test code that currently passes in an entropy provider
should just work if null is passed. So I suggest cleaning that up while you're
touching all these places. (should apply to all tests currently doing something
like this)

[brettw, 2016-09-21 16:15:01]

LGTM. Thanks for following up, there were more callers than I would have
guessed!

[robliao, 2016-09-21 17:24:29]

https://codereview.chromium.org/2358723002/diff/1/base/metrics/field_trial.h
File base/metrics/field_trial.h (right):

https://codereview.chromium.org/2358723002/diff/1/base/metrics/field_trial.h#...
base/metrics/field_trial.h:344: // entropy values. If one time randomization is
not desired, pass in nullptr
On 2016/09/21 15:52:00, Alexei Svitkine (very slow) wrote:
> Nit: I think in comments just using "null" rather than "nullptr" is preferred
-
> since it's talking about the concept of null. I remember that was the
conclusion
> from the chromium-dev@ thread where we first discussed using nullptr in
Chromium
> instead of NULL.

Change this to lowercase null then to deemphasize the use of NULL.

https://codereview.chromium.org/2358723002/diff/1/chrome/browser/autocomplete...
File chrome/browser/autocomplete/search_provider_unittest.cc (right):

https://codereview.chromium.org/2358723002/diff/1/chrome/browser/autocomplete...
chrome/browser/autocomplete/search_provider_unittest.cc:511:
base::MakeUnique<metrics::SHA1EntropyProvider>("foo")));
On 2016/09/21 15:52:00, Alexei Svitkine (very slow) wrote:
> Nit: Actually all the test code that currently passes in an entropy provider
> should just work if null is passed. So I suggest cleaning that up while you're
> touching all these places. (should apply to all tests currently doing
something
> like this)

I haven't done an analysis of the tests to verify that this behavior is okay.

This CL shouldn't trigger any new behaviors since it's a signature change, so we
can do that in a followup CL if need be.

[robliao, 2016-09-21 17:24:37]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-09-21 17:24:38]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2358723002/#ps20001
(title: "Change Comment nullptr to null")

[commit-bot: I haz the power, 2016-09-21 17:25:09]

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

[Alexei Svitkine (slow), 2016-09-21 18:07:24]

lgtm

[commit-bot: I haz the power, 2016-09-21 18:45:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-21 18:48:50]

Description was changed from

==========
Convert FieldTrialList to Accept a std::unique_ptr

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng
==========

to

==========
Convert FieldTrialList to Accept a std::unique_ptr

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Committed: https://crrev.com/79393ffbe9d9a1615146b2de220a893dec26dd7b
Cr-Commit-Position: refs/heads/master@{#420118}
==========

[commit-bot: I haz the power, 2016-09-21 18:48:52]

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