Add support for child accounts on Android.

chrome/android/java/src/org/chromium/chrome/browser/child_accounts/ChildAccountService.java

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.child_accounts;
+
+import android.accounts.Account;
+import android.accounts.AccountManager;
+import android.accounts.AccountManagerCallback;
+import android.accounts.AccountManagerFuture;
+import android.accounts.AuthenticatorException;
+import android.accounts.OperationCanceledException;
+import android.content.Context;
+import android.os.AsyncTask;
+import android.util.Log;
+
+import org.chromium.base.CommandLine;
+import org.chromium.chrome.ChromeSwitches;
+import org.chromium.sync.signin.AccountManagerHelper;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.concurrent.ExecutionException;
+
+/**
+ * This class detects child accounts and enables special treatment for them.
+ */
+public class ChildAccountService {
+
+    private static final String TAG = "ChildAccountService";
+
+    /**
+     * An account feature (corresponding to a Gaia service flag) that specifies whether the account
+     * is a child account.
+     */
+    private static final String FEATURE_IS_CHILD_ACCOUNT_KEY = "service_uca";
+
+    /**
+     * The maximum amount of time to wait for the child account check, in milliseconds.
+     */
+    private static final int CHILD_ACCOUNT_TIMEOUT_MS = 1000;
+
+    private static final Object sLock = new Object();
+
+    private static ChildAccountService sChildAccountManager;

[Marc Treib, 2014/12/10 11:53:27]

nit: rename to sChildAccountService

[Bernhard Bauer, 2014/12/11 09:07:58]

Done.

+
+    private final Context mContext;
+
+    // "Boolean" rather than "boolean" to force a crash if hasChildAccount is called too soon

[Marc Treib, 2014/12/10 11:53:27]

So essentially a three-way bool?
http://thedailywtf.com/articles/What_Is_Truth_0x3f_
Not an actual complaint :D
The comment is a bit inaccurate though, as "null" is really just used to mean
"don't know yet".

[Bernhard Bauer, 2014/12/11 09:07:58]

Well, it would be an optional boolean, if Java had a proper type system... ;-)
Yes, but in addition, it's not allowed to call e.g. hasChildAccount() if we
don't know yet. This field being null will automatically enforce that by
crashing in that case :)

[Marc Treib, 2014/12/11 12:21:31]

I'd still argue that the "optional" part is what you actually want, while the
crashing is only a (desirable) side-effect, and so the comment should reflect
that. I'd put the "crash if not known yet" comment down into hasChildAccount.

[Bernhard Bauer, 2014/12/11 13:38:59]

Updated the comment.

