Properly destroy TabModelSelectorTabObserver in TabGroupUiMediator

Currently, when the activity is destroyed and re-created due to
dark/light mode switching, the TabModelSelectorTabObserver in
TabGroupUiMediator is not properly destroyed. This CL fixes this issue
by destroying it when tab is detached from activity.

Bug: 1087826
Change-Id: Ia11138923cb9d4e16e2ce35db18ca74afa50412f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2227890Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774692}

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGroupUiMediator.java

@@ -46,6 +46,7 @@ import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
 import org.chromium.chrome.tab_ui.R;
 import org.chromium.components.embedder_support.util.UrlConstants;
 import org.chromium.content_public.browser.LoadUrlParams;
+import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.modelutil.PropertyModel;
 
 import java.util.ArrayList;
@@ -108,13 +109,13 @@ public class TabGroupUiMediator implements SnackbarManager.SnackbarController {
     private final TabGridDialogMediator.DialogController mTabGridDialogController;
     private final ThemeColorProvider.ThemeColorObserver mThemeColorObserver;
     private final ThemeColorProvider.TintObserver mTintObserver;
-    private final TabModelSelectorTabObserver mTabModelSelectorTabObserver;
     private final TabModelSelectorObserver mTabModelSelectorObserver;
     private final ActivityLifecycleDispatcher mActivityLifecycleDispatcher;
     private final SnackbarManager.SnackbarManageable mSnackbarManageable;
     private final Snackbar mUndoClosureSnackBar;
     private TabGroupModelFilter.Observer mTabGroupModelFilterObserver;
     private PauseResumeWithNativeObserver mPauseResumeWithNativeObserver;
+    private TabModelSelectorTabObserver mTabModelSelectorTabObserver;
     private boolean mIsTabGroupUiVisible;
     private boolean mIsShowingOverViewMode;
     private boolean mActivatedButNotShown;
@@ -261,6 +262,16 @@ public class TabGroupUiMediator implements SnackbarManager.SnackbarController {
                 if (!mIsTabGroupUiVisible || numTabs == 1) numTabs = 0;
                 RecordHistogram.recordCountHistogram("TabStrip.TabCountOnPageLoad", numTabs);
             }
+
+            @Override
+            public void onActivityAttachmentChanged(Tab tab, WindowAndroid window) {
+                // Remove this when tab is detached since the TabModelSelectorTabObserver is not
+                // properly destroyed when there is a normal/night mode switch.
+                if (window == null) {
+                    this.destroy();
+                    mTabModelSelectorTabObserver = null;
+                }
+            }
         };
 
         mTabModelSelectorObserver = new EmptyTabModelSelectorObserver() {
@@ -488,10 +499,12 @@ public class TabGroupUiMediator implements SnackbarManager.SnackbarController {
         if (mPauseResumeWithNativeObserver != null) {
             mActivityLifecycleDispatcher.unregister(mPauseResumeWithNativeObserver);
         }
+        if (mTabModelSelectorTabObserver != null) {
+            mTabModelSelectorTabObserver.destroy();
+        }
         mOverviewModeBehavior.removeOverviewModeObserver(mOverviewModeObserver);
         mThemeColorProvider.removeThemeColorObserver(mThemeColorObserver);
         mThemeColorProvider.removeTintObserver(mTintObserver);
-        mTabModelSelectorTabObserver.destroy();
     }
 
     private void maybeActivateConditionalTabStrip(@ReasonToShow int reason) {

chrome/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabGroupUiMediatorUnitTest.java

@@ -50,6 +50,7 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabCreationState;
 import org.chromium.chrome.browser.tab.TabImpl;
 import org.chromium.chrome.browser.tab.TabLaunchType;
+import org.chromium.chrome.browser.tab.TabObserver;
 import org.chromium.chrome.browser.tab.TabSelectionType;
 import org.chromium.chrome.browser.tabmodel.TabCreatorManager;
 import org.chromium.chrome.browser.tabmodel.TabModel;
@@ -142,6 +143,8 @@ public class TabGroupUiMediatorUnitTest {
     ArgumentCaptor<TabGroupModelFilter.Observer> mTabGroupModelFilterObserverArgumentCaptor;
     @Captor
     ArgumentCaptor<PauseResumeWithNativeObserver> mPauseResumeWithNativeObserverArgumentCaptor;
+    @Captor
+    ArgumentCaptor<TabObserver> mTabObserverCaptor;
 
     private TabImpl mTab1;
     private TabImpl mTab2;
@@ -260,6 +263,9 @@ public class TabGroupUiMediatorUnitTest {
         doReturn(POSITION1).when(mTabModel).indexOf(mTab1);
         doReturn(POSITION2).when(mTabModel).indexOf(mTab2);
         doReturn(POSITION3).when(mTabModel).indexOf(mTab3);
+        doNothing().when(mTab1).addObserver(mTabObserverCaptor.capture());
+        doNothing().when(mTab2).addObserver(mTabObserverCaptor.capture());
+        doNothing().when(mTab3).addObserver(mTabObserverCaptor.capture());
 
         if (TabUiFeatureUtilities.isConditionalTabStripEnabled()) {
             doReturn(false).when(mTabModelFilter).isIncognito();
@@ -1287,4 +1293,19 @@ public class TabGroupUiMediatorUnitTest {
         assertThat(
                 mModel.get(TabGroupUiProperties.LEFT_BUTTON_ON_CLICK_LISTENER), equalTo(listener));
     }
+
+    @Test
+    public void testTabModelSelectorTabObserverDestroyWhenDetach() {
+        InOrder tabObserverDestroyInOrder = inOrder(mTab1);
+        initAndAssertProperties(mTab1);
+
+        mTabObserverCaptor.getValue().onActivityAttachmentChanged(mTab1, null);
+
+        tabObserverDestroyInOrder.verify(mTab1).removeObserver(mTabObserverCaptor.capture());
+
+        mTabGroupUiMediator.destroy();
+
+        tabObserverDestroyInOrder.verify(mTab1, never())
+                .removeObserver(mTabObserverCaptor.capture());
+    }
 }