Fixed permission check in SystemAccountManagerDelegate.updateCredentials

components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java

Index: components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java
diff --git a/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java b/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java
index ebbc3ddaaaa8e726fa7f027b8b75d7925ed40c3e..1f27097c1f1297f8058ec91fb10aa9433ced9998 100644
--- a/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java
+++ b/components/signin/core/browser/android/java/src/org/chromium/components/signin/SystemAccountManagerDelegate.java
@@ -16,6 +16,7 @@ import android.app.Activity;
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.os.AsyncTask;
+import android.os.Build;
 import android.os.Bundle;
 import android.os.Process;
 import android.os.SystemClock;
@@ -161,13 +162,15 @@ public class SystemAccountManagerDelegate implements AccountManagerDelegate {
     public void updateCredentials(
             Account account, Activity activity, final Callback<Boolean> callback) {
         ThreadUtils.assertOnUiThread();
-        if (!hasGetAccountsPermission()) {
-            ThreadUtils.postOnUiThread(new Runnable() {
-                @Override
-                public void run() {
-                    callback.onResult(false);
-                }
-            });
+        if (!hasManageAccountsPermission()) {
+            if (callback != null) {

[nyquist, 2017/03/10 19:07:45]

if (callback == null) return;

It reduces indents, etc...

[bsazonov, 2017/03/13 12:27:12]

The Style Guide allows both one-line and braced if in this case, so it's up to
the developer (and the code consistency). I think that returns on separate lines
are easier to spot (and the rest of the file uses braced ifs anyway).

If you want, I can replace it with:

if (!hasManageAccountsPermission()) {
    if (callback == null) {
        return;
    }
    ThreadUtils.postOnUiThread(new Runnable() {
        @Override
        public void run() {
            callback.onResult(false);
        }
    });
    return;
}

Should I?

+                ThreadUtils.postOnUiThread(new Runnable() {
+                    @Override
+                    public void run() {
+                        callback.onResult(false);
+                    }
+                });
+            }
             return;
         }
 
@@ -198,4 +201,13 @@ public class SystemAccountManagerDelegate implements AccountManagerDelegate {
                        Manifest.permission.GET_ACCOUNTS, Process.myPid(), Process.myUid())
                 == PackageManager.PERMISSION_GRANTED;
     }
+
+    protected boolean hasManageAccountsPermission() {
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {

[nyquist, 2017/03/10 19:07:45]

Optional nit: This can just be on one line. Fine like this too though.

[bsazonov, 2017/03/13 12:27:12]

See previous comment.

+            return true;
+        }
+        return ApiCompatibilityUtils.checkPermission(mApplicationContext,
+                       "android.permission.MANAGE_ACCOUNTS", Process.myPid(), Process.myUid())
+                == PackageManager.PERMISSION_GRANTED;
+    }
 }