Unified Bookmarks and Recent Tabs signin methods.

chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java
index d7531427088b067a30820ae49fcc6c172765ae04..b9522905c9749e78cfee6fda498dcd5334132034 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/AccountSigninActivity.java
@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.signin;
 
+import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
 import android.support.v7.app.AppCompatActivity;
@@ -19,6 +20,7 @@ import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.preferences.Preferences;
 import org.chromium.chrome.browser.preferences.PreferencesLauncher;
 import org.chromium.chrome.browser.profiles.Profile;
+import org.chromium.chrome.browser.signin.SigninManager.SignInCallback;
 import org.chromium.chrome.browser.sync.ui.SyncCustomizationFragment;
 
 /**
@@ -26,16 +28,27 @@ import org.chromium.chrome.browser.sync.ui.SyncCustomizationFragment;
  * sign in to. The AccountSigninView.Delegate interface is fulfilled by the AppCompatActivity.
  */
 public class AccountSigninActivity extends AppCompatActivity
-        implements AccountSigninView.Listener, AccountSigninView.Delegate,
-                   SigninManager.SignInCallback{
-    private static final String TAG = "SigninActivity";
-
-    private static final String CONFIRM_IMPORT_SYNC_DATA_DIALOG_TAG =
-            "signin_import_data_tag";
+        implements AccountSigninView.Listener, AccountSigninView.Delegate {
+    private static final String TAG = "AccountSigninActivity";
+    private static final String INTENT_SIGNIN_ACCESS_POINT =
+            "AccountSigninActivity.SigninAccessPoint";
 
     private AccountSigninView mView;
-    private String mAccountName;
-    private boolean mShowSyncSettings = false;
+    private ProfileDataCache mProfileDataCache;
+    private int mAccessPoint;
+
+    /**
+     * A convenience method to create a AccountSigninActivity passing the access point as an
+     * intent.
+     * @param accessPoint - A SigninAccessPoint designating where the activity is created from.
+     */
+    public static void startAccountSigninActivity(Context context, int accessPoint) {
+        assert accessPoint > 0 && accessPoint < SigninAccessPoint.MAX : "invalid access point";

[Bernhard Bauer, 2016/04/20 10:59:06]

Technically, 0 is a valid enum value here (it's ACCESS_POINT_START_PAGE). I
don't think it's used on Android, but if we want to verify that (as opposed to
just verifying that it's in the general range), we should explicitly check for
the all the values that we know are used on Android.

[PEConn, 2016/04/25 11:21:23]

Done.

+
+        Intent intent = new Intent(context, AccountSigninActivity.class);
+        intent.putExtra(INTENT_SIGNIN_ACCESS_POINT, accessPoint);
+        context.startActivity(intent);
+    }
 
     @Override
     @SuppressFBWarnings("DM_EXIT")
@@ -54,55 +67,98 @@ public class AccountSigninActivity extends AppCompatActivity
         // We don't trust android to restore the saved state correctly, so pass null.
         super.onCreate(null);
 
+        mAccessPoint = getIntent().getIntExtra(INTENT_SIGNIN_ACCESS_POINT, -1);
+        assert mAccessPoint != -1
+                : "A access point must be provided when starting BookmarkSigninActivity";

[Bernhard Bauer, 2016/04/20 10:59:06]

Nit: "An access point" :)

[PEConn, 2016/04/25 11:21:23]

Done.

+
+        if (savedInstanceState == null && getAccessPoint() == SigninAccessPoint.BOOKMARK_MANAGER) {
+            RecordUserAction.record("Stars_SignInPromoActivity_Launched");
+        }
+
         mView = (AccountSigninView) LayoutInflater.from(this).inflate(
                 R.layout.account_signin_view, null);
-        mView.init(new ProfileDataCache(this, Profile.getLastUsedProfile()));
+        mView.init(getProfileDataCache());
         mView.setListener(this);
         mView.setDelegate(this);
 
+        if (getAccessPoint() == SigninAccessPoint.BOOKMARK_MANAGER
+                || getAccessPoint() == SigninAccessPoint.RECENT_TABS) {
+            mView.configureForRecentTabsOrBookmarksPage();
+        }
+
+        SigninManager.logSigninStartAccessPoint(getAccessPoint());
+
         setContentView(mView);
     }
 
     @Override
-    public void onAccountSelectionCanceled() {
-        finish();
+    public void onDestroy() {
+        super.onDestroy();
+
+        if (mProfileDataCache != null) {
+            mProfileDataCache.destroy();
+            mProfileDataCache = null;
+        }
     }
 
-    @Override
-    public void onNewAccount() {
-        AccountAdder.getInstance().addAccount(this, AccountAdder.ADD_ACCOUNT_RESULT);
+    public ProfileDataCache getProfileDataCache() {
+        if (mProfileDataCache == null) {
+            mProfileDataCache = new ProfileDataCache(this, Profile.getLastUsedProfile());
+        }
+        return mProfileDataCache;
+    }
+
+    public int getAccessPoint() {
+        return mAccessPoint;
     }
 
     @Override
-    public void onAccountSelected(String accountName, boolean settingsClicked) {
-        mShowSyncSettings = settingsClicked;
-        mAccountName = accountName;
-        RecordUserAction.record("Signin_Signin_FromSettings");
-        SigninManager.get(this).signIn(accountName, this, this);
+    public void onAccountSelectionCanceled() {
+        finish();
     }
 
     @Override
-    public void onFailedToSetForcedAccount(String forcedAccountName) {
-        assert false : "No forced accounts in account switching preferences.";
+    public void onNewAccount() {
+        if (getAccessPoint() == SigninAccessPoint.BOOKMARK_MANAGER) {
+            RecordUserAction.record("Stars_SignInPromoActivity_NewAccount");
+        }
 
+        AccountAdder.getInstance().addAccount(this, AccountAdder.ADD_ACCOUNT_RESULT);
     }
 
     @Override
-    public void onSignInComplete() {
-        if (mShowSyncSettings) {
-            Intent intent = PreferencesLauncher.createIntentForSettingsPage(this,
-                    SyncCustomizationFragment.class.getName());
-            Bundle args = new Bundle();
-            args.putString(SyncCustomizationFragment.ARGUMENT_ACCOUNT, mAccountName);
-            intent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, args);
-            startActivity(intent);
+    public void onAccountSelected(final String accountName, final boolean settingsClicked) {
+        if (getAccessPoint() == SigninAccessPoint.BOOKMARK_MANAGER) {

[Bernhard Bauer, 2016/04/20 10:59:06]

Use a switch statement?

[PEConn, 2016/04/25 11:21:23]

I'm not sure it looks that much neater. Done though, mainly for the novelty of
getting to use a switch statement.

[Bernhard Bauer, 2016/04/26 14:51:20]

Whatever makes you happy, man 😉

FWIW, I do think it looks nicer, and it has the advantage of the compiler
checking that you e.g. don't accidentally duplicate values.

[PEConn, 2016/04/27 10:19:06]

Acknowledged.

+            RecordUserAction.record("Stars_SignInPromoActivity_SignedIn");
+            RecordUserAction.record("Signin_Signin_FromBookmarkManager");
+        } else if (getAccessPoint() == SigninAccessPoint.RECENT_TABS) {
+            RecordUserAction.record("Signin_Signin_FromRecentTabs");
+        } else if (getAccessPoint() == SigninAccessPoint.SETTINGS) {
+            RecordUserAction.record("Signin_Signin_FromSettings");
         }
 
-        finish();
+        final Context context = this;
+        SigninManager.get(this).signIn(accountName, this, new SignInCallback(){
+
+            @Override
+            public void onSignInComplete() {
+                if (settingsClicked) {
+                    Intent intent = PreferencesLauncher.createIntentForSettingsPage(context,
+                            SyncCustomizationFragment.class.getName());
+                    Bundle args = new Bundle();
+                    args.putString(SyncCustomizationFragment.ARGUMENT_ACCOUNT, accountName);
+                    intent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, args);
+                    startActivity(intent);
+                }
+
+                finish();
+            }
+
+            @Override
+            public void onSignInAborted() {}
+        });
     }
 
     @Override
-    public void onSignInAborted() {
-        assert false : "Signin cannot be aborted when forced.";
-    }
+    public void onFailedToSetForcedAccount(String forcedAccountName) {}
 }