Add Google Play services info to chrome://version

Add Google Play services info to chrome://version

Adds a new entry to the version info page that displays the version
number of the client library and the one for the installed Play
Services app. It also displays whether First Party APIs are available.

BUG=None

Review-Url: https://codereview.chromium.org/2751773002
Cr-Commit-Position: refs/heads/master@{#457527}
Committed: https://chromium.googlesource.com/chromium/src/+/4576d3a030396c7542eeb7e07017e4ac1e9ffa28

[dgn, 2017-03-14 18:26:57]

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

[dgn, 2017-03-14 18:27:10]

dgn@chromium.org changed reviewers:
+ bauerb@chromium.org

[dgn, 2017-03-14 18:27:11]

PTAL.
Preview: https://goo.gl/photos/FrhtWMdJEuUivCpdA

[commit-bot: I haz the power, 2017-03-14 18:27:45]

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

[dgn, 2017-03-14 18:29:00]

ktam@: FYI, since you recently asked about it

[Bernhard Bauer, 2017-03-14 18:31:55]

Neat!

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93:
long installedGmsVersion;
Make this final to ensure we assign to it exactly once?

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:94:
String accessType;
Move this to where it's assigned?

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:115:
return String.format(Locale.US, fmtString, installedGmsVersion, sdkVersion,
accessType);
Is installedGmsVersion and sdkVersion flipped?

[dgn, 2017-03-14 19:41:51]

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

[commit-bot: I haz the power, 2017-03-14 19:42:23]

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-03-14 20:51:51]

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

[commit-bot: I haz the power, 2017-03-14 20:51:52]

Dry run: This issue passed the CQ dry run.

[dgn, 2017-03-15 10:53:56]

PTAL

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93:
long installedGmsVersion;
On 2017/03/14 18:31:54, Bernhard Bauer wrote:
> Make this final to ensure we assign to it exactly once?

Doesn't work, the compilers seems to think that the code can throw and still
assign the variable in the block -_-

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:94:
String accessType;
On 2017/03/14 18:31:54, Bernhard Bauer wrote:
> Move this to where it's assigned?

Done.

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:115:
return String.format(Locale.US, fmtString, installedGmsVersion, sdkVersion,
accessType);
On 2017/03/14 18:31:54, Bernhard Bauer wrote:
> Is installedGmsVersion and sdkVersion flipped?

😨 thanks

[Bernhard Bauer, 2017-03-15 11:55:43]

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93:
long installedGmsVersion;
On 2017/03/15 10:53:55, dgn wrote:
> On 2017/03/14 18:31:54, Bernhard Bauer wrote:
> > Make this final to ensure we assign to it exactly once?
> 
> Doesn't work, the compilers seems to think that the code can throw and still
> assign the variable in the block -_-

Hmph, that's why I wanted to do it :-D Could we extract this to a method?

[dgn, 2017-03-15 16:38:08]

PTAL

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93:
long installedGmsVersion;
On 2017/03/15 11:55:43, Bernhard Bauer wrote:
> On 2017/03/15 10:53:55, dgn wrote:
> > On 2017/03/14 18:31:54, Bernhard Bauer wrote:
> > > Make this final to ensure we assign to it exactly once?
> > 
> > Doesn't work, the compilers seems to think that the code can throw and still
> > assign the variable in the block -_-
> 
> Hmph, that's why I wanted to do it :-D Could we extract this to a method?

Done.

[dgn, 2017-03-15 16:38:15]

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

[commit-bot: I haz the power, 2017-03-15 16:38:36]

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-03-15 17:37:22]

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

[commit-bot: I haz the power, 2017-03-15 17:37:23]

Dry run: This issue passed the CQ dry run.

[Bernhard Bauer, 2017-03-15 21:23:43]

LGTM

https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:90:
String fmtString = "SDK=%s; Installed=%s; Access=%s";
Nit: Now that I think about it, inline the string to below? That would make it
easier to spot parameter mismatches.

[dgn, 2017-03-16 11:21:06]

Thanks!

https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java
(right):

https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:90:
String fmtString = "SDK=%s; Installed=%s; Access=%s";
On 2017/03/15 21:23:43, Bernhard Bauer wrote:
> Nit: Now that I think about it, inline the string to below? That would make it
> easier to spot parameter mismatches.

Done.

