Remove more //chrome dependencies from VariationsService

Remove more //chrome dependencies from VariationsService

- chrome/browser/variations/generated_resources_map.h: Use a client method
  instead.
- chrome_switches.h: Move the appropriate switches into
  //components/variations, and pass the remaining switch in from the embedder.

Note that ideally generated_resources_map.h would be componentized as well.
However, that's currently an open problem, as the generation of that file is
reliant on the embedder's strings already have been generated. It is not
currently clear whether we'll be able to componentize it.

BUG=516678
TBR=thakis

Committed: https://crrev.com/3e603508d644161342d6c813cc6d064c4d590842
Cr-Commit-Position: refs/heads/master@{#346104}

[blundell, 2015-08-27 16:59:03]

blundell@chromium.org changed reviewers:
+ asvitkine@chromium.org

[blundell, 2015-08-27 16:59:04]

Nearing the end :).

[Alexei Svitkine (slow), 2015-08-27 18:27:10]

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:212: void
OverrideUIString(VariationsServiceClient* client,
Why not have the whole OverrideUIString() function be part of the client
interface?

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
File components/variations/variations_switches.cc (right):

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
components/variations/variations_switches.cc:14: const char
kFakeVariationsChannel[]         = "fake-variations-channel";
Nit: These don't need so many spaces before the =. I suggest just 1, no need for
aligning them.

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
File components/variations/variations_switches.h (right):

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
components/variations/variations_switches.h:8: namespace chrome_variations {
Eventually this should be "variations". Any reason it can't start that way?

[blundell, 2015-08-27 18:33:02]

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:212: void
OverrideUIString(VariationsServiceClient* client,
On 2015/08/27 18:27:10, Alexei Svitkine wrote:
> Why not have the whole OverrideUIString() function be part of the client
> interface?

In the interest of sharing as much code as possible -- I wouldn't expect this
behavior to change per-embedder. Can change if you feel strongly.

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
File components/variations/variations_switches.cc (right):

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
components/variations/variations_switches.cc:14: const char
kFakeVariationsChannel[]         = "fake-variations-channel";
On 2015/08/27 18:27:10, Alexei Svitkine wrote:
> Nit: These don't need so many spaces before the =. I suggest just 1, no need
for
> aligning them.

Done.

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
File components/variations/variations_switches.h (right):

https://codereview.chromium.org/1309843005/diff/1/components/variations/varia...
components/variations/variations_switches.h:8: namespace chrome_variations {
On 2015/08/27 18:27:10, Alexei Svitkine wrote:
> Eventually this should be "variations". Any reason it can't start that way?

The same reason that the other code I've moved over has stayed in the
chrome_variations namespace: to avoid churn in VariationsService.

[Alexei Svitkine (slow), 2015-08-27 18:44:00]

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:212: void
OverrideUIString(VariationsServiceClient* client,
On 2015/08/27 18:33:01, blundell wrote:
> On 2015/08/27 18:27:10, Alexei Svitkine wrote:
> > Why not have the whole OverrideUIString() function be part of the client
> > interface?
> 
> In the interest of sharing as much code as possible -- I wouldn't expect this
> behavior to change per-embedder. Can change if you feel strongly.

I see. I'm thinking that if variations/service might be re-used by other things,
like Chromecast, that they might not want the dependency on the resource bundle
stuff.

But I guess there's also a dependency on FormFactor that's in ui/ too.

I dunno... I still feel like abstracting GetIndexForResource() is weird - I'd
rather keep this whole function together.

Logically, if we do want to refactor things out, I'd rather have a helper class
that gets initialized with the two arrays from generated_resources_map.h and
contains the complete OverrideUIString() logic. But if not going to that length,
I'd prefer that OverrideUIString() stays as-is and just moves to the client.

[blundell, 2015-08-27 19:05:58]

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

https://codereview.chromium.org/1309843005/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:212: void
OverrideUIString(VariationsServiceClient* client,
On 2015/08/27 18:44:00, Alexei Svitkine wrote:
> On 2015/08/27 18:33:01, blundell wrote:
> > On 2015/08/27 18:27:10, Alexei Svitkine wrote:
> > > Why not have the whole OverrideUIString() function be part of the client
> > > interface?
> > 
> > In the interest of sharing as much code as possible -- I wouldn't expect
this
> > behavior to change per-embedder. Can change if you feel strongly.
> 
> I see. I'm thinking that if variations/service might be re-used by other
things,
> like Chromecast, that they might not want the dependency on the resource
bundle
> stuff.
> 
> But I guess there's also a dependency on FormFactor that's in ui/ too.
> 
> I dunno... I still feel like abstracting GetIndexForResource() is weird - I'd
> rather keep this whole function together.
> 
> Logically, if we do want to refactor things out, I'd rather have a helper
class
> that gets initialized with the two arrays from generated_resources_map.h and
> contains the complete OverrideUIString() logic. But if not going to that
length,
> I'd prefer that OverrideUIString() stays as-is and just moves to the client.

Done. Ideally this wouldn't have to be componentized at all, at least for the
purposes of iOS. But at this time I can't figure out what to do about the
generated_resources_map stuff.

[Alexei Svitkine (slow), 2015-08-27 19:23:31]

lgtm

[blundell, 2015-08-28 08:06:33]

blundell@chromium.org changed reviewers:
+ thakis@chromium.org

[blundell, 2015-08-28 08:06:43]

TBR=thakis for chrome_browser_main

[blundell, 2015-08-28 08:06:50]

The CQ bit was checked by blundell@chromium.org

[commit-bot: I haz the power, 2015-08-28 08:07:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1309843005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1309843005/40001

[commit-bot: I haz the power, 2015-08-28 09:10:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-28 09:10:54]

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