Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(166)

Issue 2751773002: Add Google Play services info to chrome://version (Closed)

Created:
3 years, 9 months ago by dgn
Modified:
3 years, 9 months ago
CC:
chromium-reviews, oshima+watch_chromium.org, agrieve+watch_chromium.org, Kingston T
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 8

Patch Set 2 : address comments #

Patch Set 3 : refactor getting apk version to allow final variable #

Patch Set 4 : rebase #

Total comments: 2

Patch Set 5 : address comment #

Total comments: 2

Patch Set 6 : add the string only in android #

Total comments: 2

Patch Set 7 : fix indentation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+96 lines, -11 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java View 1 2 3 4 2 chunks +49 lines, -0 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/android/android_about_app_info.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/android/android_about_app_info.cc View 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/version_ui.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M components/version_ui/resources/about_version.html View 1 chunk +8 lines, -0 lines 0 comments Download
M components/version_ui/version_ui_constants.h View 1 chunk +2 lines, -0 lines 0 comments Download
M components/version_ui/version_ui_constants.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M components/version_ui_strings.grdp View 1 2 3 4 5 6 1 chunk +16 lines, -11 lines 0 comments Download

Messages

Total messages: 46 (26 generated)
dgn
PTAL. Preview: https://goo.gl/photos/FrhtWMdJEuUivCpdA
3 years, 9 months ago (2017-03-14 18:27:11 UTC) #3
dgn
ktam@: FYI, since you recently asked about it
3 years, 9 months ago (2017-03-14 18:29:00 UTC) #5
Bernhard Bauer
Neat! https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode93 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93: long installedGmsVersion; Make this final to ensure we ...
3 years, 9 months ago (2017-03-14 18:31:55 UTC) #6
dgn
PTAL https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode93 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93: long installedGmsVersion; On 2017/03/14 18:31:54, Bernhard Bauer wrote: ...
3 years, 9 months ago (2017-03-15 10:53:56 UTC) #11
Bernhard Bauer
https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode93 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93: long installedGmsVersion; On 2017/03/15 10:53:55, dgn wrote: > On ...
3 years, 9 months ago (2017-03-15 11:55:43 UTC) #12
dgn
PTAL https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/1/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode93 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:93: long installedGmsVersion; On 2017/03/15 11:55:43, Bernhard Bauer wrote: ...
3 years, 9 months ago (2017-03-15 16:38:08 UTC) #13
Bernhard Bauer
LGTM https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode90 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:90: String fmtString = "SDK=%s; Installed=%s; Access=%s"; Nit: Now ...
3 years, 9 months ago (2017-03-15 21:23:43 UTC) #18
dgn
Thanks! https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java File chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java (right): https://codereview.chromium.org/2751773002/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java#newcode90 chrome/android/java/src/org/chromium/chrome/browser/ChromeVersionInfo.java:90: String fmtString = "SDK=%s; Installed=%s; Access=%s"; On 2017/03/15 ...
3 years, 9 months ago (2017-03-16 11:21:06 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2751773002/80001
3 years, 9 months ago (2017-03-16 11:21:23 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2751773002/80001
3 years, 9 months ago (2017-03-16 11:25:08 UTC) #25
commit-bot: I haz the power
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_presubmit/builds/387009)
3 years, 9 months ago (2017-03-16 11:29:16 UTC) #27
dgn
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
3 years, 9 months ago (2017-03-16 11:37:20 UTC) #29
sdefresne
https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_strings.grdp File components/version_ui_strings.grdp (right): https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_strings.grdp#newcode24 components/version_ui_strings.grdp:24: <message name="IDS_VERSION_UI_GMS" desc="label for the Google Play services info ...
3 years, 9 months ago (2017-03-16 12:40:03 UTC) #30
dgn
PTAL https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_strings.grdp File components/version_ui_strings.grdp (right): https://codereview.chromium.org/2751773002/diff/80001/components/version_ui_strings.grdp#newcode24 components/version_ui_strings.grdp:24: <message name="IDS_VERSION_UI_GMS" desc="label for the Google Play services ...
3 years, 9 months ago (2017-03-16 17:14:03 UTC) #33
sdefresne
lgtm https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_strings.grdp File components/version_ui_strings.grdp (right): https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_strings.grdp#newcode24 components/version_ui_strings.grdp:24: <if expr="is_android"> nit: please indent <if ...> at ...
3 years, 9 months ago (2017-03-16 17:28:43 UTC) #34
dgn
Thanks! https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_strings.grdp File components/version_ui_strings.grdp (right): https://codereview.chromium.org/2751773002/diff/100001/components/version_ui_strings.grdp#newcode24 components/version_ui_strings.grdp:24: <if expr="is_android"> On 2017/03/16 17:28:43, sdefresne wrote: > ...
3 years, 9 months ago (2017-03-16 18:02:56 UTC) #35
Miguel Garcia
lgtm
3 years, 9 months ago (2017-03-16 19:17:59 UTC) #38
Miguel Garcia
lgtm
3 years, 9 months ago (2017-03-16 19:19:37 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2751773002/120001
3 years, 9 months ago (2017-03-16 19:29:03 UTC) #43
commit-bot: I haz the power
3 years, 9 months ago (2017-03-16 20:01:34 UTC) #46
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/4576d3a030396c7542eeb7e07017...

Powered by Google App Engine
This is Rietveld 408576698