Add support for closing Android Shell instances from Java.

This reverts commit 4e5242f7faa3f7ced3e8a955a8154c43016986d8.

This differs from the first patch in that creating a new shell does not
clean up the previous instances (i.e. by launching a new window via js).
That had timing dependencies that broke the webkit tests and I believe
the correct solution is to allow switching back and forth to various shells
and allowing the user to delete them.  That will be a larger patch that
I will handle separately.

TEST=webkit/tools/layout_tests/run_webkit_tests.py --platform=android --debug
TEST=build/android/test_runner.py instrumentation --test-apk ContentShellTest --test_data chrome:chrome/test/data/android/device_files -f ContentShellShellManagementTest#
BUG=

Review URL: https://codereview.chromium.org/144473002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247099 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 113 insertions, 16 deletions

diff --git a/content/shell/android/java/src/org/chromium/content_shell/Shell.java b/content/shell/android/java/src/org/chromium/content_shell/Shell.java
index 1ec5919..3a442a0 100644
--- a/content/shell/android/java/src/org/chromium/content_shell/Shell.java
+++ b/content/shell/android/java/src/org/chromium/content_shell/Shell.java
@@ -49,6 +49,7 @@ public class Shell extends LinearLayout {
 
     private ClipDrawable mProgressDrawable;
 
+    private long mNativeShell;
     private ContentViewRenderView mContentViewRenderView;
     private WindowAndroid mWindow;
 
@@ -80,13 +81,41 @@ public class Shell extends LinearLayout {
     }
 
     /**
+     * Initializes the Shell for use.
+     *
+     * @param nativeShell The pointer to the native Shell object.
      * @param window The owning window for this shell.
      */
-    public void setWindow(WindowAndroid window) {
+    public void initialize(long nativeShell, WindowAndroid window) {
+        mNativeShell = nativeShell;
         mWindow = window;
     }
 
     /**
+     * Closes the shell and cleans up the native instance, which will handle destroying all
+     * dependencies.
+     */
+    public void close() {
+        if (mNativeShell == 0) return;
+        nativeCloseShell(mNativeShell);
+    }
+
+    @CalledByNative
+    private void onNativeDestroyed() {
+        mWindow = null;
+        mNativeShell = 0;
+        mContentView.destroy();
+    }
+
+    /**
+     * @return Whether the Shell has been destroyed.
+     * @see #onNativeDestroyed()
+     */
+    public boolean isDestroyed() {
+        return mNativeShell == 0;
+    }
+
+    /**
      * @return Whether or not the Shell is loading content.
      */
     public boolean isLoading() {
@@ -242,4 +271,6 @@ public class Shell extends LinearLayout {
             imm.hideSoftInputFromWindow(mUrlTextView.getWindowToken(), 0);
         }
     }
+
+    private static native void nativeCloseShell(long shellPtr);
 }
diff --git a/content/shell/android/java/src/org/chromium/content_shell/ShellManager.java b/content/shell/android/java/src/org/chromium/content_shell/ShellManager.java
index c1f1e3d..9303be8 100644
--- a/content/shell/android/java/src/org/chromium/content_shell/ShellManager.java
+++ b/content/shell/android/java/src/org/chromium/content_shell/ShellManager.java
@@ -11,6 +11,7 @@ import android.widget.FrameLayout;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
+import org.chromium.base.ThreadUtils;
 import org.chromium.content.browser.ContentView;
 import org.chromium.content.browser.ContentViewRenderView;
 import org.chromium.ui.base.WindowAndroid;
@@ -82,7 +83,10 @@ public class ShellManager extends FrameLayout {
      * @param url The URL the shell should load upon creation.
      */
     public void launchShell(String url) {
+        ThreadUtils.assertOnUiThread();
+        Shell previousShell = mActiveShell;
         nativeLaunchShell(url);
+        if (previousShell != null) previousShell.close();
     }
 
     /**
@@ -96,15 +100,21 @@ public class ShellManager extends FrameLayout {
 
     @SuppressWarnings("unused")
     @CalledByNative
-    private Object createShell() {
+    private Object createShell(long nativeShellPtr) {
         assert mContentViewRenderView != null;
         LayoutInflater inflater =
                 (LayoutInflater) getContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE);
         Shell shellView = (Shell) inflater.inflate(R.layout.shell_view, null);
-        shellView.setWindow(mWindow);
+        shellView.initialize(nativeShellPtr, mWindow);
 
-        if (mActiveShell != null) closeShell(mActiveShell);
+        // TODO(tedchoc): Allow switching back to these inactive shells.
+        if (mActiveShell != null) removeShell(mActiveShell);
 
+        showShell(shellView);
+        return shellView;
+    }
+
+    private void showShell(Shell shellView) {
         shellView.setContentViewRenderView(mContentViewRenderView);
         addView(shellView, new FrameLayout.LayoutParams(
                 FrameLayout.LayoutParams.MATCH_PARENT, FrameLayout.LayoutParams.MATCH_PARENT));
@@ -114,18 +124,15 @@ public class ShellManager extends FrameLayout {
             mContentViewRenderView.setCurrentContentView(contentView);
             contentView.onShow();
         }
-
-        return shellView;
     }
 
-    @SuppressWarnings("unused")
     @CalledByNative
-    private void closeShell(Shell shellView) {
+    private void removeShell(Shell shellView) {
         if (shellView == mActiveShell) mActiveShell = null;
+        if (shellView.getParent() == null) return;
         ContentView contentView = shellView.getContentView();
         if (contentView != null) contentView.onHide();
         shellView.setContentViewRenderView(null);
-        shellView.setWindow(null);
         removeView(shellView);
     }
 
diff --git a/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellShellManagementTest.java b/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellShellManagementTest.java
new file mode 100644
index 0000000..f3a3c36
--- /dev/null
+++ b/content/shell/android/javatests/src/org/chromium/content_shell_apk/ContentShellShellManagementTest.java
@@ -0,0 +1,47 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.content_shell_apk;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.Feature;
+import org.chromium.base.test.util.UrlUtils;
+import org.chromium.content_shell.Shell;
+
+/**
+ * Test suite to verify the behavior of the shell management logic.
+ */
+public class ContentShellShellManagementTest extends ContentShellTestBase {
+
+    private static final String TEST_PAGE_1 = UrlUtils.encodeHtmlDataUri(
+            "<html><body style='background: red;'></body></html>");
+    private static final String TEST_PAGE_2 = UrlUtils.encodeHtmlDataUri(
+            "<html><body style='background: green;'></body></html>");
+
+    @SmallTest
+    @Feature({"Main"})
+    public void testMultipleShellsLaunched() throws InterruptedException {
+        final ContentShellActivity activity = launchContentShellWithUrl(TEST_PAGE_1);
+        assertEquals(TEST_PAGE_1, activity.getActiveShell().getContentView().getUrl());
+
+        Shell previousActiveShell = activity.getActiveShell();
+        assertFalse(previousActiveShell.isDestroyed());
+
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                activity.getShellManager().launchShell(TEST_PAGE_2);
+            }
+        });
+        waitForActiveShellToBeDoneLoading();
+        assertEquals(TEST_PAGE_2, activity.getActiveShell().getContentView().getUrl());
+
+        assertNotSame(previousActiveShell, activity.getActiveShell());
+        assertTrue(previousActiveShell.isDestroyed());
+        assertFalse(previousActiveShell.getContentView().isAlive());
+    }
+
+}
diff --git a/content/shell/android/shell_manager.cc b/content/shell/android/shell_manager.cc
index 9dbd46d..4d11578 100644
--- a/content/shell/android/shell_manager.cc
+++ b/content/shell/android/shell_manager.cc
@@ -34,13 +34,16 @@ namespace content {
 jobject CreateShellView(Shell* shell) {
   JNIEnv* env = base::android::AttachCurrentThread();
   jobject j_shell_manager = g_global_state.Get().j_shell_manager.obj();
-  return Java_ShellManager_createShell(env, j_shell_manager).Release();
+  return Java_ShellManager_createShell(
+      env,
+      j_shell_manager,
+      reinterpret_cast<intptr_t>(shell)).Release();
 }
 
-void CloseShellView(jobject shell_view) {
+void RemoveShellView(jobject shell_view) {
   JNIEnv* env = base::android::AttachCurrentThread();
   jobject j_shell_manager = g_global_state.Get().j_shell_manager.obj();
-  Java_ShellManager_closeShell(env, j_shell_manager, shell_view);
+  Java_ShellManager_removeShell(env, j_shell_manager, shell_view);
 }
 
 // Register native methods
diff --git a/content/shell/android/shell_manager.h b/content/shell/android/shell_manager.h
index 6716251..93ccec4 100644
--- a/content/shell/android/shell_manager.h
+++ b/content/shell/android/shell_manager.h
@@ -24,8 +24,8 @@ namespace content {
 // object.
 jobject CreateShellView(Shell* shell);
 
-// Closes a previously created shell view.
-void CloseShellView(jobject shell_view);
+// Removes a previously created shell view.
+void RemoveShellView(jobject shell_view);
 
 // Registers the ShellManager native methods.
 bool RegisterShellManager(JNIEnv* env);
diff --git a/content/shell/browser/shell_android.cc b/content/shell/browser/shell_android.cc
index b11ba37..80a653e 100644
--- a/content/shell/browser/shell_android.cc
+++ b/content/shell/browser/shell_android.cc
@@ -30,6 +30,10 @@ void Shell::PlatformExit() {
 }
 
 void Shell::PlatformCleanUp() {
+  JNIEnv* env = AttachCurrentThread();
+  if (java_object_.is_null())
+    return;
+  Java_Shell_onNativeDestroyed(env, java_object_.obj());
 }
 
 void Shell::PlatformEnableUIControl(UIControl control, bool is_enabled) {
@@ -83,8 +87,7 @@ bool Shell::PlatformIsFullscreenForTabOrPending(
 }
 
 void Shell::Close() {
-  CloseShellView(java_object_.obj());
-  java_object_.Reset();
+  RemoveShellView(java_object_.obj());
   delete this;
 }
 
@@ -93,4 +96,10 @@ bool Shell::Register(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
+// static
+void CloseShell(JNIEnv* env, jclass clazz, jlong shellPtr) {
+  Shell* shell = reinterpret_cast<Shell*>(shellPtr);
+  shell->Close();
+}
+
 }  // namespace content