[dgn, 2017-03-16 11:21:11]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-03-16 11:21:12]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2751773002/#ps80001
(title: "address comment")

[commit-bot: I haz the power, 2017-03-16 11:21:23]

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

[dgn, 2017-03-16 11:21:40]

The CQ bit was unchecked by dgn@chromium.org

[dgn, 2017-03-16 11:24:49]

The CQ bit was checked by dgn@chromium.org

[commit-bot: I haz the power, 2017-03-16 11:25:08]

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-03-16 11:29:14]

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

[commit-bot: I haz the power, 2017-03-16 11:29:16]

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...)

[dgn, 2017-03-16 11:37:20]

dgn@chromium.org changed reviewers:
+ miguelg@chromium.org, sdefresne@chromium.org

[dgn, 2017-03-16 11:37:20]

miguelg@chromium.org: Please review changes in 
//c/b/ui/android/android_about_app_info.*

sdefresne@chromium.org: Please review changes in 
//components/version_ui_strings.grdp

[sdefresne, 2017-03-16 12:40:03]

https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_s...
File components/version_ui_strings.grdp (right):

https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_s...
components/version_ui_strings.grdp:24: <message name="IDS_VERSION_UI_GMS"
desc="label for the Google Play services info on the about:version page">
I guess this string is specific to Android. Please put it behind <if
expr="is_android"></if>.

[dgn, 2017-03-16 16:53:18]

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

[commit-bot: I haz the power, 2017-03-16 16:53:40]

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

[dgn, 2017-03-16 17:14:03]

PTAL

https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_s...
File components/version_ui_strings.grdp (right):

https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_s...
components/version_ui_strings.grdp:24: <message name="IDS_VERSION_UI_GMS"
desc="label for the Google Play services info on the about:version page">
On 2017/03/16 12:40:02, sdefresne wrote:
> I guess this string is specific to Android. Please put it behind <if
> expr="is_android"></if>.

Done.

[sdefresne, 2017-03-16 17:28:43]

lgtm

https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_...
File components/version_ui_strings.grdp (right):

https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_...
components/version_ui_strings.grdp:24: <if expr="is_android">
nit: please indent <if ...> at the same level as <message> above (this appears
to be more common than the indentation you used).

[dgn, 2017-03-16 18:02:56]

Thanks!

https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_...
File components/version_ui_strings.grdp (right):

https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_...
components/version_ui_strings.grdp:24: <if expr="is_android">
On 2017/03/16 17:28:43, sdefresne wrote:
> nit: please indent <if ...> at the same level as <message> above (this appears
> to be more common than the indentation you used).

Done. Also fixed the indentation of the chromeos block below.

[dgn, 2017-03-16 18:56:32]

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

[commit-bot: I haz the power, 2017-03-16 18:57:02]

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

[Miguel Garcia, 2017-03-16 19:17:59]

lgtm

[Miguel Garcia, 2017-03-16 19:19:37]

lgtm

[dgn, 2017-03-16 19:28:30]

The CQ bit was unchecked by dgn@chromium.org

[dgn, 2017-03-16 19:28:30]

The CQ bit was checked by dgn@chromium.org

[dgn, 2017-03-16 19:28:31]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org,
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2751773002/#ps120001
(title: "fix indentation")

[commit-bot: I haz the power, 2017-03-16 19:29:03]

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-03-16 20:00:49]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489692510811090,
"parent_rev": "744be9cb71e9fc448d2414113f3ad3b0ef1d63ef", "commit_rev":
"4576d3a030396c7542eeb7e07017e4ac1e9ffa28"}

[commit-bot: I haz the power, 2017-03-16 20:01:33]

Description was changed from

==========
Add Google Play services info to chrome://version

Adds a new entry to the version info page that displays the version
number of the client library and the one for the installed Play
Services app. It also displays whether First Party APIs are available.

BUG=None
==========

to

==========
Add Google Play services info to chrome://version

Adds a new entry to the version info page that displays the version
number of the client library and the one for the installed Play
Services app. It also displays whether First Party APIs are available.

BUG=None

Review-Url: https://codereview.chromium.org/2751773002
Cr-Commit-Position: refs/heads/master@{#457527}
Committed:
https://chromium.googlesource.com/chromium/src/+/4576d3a030396c7542eeb7e07017...
==========

[commit-bot: I haz the power, 2017-03-16 20:01:34]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/4576d3a030396c7542eeb7e07017...