[Variations] Refactor all state used for study filtering into a container struct.

[Variations] Refactor all state used for study filtering into a container struct.

BUG=727984
TEST=compiles/existing tests
R=asvitkine@chromium.org

Review-Url: https://codereview.chromium.org/2924983003
Cr-Commit-Position: refs/heads/master@{#477788}
Committed: https://chromium.googlesource.com/chromium/src/+/52a81bdab704346032d8d16c83251d22b2a5eb0c

[Ilya Sherman, 2017-06-07 04:49:13]

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

[commit-bot: I haz the power, 2017-06-07 04:49:28]

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-06-07 06:25:14]

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

[commit-bot: I haz the power, 2017-06-07 06:25:15]

Dry run: This issue passed the CQ dry run.

[Alexei Svitkine (slow), 2017-06-07 15:18:08]

lgtm, thanks!

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
File components/variations/service/variations_service.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
components/variations/service/variations_service.cc:629: const std::string
latest_country = GetLatestCountry();
Nit: Move closer to where it's used - above line 642.

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
File components/variations/study_filtering.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
components/variations/study_filtering.cc:171: const std::string&
GetStudyCountry(const Study& study,
Nit: I think name is misleading - since this is getting the client state country
for filtering the study.

How about GetClientStateCountryForStudy()?

https://codereview.chromium.org/2924983003/diff/20001/components/variations/v...
File components/variations/variations_seed_processor.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/v...
components/variations/variations_seed_processor.cc:186: for (size_t i = 0; i <
filtered_studies.size(); ++i) {
Nit: Change to C++11 for loop syntax if you're touching it already.

[Ilya Sherman, 2017-06-07 21:50:08]

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
File components/variations/service/variations_service.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
components/variations/service/variations_service.cc:629: const std::string
latest_country = GetLatestCountry();
On 2017/06/07 15:18:08, Alexei Svitkine (very slow) wrote:
> Nit: Move closer to where it's used - above line 642.

Done.

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
File components/variations/study_filtering.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/s...
components/variations/study_filtering.cc:171: const std::string&
GetStudyCountry(const Study& study,
On 2017/06/07 15:18:08, Alexei Svitkine (very slow) wrote:
> Nit: I think name is misleading - since this is getting the client state
country
> for filtering the study.
> 
> How about GetClientStateCountryForStudy()?

Done.  (I dropped "State" though, i.e. GetClientCountryForStudy().)

https://codereview.chromium.org/2924983003/diff/20001/components/variations/v...
File components/variations/variations_seed_processor.cc (right):

https://codereview.chromium.org/2924983003/diff/20001/components/variations/v...
components/variations/variations_seed_processor.cc:186: for (size_t i = 0; i <
filtered_studies.size(); ++i) {
On 2017/06/07 15:18:08, Alexei Svitkine (very slow) wrote:
> Nit: Change to C++11 for loop syntax if you're touching it already.

Done.

[Ilya Sherman, 2017-06-07 21:50:27]

The CQ bit was checked by isherman@chromium.org

[Ilya Sherman, 2017-06-07 21:50:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2924983003/#ps40001
(title: "A bit more cleanup")

[commit-bot: I haz the power, 2017-06-07 21:51:12]

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-06-07 23:30:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496872227738040,
"parent_rev": "e784724ab7bdebd696a585003d28c835352ddc75", "commit_rev":
"52a81bdab704346032d8d16c83251d22b2a5eb0c"}

[commit-bot: I haz the power, 2017-06-07 23:30:23]

Description was changed from

==========
[Variations] Refactor all state used for study filtering into a container
struct.

BUG=727984
TEST=compiles/existing tests
R=asvitkine@chromium.org
==========

to

==========
[Variations] Refactor all state used for study filtering into a container
struct.

BUG=727984
TEST=compiles/existing tests
R=asvitkine@chromium.org

Review-Url: https://codereview.chromium.org/2924983003
Cr-Commit-Position: refs/heads/master@{#477788}
Committed:
https://chromium.googlesource.com/chromium/src/+/52a81bdab704346032d8d16c8325...
==========

[commit-bot: I haz the power, 2017-06-07 23:30:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/52a81bdab704346032d8d16c8325...