Use experimentation framework parameters for location sensitivity

Use experimentation framework parameters for location sensitivity.
Added a private API for this.

BUG=164227
TEST=No

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216847

[vadimt, 2013-08-09 17:52:13]

kalman@, please provide OWNER's approval for changes metricsPrivate API
rgustafson@, skare@, please make pre-owner review for the Now extension changes

[not at google - send to devlin, 2013-08-09 18:09:22]

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
(right):

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:82:
EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &name));
prefer using generated code here even if it's simple.

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:85: if
(chrome_variations::GetVariationParams(name, &params)) {
early return here, and set an error message on failure:

if (!chrome_variasions::Ge(..)) {
  error_ = ???
  return false;
}

...
return true.

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:91:
settings->SetWithoutPathExpansion(
SetStringWithoutPathExpansion

[vadimt, 2013-08-09 19:30:19]

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
File chrome/browser/extensions/api/metrics_private/metrics_private_api.cc
(right):

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:82:
EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &name));
On 2013/08/09 18:09:22, kalman wrote:
> prefer using generated code here even if it's simple.

Done.

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:85: if
(chrome_variations::GetVariationParams(name, &params)) {
On 2013/08/09 18:09:22, kalman wrote:
> early return here, and set an error message on failure:
> 
> if (!chrome_variasions::Ge(..)) {
>   error_ = ???
>   return false;
> }
> 
> ...
> return true.

Done.

https://codereview.chromium.org/22382006/diff/1/chrome/browser/extensions/api...
chrome/browser/extensions/api/metrics_private/metrics_private_api.cc:91:
settings->SetWithoutPathExpansion(
On 2013/08/09 18:09:22, kalman wrote:
> SetStringWithoutPathExpansion

Done.

[not at google - send to devlin, 2013-08-09 19:45:11]

https://codereview.chromium.org/22382006/diff/5001/chrome/common/extensions/a...
File chrome/common/extensions/api/metrics_private.json (right):

https://codereview.chromium.org/22382006/diff/5001/chrome/common/extensions/a...
chrome/common/extensions/api/metrics_private.json:73: "additionalProperties": {
"type": "any" }
wait... will these always be string -> string mappings? In which case you can
have type:string here and it will generate even nicer code for you.

[vadimt, 2013-08-09 20:25:21]

https://codereview.chromium.org/22382006/diff/5001/chrome/common/extensions/a...
File chrome/common/extensions/api/metrics_private.json (right):

https://codereview.chromium.org/22382006/diff/5001/chrome/common/extensions/a...
chrome/common/extensions/api/metrics_private.json:73: "additionalProperties": {
"type": "any" }
On 2013/08/09 19:45:12, kalman wrote:
> wait... will these always be string -> string mappings? In which case you can
> have type:string here and it will generate even nicer code for you.

Done.

[not at google - send to devlin, 2013-08-09 20:32:17]

nice, huh? lgtm

[vadimt, 2013-08-09 20:40:32]

It seems strange that the generated name is additional_properties. I'd expect it
to be 'params'.
Like:

  if (!chrome_variations::GetVariationParams(
       params->name, &result.params)) {

(looks like a bug in generator)

[not at google - send to devlin, 2013-08-09 20:46:20]

On 2013/08/09 20:40:32, vadimt wrote:
> It seems strange that the generated name is additional_properties. I'd expect
it
> to be 'params'.
> Like:
> 
>   if (!chrome_variations::GetVariationParams(
>        params->name, &result.params)) {
> 
> (looks like a bug in generator)

It can't be result.params; the "params" object might have some named properties,
for example "params"; that's why it's under additional_properties.

IIRC params gets squashed down into the result type because there's only a
single result parameter.

[skare_, 2013-08-09 20:47:18]

lgtm

[rgustafson, 2013-08-09 20:57:27]

lgtm

[vadimt, 2013-08-09 21:01:11]

arv@, please provide OWNER's approval for
chrome/browser/resources/google_now/background.js

[not at google - send to devlin, 2013-08-09 21:06:45]

On 2013/08/09 21:01:11, vadimt wrote:
> arv@, please provide OWNER's approval for
> chrome/browser/resources/google_now/background.js

you should add yourself to owners in c/b/r/google_now

[robliao, 2013-08-09 22:01:53]

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:124:
tasks.instrumentApiFunction(chrome.metricsPrivate, 'getVariationParams', 1);
Sync to the latest for updating to instrumentChromeApiFunction.

[vadimt, 2013-08-09 22:08:59]

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:124:
tasks.instrumentApiFunction(chrome.metricsPrivate, 'getVariationParams', 1);
On 2013/08/09 22:01:54, Robert Liao wrote:
> Sync to the latest for updating to instrumentChromeApiFunction.

Will do before submittinng to CQ.

[arv (Not doing code reviews), 2013-08-09 23:13:15]

LGTM

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:408: parseInt(params &&
params.minDistanceInMeters) ||
don't use parseInt without radix

[vadimt, 2013-08-10 01:16:07]

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/22382006/diff/14001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:408: parseInt(params &&
params.minDistanceInMeters) ||
On 2013/08/09 23:13:16, arv wrote:
> don't use parseInt without radix

Done.

[commit-bot: I haz the power, 2013-08-10 01:18:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/22382006/28001

[commit-bot: I haz the power, 2013-08-10 01:29:09]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-08-10 01:37:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/22382006/47001

[commit-bot: I haz the power, 2013-08-10 14:01:06]

Change committed as 216847