Commit cb102492 authored by gunsch's avatar gunsch Committed by Commit bot

Chromecast: call ClosePage(), wait 50ms before deleting WebContents.

Without this, Chromecast apps will not have time to handle
window.unload events when user switches to a new app.

R=lcwu@chromium.org,byungchul@chromium.org
BUG=400876

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

Cr-Commit-Position: refs/heads/master@{#296811}
parent c9c8408f
......@@ -120,7 +120,7 @@ public class CastShellActivity extends Activity {
unregisterBroadcastReceiver();
if (mNativeCastWindow != 0) {
mCastWindowManager.stopCastWindow(mNativeCastWindow);
mCastWindowManager.stopCastWindow(mNativeCastWindow, false /* gracefully */);
mNativeCastWindow = 0;
}
}
......@@ -176,7 +176,7 @@ public class CastShellActivity extends Activity {
protected void finishGracefully() {
if (mNativeCastWindow != 0) {
mCastWindowManager.stopCastWindow(mNativeCastWindow);
mCastWindowManager.stopCastWindow(mNativeCastWindow, true /* gracefully */);
mNativeCastWindow = 0;
}
}
......
......@@ -98,10 +98,11 @@ public class CastWindowManager extends FrameLayout {
* Stops a native cast shell instance created by {@link #launchCastWindow(String)}.
* @param nativeCastWindow Pointer of native cast shell instance returned
* by {@link #launchCastWindow(String)}.
* @param gracefully Whether or not to call RVH::ClosePage to deliver unload event.
* @see #launchCastWindow(String)
*/
public void stopCastWindow(long nativeCastWindow) {
nativeStopCastWindow(nativeCastWindow);
public void stopCastWindow(long nativeCastWindow, boolean gracefully) {
nativeStopCastWindow(nativeCastWindow, gracefully);
}
@SuppressWarnings("unused")
......@@ -150,6 +151,7 @@ public class CastWindowManager extends FrameLayout {
private static native void nativeInit(Object shellManagerInstance);
private static native long nativeLaunchCastWindow(String url);
private static native void nativeStopCastWindow(long pointerOfNativeCastWindow);
private static native void nativeStopCastWindow(long pointerOfNativeCastWindow,
boolean gracefully);
public static native void nativeEnableDevTools(boolean enable);
}
......@@ -4,7 +4,7 @@
#include "chromecast/shell/browser/android/cast_window_android.h"
#include "base/message_loop/message_loop.h"
#include "base/message_loop/message_loop_proxy.h"
#include "base/path_service.h"
#include "chromecast/shell/browser/android/cast_window_manager.h"
#include "content/public/browser/devtools_agent_host.h"
......@@ -17,13 +17,22 @@
namespace chromecast {
namespace shell {
namespace {
// The time (in milliseconds) we wait for after a page is closed (i.e.
// after an app is stopped) before we delete the corresponding WebContents.
const int kWebContentsDestructionDelayInMs = 50;
} // namespace
// static
bool CastWindowAndroid::RegisterJni(JNIEnv* env) {
return RegisterNativesImpl(env);
}
CastWindowAndroid::CastWindowAndroid(content::WebContents* web_contents)
: content::WebContentsObserver(web_contents) {
: content::WebContentsObserver(web_contents),
weak_factory_(this) {
}
CastWindowAndroid::~CastWindowAndroid() {
......@@ -71,6 +80,13 @@ CastWindowAndroid* CastWindowAndroid::CreateCastWindowAndroid(
}
void CastWindowAndroid::Close() {
// Close page first, which fires the window.unload event. The WebContents
// itself will be destroyed after browser-process has received renderer
// notification that the page is closed.
web_contents_->GetRenderViewHost()->ClosePage();
}
void CastWindowAndroid::Destroy() {
// Note: if multiple windows becomes supported, this may close other devtools
// sessions.
content::DevToolsAgentHost::DetachAllClients();
......@@ -101,7 +117,29 @@ void CastWindowAndroid::AddNewContents(content::WebContents* source,
void CastWindowAndroid::CloseContents(content::WebContents* source) {
DCHECK_EQ(source, web_contents_.get());
Close();
// We need to delay the deletion of web_contents_ (currently for 50ms) to
// give (and guarantee) the renderer enough time to finish 'onunload'
// handler (but we don't want to wait any longer than that to delay the
// starting of next app).
if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kTestType)) {
// When shutting down in a test context, the last remaining WebContents
// is torn down at browser-thread shutdown time. Call Destroy directly to
// avoid losing the last posted task to delete this object.
// TODO(gunsch): This could probably be avoided by using a
// CompletionCallback in StopCurrentApp to wait until the app is completely
// stopped. This might require a separate message loop and might only be
// appropriate for test contexts or during shutdown, since it triggers a
// wait on the main thread.
Destroy();
return;
}
base::MessageLoopProxy::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&CastWindowAndroid::Destroy, weak_factory_.GetWeakPtr()),
base::TimeDelta::FromMilliseconds(kWebContentsDestructionDelayInMs));
}
bool CastWindowAndroid::CanOverscrollContent() const {
......@@ -128,7 +166,7 @@ void CastWindowAndroid::DeactivateContents(content::WebContents* contents) {
void CastWindowAndroid::RenderProcessGone(base::TerminationStatus status) {
LOG(ERROR) << "Render process gone: status=" << status;
Close();
Destroy();
}
} // namespace shell
......
......@@ -15,6 +15,7 @@
#include "base/command_line.h"
#include "base/macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "build/build_config.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
......@@ -45,7 +46,11 @@ class CastWindowAndroid : public content::WebContentsDelegate,
virtual ~CastWindowAndroid();
void LoadURL(const GURL& url);
// Calls RVH::ClosePage() and waits for acknowledgement before closing/
// deleting the window.
void Close();
// Destroys this window immediately.
void Destroy();
// Registers the JNI methods for CastWindowAndroid.
static bool RegisterJni(JNIEnv* env);
......@@ -81,6 +86,8 @@ class CastWindowAndroid : public content::WebContentsDelegate,
base::android::ScopedJavaGlobalRef<jobject> java_object_;
scoped_ptr<content::WebContents> web_contents_;
base::WeakPtrFactory<CastWindowAndroid> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(CastWindowAndroid);
};
......
......@@ -65,11 +65,15 @@ jlong LaunchCastWindow(JNIEnv* env, jclass clazz, jstring jurl) {
url));
}
void StopCastWindow(JNIEnv* env, jclass clazz, jlong nativeCastWindow) {
void StopCastWindow(JNIEnv* env, jclass clazz,
jlong nativeCastWindow, jboolean gracefully) {
CastWindowAndroid* window =
reinterpret_cast<CastWindowAndroid*>(nativeCastWindow);
DCHECK(window);
window->Close();
if (gracefully)
window->Close();
else
window->Destroy();
}
void EnableDevTools(JNIEnv* env, jclass clazz, jboolean enable) {
......
......@@ -5,6 +5,7 @@
#include "chromecast/shell/browser/cast_browser_context.h"
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/macros.h"
#include "base/path_service.h"
#include "chromecast/common/cast_paths.h"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment