Add profile supplier param to IdentityDiscController constructor.

This CL adds |mProfileSupplier| local variable that is observed for
profile changes. |mProfileSupplier| is used to pass profile param to
IdentityServicesProvider.

By this CL;
- |mProfileSupplier| notifies the class when the current profile is
changed.
- When current profile changes, setProfile function is triggered and
mIdentityManager is updated with the new profile.
- Introducing |getSyncAccountInfo| function that uses current profile
(i.e., off-the-record or regular) and returns account information. This
function returns null for OTR profiles.
- Passing current profile to tracker instead of using always regular
profile to fix unsafe usage of profile.
- Adding |testIdentityDiscWithSwitchToIncognito| to test identity disc
disappear when switching to incognito NTP from sign-in state.

Bug: 1041781, 1075562, 1048632
Change-Id: Icd443c03033b5a20c1ba0b680f57c05dfc21222d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2307253
Commit-Queue: Side YILMAZ <sideyilmaz@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803158}

chrome/android/java/src/org/chromium/chrome/browser/identity_disc/IdentityDiscController.java

@@ -68,6 +68,8 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
     private final Context mContext;
     private ActivityLifecycleDispatcher mActivityLifecycleDispatcher;
     private final ObservableSupplier<Boolean> mBottomToolbarVisibilitySupplier;
+    private final ObservableSupplier<Profile> mProfileSupplier;
+    private final Callback<Profile> mProfileSupplierObserver = this::setProfile;
 
     private @Nullable Callback<Boolean> mBottomToolbarVisibilityObserver;
 
@@ -98,10 +100,12 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
      */
     public IdentityDiscController(Context context,
             ActivityLifecycleDispatcher activityLifecycleDispatcher,
-            ObservableSupplier<Boolean> bottomToolbarVisibilitySupplier) {
+            ObservableSupplier<Boolean> bottomToolbarVisibilitySupplier,
+            ObservableSupplier<Profile> profileSupplier) {
         mContext = context;
         mActivityLifecycleDispatcher = activityLifecycleDispatcher;
         mBottomToolbarVisibilitySupplier = bottomToolbarVisibilitySupplier;
+        mProfileSupplier = profileSupplier;
         mActivityLifecycleDispatcher.register(this);
 
         mButtonData = new ButtonData(false, null,
@@ -128,11 +132,10 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
         mActivityLifecycleDispatcher = null;
         mNativeIsInitialized = true;
 
-        mIdentityManager = IdentityServicesProvider.get().getIdentityManager();
-        mIdentityManager.addObserver(this);
+        mProfileSupplier.addObserver(mProfileSupplierObserver);
 
-        mBottomToolbarVisibilityObserver = (bottomToolbarIsVisible)
-                -> notifyObservers(mIdentityManager.getPrimaryAccountInfo() != null);
+        mBottomToolbarVisibilityObserver =
+                (bottomToolbarIsVisible) -> notifyObservers(getSyncAccountInfo() != null);
         mBottomToolbarVisibilitySupplier.addObserver(mBottomToolbarVisibilityObserver);
     }
 
@@ -174,8 +177,7 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
             return;
         }
 
-        String email = CoreAccountInfo.getEmailFrom(
-                mIdentityManager.getPrimaryAccountInfo(ConsentLevel.SYNC));
+        String email = CoreAccountInfo.getEmailFrom(getSyncAccountInfo());
         boolean canShowIdentityDisc = email != null;
         boolean menuBottomOnBottom =
                 bottomToolbarVisible && BottomToolbarVariationManager.isMenuButtonOnBottom();
@@ -247,8 +249,7 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
         if (mState == IdentityDiscState.NONE) return;
         assert mProfileDataCache[mState] != null;
 
-        CoreAccountInfo accountInfo = mIdentityManager.getPrimaryAccountInfo();
-        if (accountEmail.equals(CoreAccountInfo.getEmailFrom(accountInfo))) {
+        if (accountEmail.equals(CoreAccountInfo.getEmailFrom(getSyncAccountInfo()))) {
             notifyObservers(true);
         }
     }
