WebAPKs: Fix play-install variations check

WebAPKs: Fix play-install variations check

This CL:
1) Fixes ChromeWebApkHost#canUseGooglePlayToInstallWebApk() to use
variations::GetVariationParamValueByFeature() instead of
variations::GetVariationParamValue(). This matches the server side variations
configuration.
2) Updates the cached value of whether WebAPKs are enabled (Returned by
ChromeWebApkHost#isEnabled()) prior to calling
ChromeWebApkHost#canUseGooglePlayToInstallWebApk().

BUG=684804
Review-Url: https://codereview.chromium.org/2655803002
Cr-Commit-Position: refs/heads/master@{#446097}
Committed: https://chromium.googlesource.com/chromium/src/+/2a1ffdc2376233aaef5110a1c406a50539a795bd

[pkotwicz, 2017-01-25 02:05:57]

Description was changed from

==========
WebAPKs: Fix play-install variations check

This CL:
1) Fixes ChromeWebApkHost#canUseGooglePlayToInstallWebApk() to use
variations::GetVariationParamValueByFeature() instead of
variations::GetVariationParamValue(). This matches the server side variations
configuration.
2) Updates the cached value of whether WebAPKs are enabled (Returned by
ChromeWebApkHost#isEnabled()) prior to calling
ChromeWebApkHost#canUseGooglePlayToInstallWebApk().

BUG=684804
==========

to

==========
WebAPKs: Fix play-install variations check

This CL:
1) Fixes ChromeWebApkHost#canUseGooglePlayToInstallWebApk() to use
variations::GetVariationParamValueByFeature() instead of
variations::GetVariationParamValue(). This matches the server side variations
configuration.
2) Updates the cached value of whether WebAPKs are enabled (Returned by
ChromeWebApkHost#isEnabled()) prior to calling
ChromeWebApkHost#canUseGooglePlayToInstallWebApk().

BUG=684804
==========

[pkotwicz, 2017-01-25 02:05:57]

pkotwicz@chromium.org changed reviewers:
+ dominickn@chromium.org

[pkotwicz, 2017-01-25 02:10:28]

Dominick, can you please take a look?

This CL will unfortunately also need to be merged back to M57.

I will send a separate CL to make the "Please enable unknown sources" show less
often - not on every Chrome start

[pkotwicz, 2017-01-25 03:02:59]

Update: I will write the CL to make the "Please enable unknown sources" dialog
show less often sometime later this week but not today.

[dominickn, 2017-01-25 04:05:31]

lgtm, but this class now feels very fragile with when things can and can't be
called (before native library is loaded, after cacheEnabled() is called, etc.).
It would be good to have a think about whether the complexity in here can be
reduced. Also, having to touch preferences every time isEnabled() is called is
really painful.

For instance, making this class a singleton instance like the InstantAppsHandler
so that you can cache the boolean might be a better way to structure it.

[pkotwicz, 2017-01-25 04:45:29]

Ok, I will add a comment to ChromeWebApkHost#isEnabled() about the method being
called prior to native being loaded

I am planning on adding tests for the follow up CL. Hence the delay.
I have filed crbug.com/683354 about caching the return value of isEnabled()

[pkotwicz, 2017-01-25 04:49:10]

Clarifying things will require lots of changes to comments. I filed
crbug.com/684999 to clarify which methods in ChromeWebApkHost are safe to call
prior to native being loaded and which are not

[pkotwicz, 2017-01-25 04:49:20]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-01-25 04:49:49]

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-01-25 08:53:44]

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

[commit-bot: I haz the power, 2017-01-25 08:53:44]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[pkotwicz, 2017-01-25 15:30:10]

pkotwicz@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[pkotwicz, 2017-01-25 15:30:10]

dfalcantara@ for chrome_jni_registrar.cc OWNERS

[gone, 2017-01-25 19:11:50]

that file lgtm

[pkotwicz, 2017-01-25 19:45:24]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2017-01-25 19:45:59]

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-01-25 19:56:17]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485373524930350, "parent_rev":
"9c0001763fde79549c208242560bc0d9245c6441", "commit_rev":
"2a1ffdc2376233aaef5110a1c406a50539a795bd"}

[commit-bot: I haz the power, 2017-01-25 19:56:52]

Description was changed from

==========
WebAPKs: Fix play-install variations check

This CL:
1) Fixes ChromeWebApkHost#canUseGooglePlayToInstallWebApk() to use
variations::GetVariationParamValueByFeature() instead of
variations::GetVariationParamValue(). This matches the server side variations
configuration.
2) Updates the cached value of whether WebAPKs are enabled (Returned by
ChromeWebApkHost#isEnabled()) prior to calling
ChromeWebApkHost#canUseGooglePlayToInstallWebApk().

BUG=684804
==========

to

==========
WebAPKs: Fix play-install variations check

This CL:
1) Fixes ChromeWebApkHost#canUseGooglePlayToInstallWebApk() to use
variations::GetVariationParamValueByFeature() instead of
variations::GetVariationParamValue(). This matches the server side variations
configuration.
2) Updates the cached value of whether WebAPKs are enabled (Returned by
ChromeWebApkHost#isEnabled()) prior to calling
ChromeWebApkHost#canUseGooglePlayToInstallWebApk().

BUG=684804

Review-Url: https://codereview.chromium.org/2655803002
Cr-Commit-Position: refs/heads/master@{#446097}
Committed:
https://chromium.googlesource.com/chromium/src/+/2a1ffdc2376233aaef5110a1c406...
==========

[commit-bot: I haz the power, 2017-01-25 19:56:53]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/2a1ffdc2376233aaef5110a1c406...