+    // (i.e. without calling waitUntilFinished() or checkHasChildAccount()).
+    private Boolean mHasChildAccount;
+
+    private AccountManagerFuture<Boolean> mAccountManagerFuture;
+
+    private final List<HasChildAccountCallback> mCallbacks = new ArrayList<>();
+
+    private ChildAccountService(Context context) {
+        mContext = context;
+    }
+
+    /**
+     * Returns the shared ChildAccountManager instance, creating one if necessary.

[Marc Treib, 2014/12/10 11:53:27]

ChildAccountService

[Bernhard Bauer, 2014/12/11 09:07:58]

Done.

+     * @param context The context to initialize the ChildAccountManager with.
+     * @return The shared instance.
+     */
+    public static ChildAccountService getInstance(Context context) {
+        synchronized (sLock) {
+            if (sChildAccountManager == null) {
+                sChildAccountManager = new ChildAccountService(context.getApplicationContext());
+            }
+        }
+        return sChildAccountManager;
+    }
+
+    /**
+     * A callback to return the result of {@link #checkHasChildAccount}.
+     */
+    public static interface HasChildAccountCallback {
+
+        /**
+         * @param hasChildAccount Whether there is exactly one child account on the device.
+         */
+        public void onChildAccountChecked(boolean hasChildAccount);
+
+    }
+
+    /**
+     * Checks for the presence of child accounts on the device.
+     * @param callback Will be called with the result (see
+     * {@link HasChildAccountCallback#onChildAccountChecked}).
+     */
+    public void checkHasChildAccount(final HasChildAccountCallback callback) {
+        if (mHasChildAccount != null) {
+            callback.onChildAccountChecked(mHasChildAccount);
+            return;
+        }
+
+        Account[] googleAccounts =
+                AccountManagerHelper.get(mContext).getGoogleAccounts();
+        if (googleAccounts.length != 1) {

[Marc Treib, 2014/12/10 11:53:27]

Could this condition potentially change between calls? Seems like that might
break things.

[Bernhard Bauer, 2014/12/11 09:07:58]

Once full Unicorn support is in GMSCore etc., GMSCore will enforce the invariant
that if there is a Unicorn account in a profile, there is no other Google
account in that profile (i.e. you will either have exactly one Unicorn account
and nothing else, or any number of non-Unicorn Google accounts, and no accounts
can be added or removed.

[Marc Treib, 2014/12/11 12:21:31]

Acknowledged.

+            mHasChildAccount = false;
+            callback.onChildAccountChecked(false);
+            return;
+        }
+        Account account = googleAccounts[0];
+
+        if (forceChildAccount(account)) {

[Marc Treib, 2014/12/10 11:53:27]

Should the debug flag work even if there's more than one account on the device?

[Bernhard Bauer, 2014/12/11 09:07:58]

Like I mentioned above, right now I don't really care about the case where
there's more than one account, as that's not supported anyway.

[Marc Treib, 2014/12/11 12:21:31]

This is just a debug flag anyway, right? I was just saying that it might be
convenient if that worked even if I happen to have more than one account on the
device. But it's up to you.

[Bernhard Bauer, 2014/12/11 13:38:59]

Hm. There is some other code in Clank that relies on the fact that we only do
automatic signin if there's a single account on the device (even for the
non-child account case), and that would break.

[Marc Treib, 2014/12/11 15:04:31]

Acknowledged.
It's not worth spending extra effort to make it work. Optionally, you could add
a log message saying something like "hey, fyi, you specified this debug flag but
it won't do anything".

[Bernhard Bauer, 2014/12/11 15:44:18]

Good idea! Done.

+            mHasChildAccount = true;
+            callback.onChildAccountChecked(true);
+            return;
+        }
+
+        mCallbacks.add(callback);
+
+        if (mAccountManagerFuture != null) return;
+
+        final Timer timer = new Timer();
+
+        String[] features = {FEATURE_IS_CHILD_ACCOUNT_KEY};
+        mAccountManagerFuture = AccountManager.get(mContext).hasFeatures(account, features,
+                new AccountManagerCallback<Boolean>() {
+                    @Override
+                    public void run(AccountManagerFuture<Boolean> future) {
+                        assert future == mAccountManagerFuture;
+                        assert future.isDone();
+
+                        timer.cancel();
+
+                        boolean hasChildAccount = hasChildAccount();

[Marc Treib, 2014/12/10 11:53:27]

Should you set mHasChildAccount here?

[Bernhard Bauer, 2014/12/11 09:07:58]

hasChildAccount() will do that.

[Marc Treib, 2014/12/11 12:21:31]

Well, if you do it here (and in waitUntilFinished), then hasChildAccount won't
have to.
Right now, a valid known state can live either in mHasChildAccount or in the
future, and it think that's a bit confusing, so my vote is for populating
mHasChildAccount as soon as we know the value.

[Bernhard Bauer, 2014/12/11 13:38:59]

The problem here is that this callback will be posted to the main run loop, so
in the synchronous case it will actually run after waitUntilFinished has been
called. If we assign mHasChildAccount here and in waitUntilFinished, we won't
actually set it any earlier, we'll just have two places where we set it.

[Marc Treib, 2014/12/11 15:04:31]

I'm not worried about setting it at the earliest possible moment, but rather
there being just one place we have to check. But it sounds like that won't work
anyway :(

+                        for (HasChildAccountCallback callback : mCallbacks) {
+                            callback.onChildAccountChecked(hasChildAccount);
+                        }
+                    }
+                }, null /* handler */);
+
+        timer.schedule(new TimerTask() {
+            @Override
+            public void run() {
+                if (!mAccountManagerFuture.isDone()) mAccountManagerFuture.cancel(true);
+            }}, CHILD_ACCOUNT_TIMEOUT_MS);
+    }
+
+    private boolean forceChildAccount(Account account) {

[Marc Treib, 2014/12/10 11:53:27]

Rename to shouldForceChildAccount?

[Bernhard Bauer, 2014/12/11 09:07:58]

Done.

+        String childAccountName = CommandLine.getInstance().getSwitchValue(
+                ChromeSwitches.CHILD_ACCOUNT);

[Marc Treib, 2014/12/10 11:53:27]

nit: Indentation?

[Bernhard Bauer, 2014/12/11 09:07:58]

8 space indent for continued lines is actually correct; Android Java code is
weird like that.

[Marc Treib, 2014/12/11 12:21:31]

Acknowledged.

+        return childAccountName != null && account.name.equals(childAccountName);
+    }
+
+    private boolean getFutureResult() {
+        try {
+            return mAccountManagerFuture.getResult();
+        } catch (OperationCanceledException e) {
+            Log.e(TAG, "Timed out fetching child account flag: ", e);
+        } catch (AuthenticatorException e) {
+            Log.e(TAG, "Error while fetching child account flag: ", e);
+        } catch (IOException e) {
+            Log.e(TAG, "Error while fetching child account flag: ", e);
+        }
+        return false;
+    }
+
+    /**
+     * Synchronously checks for the presence of child accounts on the device. This method should
+     * only be called after the result has been determined (usually using
+     * {@link #checkHasChildAccount} or {@link #waitUntilFinished} to block).
+     * @return Whether there is a child account on the device.
+     */
+    public boolean hasChildAccount() {
+        // Lazily get the result from the future, so this can be called both from the
+        // AccountManagerCallback and after waiting for the future to be resolved.
+        if (mHasChildAccount == null) {
+            mHasChildAccount = getFutureResult();

[Marc Treib, 2014/12/10 11:53:27]

So this will block on the result, but only if checkHasChildAccount has been
called before? Because otherwise mAccountManagerFuture will be null and so
getFutureResult will crash.

[Bernhard Bauer, 2014/12/11 09:07:58]

Correct! :) The idea here is that you call checkHasChildAccount() early on, then
either wait to get called back, or synchronously wait. It's not the most
convenient to use, but it lets us fire the initial check off early during
startup, and only wait if necessary.

[Marc Treib, 2014/12/11 12:21:31]

Hm. I guess I'd prefer to fire off the check here if it's not running already,
but okay.

[Bernhard Bauer, 2014/12/11 13:38:59]

Yeah, the reason I don't do that is because if someone forgets calling
checkHasChildAccount() beforehand, that would increase startup time during
synchronous startup, but otherwise succeed, so no one would notice. As it is,
forgetting checkHasChildAccount() will actually result in a crash.

[Marc Treib, 2014/12/11 15:04:31]

Acknowledged.

+        }
+        return mHasChildAccount;
+    }
+
+    /**
+     * Waits until we have determined the child account status. Usually you should use callbacks
+     * instead of this method, see {@link #checkHasChildAccount}.
+     */
+    public void waitUntilFinished() {

[Marc Treib, 2014/12/10 11:53:27]

What does this method do, since it doesn't actually block on the result? It also
doesn't seem to be used anywhere?

[Bernhard Bauer, 2014/12/11 09:07:58]

getFutureResult() will block. I updated the comment to explain that.

And this is called downstream (like checkHasChildAccount).

[Marc Treib, 2014/12/11 12:21:31]

Ah, I missed the ".get()" at the end. So blocking on the future is only allowed
in a background thread, but blocking on a random AsyncTask is fine?! Oh well...

[Bernhard Bauer, 2014/12/11 13:38:59]

Welcome to Android :)

I do feel somewhat bad for sidestepping the restriction that waiting on an
AccountManager future on the main thread is not allowed, but then again when we
call this we are already waiting on the main thread for the native library to be
loaded.

[Marc Treib, 2014/12/11 15:04:31]

Acknowledged.

+        if (mAccountManagerFuture == null) return;
+        if (mAccountManagerFuture.isDone()) return;
+
+        // Waiting on the account manager future is only allowed on a background thread.
+        try {
+            new AsyncTask<Void, Void, Void>() {
+                @Override
+                protected Void doInBackground(Void... params) {
+                    getFutureResult();
+                    return null;
+                }
+            }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR).get();
+        } catch (ExecutionException e) {
+            Log.w(TAG, "Error while fetching child account flag: ", e);
+        } catch (InterruptedException e) {
+            Log.w(TAG, "Interrupted while fetching child account flag: ", e);
+        }
+    }
+
+    public void onChildAccountSigninComplete() {
+        nativeOnChildAccountSigninComplete();
+    }
+
+    private native void nativeOnChildAccountSigninComplete();
+}