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

Issue 1136633005: Add a new utility method to check the availability of Google Play Services. (Closed)

Created:
5 years, 7 months ago by Andrew Hayden (chromium.org)
Modified:
5 years, 7 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a new utility method to check the availability of Google Play Services. All code needing to check Google Play Services availability should migrate to this API and specify an explicit failure handling policy. Several policies are provided by default, including a "silent" policy that is identical in behavior to what happens today in most code using Google Play Services. In addition to the "silent" policy, the method supports standardized mechanisms from Google Play such as a modal dialog or a system notification. BUG= Committed: https://crrev.com/31af4784464fac0898e909bfb2d7972ae55c106b Cr-Commit-Position: refs/heads/master@{#330746}

Patch Set 1 #

Total comments: 1

Patch Set 2 : Use verbose logging instead of debug logging #

Patch Set 3 : Use verbose logging and non-static boolean field #

Total comments: 26

Patch Set 4 : Extract enum to helper class and clean up logic accordingly #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -2 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java View 1 2 3 3 chunks +32 lines, -2 lines 2 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java View 1 2 3 1 chunk +137 lines, -0 lines 2 comments Download

Messages

Total messages: 21 (6 generated)
andrewhayden
https://codereview.chromium.org/1136633005/diff/1/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java File chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java (right): https://codereview.chromium.org/1136633005/diff/1/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java#newcode227 chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java:227: Log.d(TAG, "Unable to use Google Play Services: " This ...
5 years, 7 months ago (2015-05-19 16:32:27 UTC) #2
andrewhayden
Bernhard and Nicolas, can you PTAL?
5 years, 7 months ago (2015-05-19 16:41:24 UTC) #5
dgn
So there are 2 different ways of displaying an error, but only one can be ...
5 years, 7 months ago (2015-05-19 17:21:00 UTC) #7
Bernhard Bauer
https://codereview.chromium.org/1136633005/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java File chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java (right): https://codereview.chromium.org/1136633005/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java#newcode150 chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java:150: return false; BTW, I think the internal subclass can ...
5 years, 7 months ago (2015-05-19 17:48:01 UTC) #9
dgn
https://codereview.chromium.org/1136633005/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java File chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java (right): https://codereview.chromium.org/1136633005/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java#newcode150 chrome/android/java/src/org/chromium/chrome/browser/externalauth/ExternalAuthUtils.java:150: return false; On 2015/05/19 17:48:01, Bernhard Bauer wrote: > ...
5 years, 7 months ago (2015-05-19 17:59:52 UTC) #10
Andrew Hayden (chromium.org)
OK, new patchset up. dgn@: Yes, the cast code should use this. Not sure what ...
5 years, 7 months ago (2015-05-20 15:27:57 UTC) #11
Bernhard Bauer
Nice, LGTM!
5 years, 7 months ago (2015-05-20 15:36:19 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1136633005/60001
5 years, 7 months ago (2015-05-20 16:17:48 UTC) #14
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 7 months ago (2015-05-20 16:21:00 UTC) #15
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/31af4784464fac0898e909bfb2d7972ae55c106b Cr-Commit-Position: refs/heads/master@{#330746}
5 years, 7 months ago (2015-05-20 16:21:53 UTC) #16
dgn
Looking at https://developer.android.com/google/auth/api-client.html#HandlingFailures, there are some errors that might require the user to sign in ...
5 years, 7 months ago (2015-05-20 16:33:07 UTC) #17
dgn
uh too late
5 years, 7 months ago (2015-05-20 16:33:19 UTC) #18
andrewhayden
Sorry, I jumped the gun. I can do a subsequent CL if necessary, responses inline ...
5 years, 7 months ago (2015-05-20 16:42:28 UTC) #19
dgn
> > https://codereview.chromium.org/1136633005/diff/60001/chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java#newcode122 > > chrome/android/java/src/org/chromium/chrome/browser/externalauth/UserRecoverableErrorHandler.java:122: > > public ModalDialog(Activity activity, int errorCode, int requestCode, ...
5 years, 7 months ago (2015-05-20 16:51:41 UTC) #20
Andrew Hayden (chromium.org)
5 years, 7 months ago (2015-05-21 12:01:00 UTC) #21
Message was sent while issue was closed.
I've uploaded https://codereview.chromium.org/1145313006 to fix the remaining
issues pointed out by dgn@ (again, sorry) and to fix problems found while trying
to integrate downstream code with the upstream API that landed in this CL.

Powered by Google App Engine
This is Rietveld 408576698