Fix crash in TabModelSelectorTabObserver#tabRemoved

Ran into this crash when changing the timing of Activity finishing in
tests.

TabModelSelectorTabObserver#tabRemoved posts a task, and then uses the
Tab after it's possibly been destroyed.

Changes to CriticalPersistedState were also necessary to avoid the
observer leak that would happen in this case.

Change-Id: I152a2d08cee86af51517f5708e57f8dc7fad4f9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535559
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827176}

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

@@ -186,6 +186,8 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
 
     private final UserDataHost mUserDataHost = new UserDataHost();
 
+    private boolean mIsDestroyed;
+
     /**
      * Creates an instance of a {@link TabImpl}.
      *
@@ -574,6 +576,11 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
         return mNativeTabAndroid != 0;
     }
 
+    @Override
+    public boolean isDestroyed() {
+        return mIsDestroyed;
+    }
+
     @Override
     public final void show(@TabSelectionType int type) {
         try {
@@ -651,6 +658,10 @@ public class TabImpl implements Tab, TabObscuringHandler.Observer {
 
     @Override
     public void destroy() {
+        // Set at the start since destroying the WebContents can lead to calling back into
+        // this class.
+        mIsDestroyed = true;
+
         // Update the title before destroying the tab. http://b/5783092
         updateTitle();
 

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

@@ -16,6 +16,11 @@ public interface TabLifecycle {
      */
     boolean isInitialized();
 
+    /**
+     * @return Whether this Tab has been destroyed.
+     */
+    boolean isDestroyed();
+
     /**
      * Prepares the tab to be shown. This method is supposed to be called before the tab is
      * displayed. It restores the ContentView if it is not available after the cold start and

chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/state/CriticalPersistedTabData.java

@@ -410,7 +410,9 @@ public class CriticalPersistedTabData extends PersistedTabData {
     }
 
     @Override
-    public void destroy() {}
+    public void destroy() {
+        mObservers.clear();
+    }
 
     /**
      * @return title of the {@link Tab}

chrome/browser/tabmodel/android/java/src/org/chromium/chrome/browser/tabmodel/TabModelSelectorTabObserver.java

@@ -75,6 +75,8 @@ public class TabModelSelectorTabObserver
                 // Post the removal of the observer so that other tab events are notified
                 // before removing the tab observer (e.g. detach tab from activity).
                 PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> {
+                    // If the tab as been destroyed we cannot access PersistedTabData.
+                    if (tab.isDestroyed()) return;
                     tab.removeObserver(TabModelSelectorTabObserver.this);
                     CriticalPersistedTabData.from(tab).removeObserver(
                             TabModelSelectorTabObserver.this);