Remove TabObservers by default when Tabs are detached from the window

Also drops the requirement that every TabObserver care about activity
detachment events by default. Interested clients can still override this
method and get custom behavior.

Bug: 1157728, 1146320
Change-Id: Icb4ca185fb46075b141aa75d8c0ff57a187f5143
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585996Reviewed-by: Mei Liang <meiliang@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837421}

chrome/android/java/src/org/chromium/chrome/browser/tab/TabImpl.java

@@ -264,6 +264,11 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
         mObservers.removeObserver(observer);
     }
 
+    @Override
+    public boolean hasObserver(TabObserver observer) {
+        return mObservers.hasObserver(observer);
+    }
+
     @Override
     public UserDataHost getUserDataHost() {
         return mUserDataHost;

chrome/android/javatests/src/org/chromium/chrome/browser/TabObserverTest.java

@@ -16,6 +16,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Restriction;
@@ -104,4 +105,11 @@ public class TabObserverTest {
         interactabilityHelper.waitForCallback(interactableCallCount);
         assertFalse("Tab should not be interactable.", mTab.isUserInteractable());
     }
+
+    @Test
+    @SmallTest
+    public void testTabDetach_observerUnregistered() {
+        ThreadUtils.runOnUiThreadBlocking(() -> mTab.updateAttachment(null, null));
+        assertFalse(mTab.hasObserver(mTabObserver));
+    }
 }

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/EmptyTabObserver.java

@@ -6,14 +6,11 @@ package org.chromium.chrome.browser.tab;
 
 import android.graphics.Bitmap;
 
-import androidx.annotation.Nullable;
-
 import org.chromium.components.find_in_page.FindMatchRectsDetails;
 import org.chromium.components.find_in_page.FindNotificationDetails;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.NavigationHandle;
 import org.chromium.net.NetError;
-import org.chromium.ui.base.WindowAndroid;
 import org.chromium.url.GURL;
 
 /**
@@ -119,9 +116,6 @@ public class EmptyTabObserver implements TabObserver {
     @Override
     public void onBackgroundColorChanged(Tab tab, int color) {}
 
-    @Override
-    public void onActivityAttachmentChanged(Tab tab, @Nullable WindowAndroid window) {}
-
     @Override
     public void onInteractabilityChanged(Tab tab, boolean isInteractable) {}
 

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/Tab.java

@@ -38,6 +38,9 @@ public interface Tab extends TabLifecycle {
      */
     void removeObserver(TabObserver observer);
 
+    /** Returns if the given {@link TabObserver} is present. */
+    boolean hasObserver(TabObserver observer);
+
     /**
      * @return {@link UserDataHost} that manages {@link UserData} objects attached to.
      *         This is used for managing Tab-specific attributes/objects without Tab

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/TabObserver.java

@@ -263,12 +263,18 @@ public interface TabObserver {
     void onBackgroundColorChanged(Tab tab, int color);
 
     /**
-     * Called when the Tab is attached or detached from an {@code Activity}.
+     * Called when the Tab is attached or detached from an {@code Activity}. By default, this will
+     * automatically unregister the tab observer if the Tab is detached from the window. To
+     * customize this behavior, override this method. When overriding this, keep in mind that tabs
+     * can outlive the activity in some cases (change of theme, changing from phone/tablet, etc).
      * @param tab The notifying {@link Tab}.
      * @param window {@link WindowAndroid} which the Tab is being associated with. {@code null} if
      *         the tab is being detached.
      */
-    void onActivityAttachmentChanged(Tab tab, @Nullable WindowAndroid window);
+    default void onActivityAttachmentChanged(Tab tab, @Nullable WindowAndroid window) {
+        if (tab == null || window != null) return;
+        tab.removeObserver(this);
+    }
 
     /**
      * A notification when tab changes whether or not it is interactable and is accepting input.