Android: Delete ContentViewCore.isAlive

This CL deletes the API |ContentViewCore.isAlive| and lets the callsites
use |WebContents.isDestroyed| (after negation) instead.

!CVC.isAlive() and WebContents.isDestroyed() basically serve the same
purpose that indicate WebContents (and its associated objecs) are in
the destroyed state. Once they return true, no calls should be made
on WebContents.

WebContents now defines |mIsAlive| for the deleted api to be replaced
with !WebContents.isDestroyed(). It is set to false by CVC.destroy()
or native WebContentsAndroid dtor, whichever comes first, to indicate
that WebContents is on its way to destruction. (WebContents.destroy()
also leads to setting it to false by way of WebcontentsAndroid dtor).

Bug: 598880
Change-Id: I8aa864e4efdd5c326e62c284bbda5987dc0e60a5
Reviewed-on: https://chromium-review.googlesource.com/1058728
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561733}

chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java

@@ -51,7 +51,6 @@
 import org.chromium.chrome.browser.util.ColorUtils;
 import org.chromium.chrome.browser.widget.ClipDrawableProgressBar.DrawingInfo;
 import org.chromium.chrome.browser.widget.ControlContainer;
-import org.chromium.content_public.browser.ContentViewCore;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.EventForwarder;
@@ -418,11 +417,6 @@ private View getContentView() {
         return tab != null ? tab.getContentView() : null;
     }
 