@@ -291,6 +292,10 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
             mBottomToolbarVisibilitySupplier.removeObserver(mBottomToolbarVisibilityObserver);
             mBottomToolbarVisibilityObserver = null;
         }
+
+        if (mNativeIsInitialized) {
+            mProfileSupplier.removeObserver(mProfileSupplierObserver);
+        }
     }
 
     /**
@@ -298,12 +303,36 @@ public class IdentityDiscController implements NativeInitObserver, ProfileDataCa
      * whether to show in-product help.
      */
     private void recordIdentityDiscUsed() {
-        // TODO (https://crbug.com/1048632): Use the current profile (i.e., regular profile or
-        // incognito profile) instead of always using regular profile. It works correctly now, but
-        // it is not safe.
-        Profile profile = Profile.getLastUsedRegularProfile();
-        Tracker tracker = TrackerFactory.getTrackerForProfile(profile);
+        assert mProfileSupplier != null && mProfileSupplier.get() != null;
+        Tracker tracker = TrackerFactory.getTrackerForProfile(mProfileSupplier.get());
         tracker.notifyEvent(EventConstants.IDENTITY_DISC_USED);
         RecordUserAction.record("MobileToolbarIdentityDiscTap");
     }
+
+    /**
+     * Returns the account info of mIdentityManager if current profile is regular, and
+     * null for off-the-record ones.
+     * @return account info for the current profile. Returns null for OTR profile.
+     */
+    private CoreAccountInfo getSyncAccountInfo() {
+        return mIdentityManager != null ? mIdentityManager.getPrimaryAccountInfo(ConsentLevel.SYNC)
+                                        : null;
+    }
+
+    /**
+     * Triggered by mProfileSupplierObserver when profile is changed in mProfileSupplier.
+     * mIdentityManager is updated with the profile, as set to null if profile is off-the-record.
+     */
+    private void setProfile(Profile profile) {
+        if (mIdentityManager != null) {
+            mIdentityManager.removeObserver(this);
+        }
+
+        if (profile.isOffTheRecord()) {
+            mIdentityManager = null;
+        } else {
+            mIdentityManager = IdentityServicesProvider.get().getIdentityManager(profile);
+            mIdentityManager.addObserver(this);
+        }
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/ui/RootUiCoordinator.java

@@ -504,8 +504,9 @@ public class RootUiCoordinator
                     new ObservableSupplierImpl<>();
             bottomToolbarVisibilitySupplier.set(false);
 
-            mIdentityDiscController = new IdentityDiscController(
-                    mActivity, mActivity.getLifecycleDispatcher(), bottomToolbarVisibilitySupplier);
+            mIdentityDiscController =
+                    new IdentityDiscController(mActivity, mActivity.getLifecycleDispatcher(),
+                            bottomToolbarVisibilitySupplier, mProfileSupplier);
             ShareButtonController shareButtonController = new ShareButtonController(mActivity,
                     mActivityTabProvider, mShareDelegateSupplier, new ShareUtils(),
                     bottomToolbarVisibilitySupplier, mActivity.getLifecycleDispatcher(),

chrome/android/javatests/src/org/chromium/chrome/browser/identity_disc/IdentityDiscControllerTest.java

@@ -106,6 +106,18 @@ public class IdentityDiscControllerTest {
                 withEffectiveVisibility(ViewMatchers.Visibility.GONE)));
     }
 
+    @Test
+    @MediumTest
+    public void testIdentityDiscWithSwitchToIncognito() {
+        mAccountManagerTestRule.addAndSignInTestAccount();
+        waitForView(allOf(withId(R.id.optional_toolbar_button), isDisplayed()));
+
+        // Identity Disc should not be visible, when switched from sign in state to incognito NTP.
+        mActivityTestRule.newIncognitoTabFromMenu();
+        waitForView(allOf(withId(R.id.optional_toolbar_button),
+                withEffectiveVisibility(ViewMatchers.Visibility.GONE)));
+    }
+
     private void leaveNTP() {
         mActivityTestRule.loadUrl(ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL);
         ChromeTabUtils.waitForTabPageLoaded(mTab, ContentUrlConstants.ABOUT_BLANK_DISPLAY_URL);