-    private ContentViewCore getActiveContent() {
-        Tab tab = getCurrentTab();
-        return tab != null ? tab.getContentViewCore() : null;
-    }
-
     private WebContents getWebContents() {
         Tab tab = getCurrentTab();
         return tab != null ? tab.getWebContents() : null;
@@ -842,16 +836,16 @@ public void onNewTabCreated(Tab tab) {
 
     private void updateContentOverlayVisibility(boolean show) {
         if (mView == null) return;
-        ContentViewCore content = getActiveContent();
+        WebContents webContents = getWebContents();
         if (show) {
             if (mView.getParent() != this) {
                 // During tab creation, we temporarily add the new tab's view to a FrameLayout to
                 // measure and lay it out. This way we could show the animation in the stack view.
                 // Therefore we should remove the view from that temporary FrameLayout here.
                 UiUtils.removeViewFromParent(mView);
 
-                if (content != null) {
-                    assert content.isAlive();
+                if (webContents != null) {
+                    assert !webContents.isDestroyed();
                     getContentView().setVisibility(View.VISIBLE);
                     if (mFullscreenManager != null) mFullscreenManager.updateViewportSize();
                 }
@@ -870,8 +864,8 @@ private void updateContentOverlayVisibility(boolean show) {
                 setFocusable(true);
                 setFocusableInTouchMode(true);
 
-                if (content != null) {
-                    if (content.isAlive()) getContentView().setVisibility(View.INVISIBLE);
+                if (webContents != null && !webContents.isDestroyed()) {
+                    getContentView().setVisibility(View.INVISIBLE);
                 }
                 removeView(mView);
             }

components/content_view/java/src/org/chromium/components/content_view/ContentView.java

@@ -86,10 +86,8 @@ public static ContentView createContentView(Context context, WebContents webCont
     }
 
     protected WebContentsAccessibility getWebContentsAccessibility() {
-        // TODO(jinsukkim): Make it possible to check WebContents directly rather than doing it
-        //     via examining the availiability of ContentViewCore;
-        return getContentViewCore() != null ? WebContentsAccessibility.fromWebContents(mWebContents)
-                                            : null;
+        return !mWebContents.isDestroyed() ? WebContentsAccessibility.fromWebContents(mWebContents)
+                                           : null;
     }
 
     @Override
@@ -210,8 +208,8 @@ public boolean onGenericMotionEvent(MotionEvent event) {
     }
 
     private ContentViewCore getContentViewCore() {
-        ContentViewCore cvc = ContentViewCore.fromWebContents(mWebContents);
-        return cvc != null && cvc.isAlive() ? cvc : null;
+        if (mWebContents.isDestroyed()) return null;
+        return ContentViewCore.fromWebContents(mWebContents);
     }
 
     private EventForwarder getEventForwarder() {

content/public/android/java/src/org/chromium/content/browser/ContentViewCoreImpl.java

@@ -156,21 +156,16 @@ public void destroy() {
         if (mNativeContentViewCore != 0) {
             nativeOnJavaContentViewCoreDestroyed(mNativeContentViewCore);
         }
-        getImeAdapter().reset();
+        // This is called to fix crash. See https://crbug.com/803244
+        // TODO(jinsukkim): Use an observer to let the manager handle it on its own.
         getGestureListenerManager().reset();
-        hidePopupsAndPreserveSelection();
-        WindowEventObserverManager.from(mWebContents).removeDisplayAndroidObserver();
+        mWebContents.destroyContentsInternal();
         mWebContents = null;
         mNativeContentViewCore = 0;
 
         // See warning in javadoc before adding more clean up code here.
     }
 
-    @Override
-    public boolean isAlive() {
-        return mNativeContentViewCore != 0;
-    }
-
     private void hidePopupsAndClearSelection() {
         getSelectionPopupController().clearSelection();
         PopupController.hideAll(mWebContents);

content/public/android/java/src/org/chromium/content/browser/WindowEventObserverManager.java

@@ -105,8 +105,7 @@ private void addDisplayAndroidObserverIfNeeded() {
         onDIPScaleChanged(display.getDipScale());
     }
 
-    // package-private for ContentViewCoreImpl.destroy()
-    void removeDisplayAndroidObserver() {
+    private void removeDisplayAndroidObserver() {
         if (mWindowAndroid != null) mWindowAndroid.getDisplay().removeObserver(this);
     }
 

content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java

@@ -124,6 +124,10 @@ public WebContents[] newArray(int size) {
     // the same life time as native MediaSession.
     private MediaSessionImpl mMediaSession;
 
+    // True while WebContents is functional and alive. Set to false when the native WebContents is
+    // gone, or destroy() is called explicitly.
+    private boolean mIsAlive;
+
     private class SmartClipCallback {
         public SmartClipCallback(final Handler smartClipHandler) {
             mHandler = smartClipHandler;
@@ -194,6 +198,7 @@ private WebContentsImpl(
         mInternalsHolder = new DefaultInternalsHolder();
         mInternalsHolder.set(internals);
         WindowEventObserverManager.from(this).addObserver(this);
+        mIsAlive = true;
     }
 
     @CalledByNative
@@ -210,6 +215,7 @@ private void clearNativePtr() {
             mObserverProxy.destroy();
             mObserverProxy = null;
         }
+        mIsAlive = false;
     }
 
     @Override
@@ -291,9 +297,13 @@ public void destroy() {
         if (mNativeWebContentsAndroid != 0) nativeDestroyWebContents(mNativeWebContentsAndroid);
     }
 
+    public void destroyContentsInternal() {
+        mIsAlive = false;
+    }
+
     @Override
     public boolean isDestroyed() {
-        return mNativeWebContentsAndroid == 0;
+        return !mIsAlive || mNativeWebContentsAndroid == 0;
     }
 
     @Override

content/public/android/java/src/org/chromium/content_public/browser/ContentViewCore.java

@@ -94,12 +94,6 @@ public interface InternalAccessDelegate {
      */
     void destroy();
 
-    /**
-     * Returns true initially, false after destroy() has been called.
-     * It is illegal to call any other method after destroy().
-     */
-    boolean isAlive();
-
     /**
      * @see View#onAttachedToWindow()
      */

content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewCore.java

@@ -23,11 +23,6 @@ public void setContainerViewInternals(InternalAccessDelegate internalDispatcher)
     @Override
     public void destroy() {}
 
-    @Override
-    public boolean isAlive() {
-        return false;
-    }
-
     @Override
     public void onAttachedToWindow() {}
 

content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellShellManagementTest.java

@@ -54,8 +54,8 @@ public void testMultipleShellsLaunched() throws InterruptedException, ExecutionE
                         .getVisibleUrl());
 
         Assert.assertNotSame(previousActiveShell, activity.getActiveShell());
+        Assert.assertNull(previousActiveShell.getWebContents());
         Assert.assertTrue(previousActiveShell.isDestroyed());
-        Assert.assertFalse(previousActiveShell.getContentViewCore().isAlive());
     }
 
 }