Skip to content

GitLab

T
tangled
Commit 48334b78 authored by benm's avatar benm Committed by Commit bot
Browse files
Options

Cache pending JS bridge sync IPC replies, and send in case of RenderFrame deletion. 

When the WebView app makes a call to java over the JavaScriptBridge,
we leave the renderer hanging on a synchronous IPC. Once control
is passed into Java, it's possible that the WebView may get destroyed
(and thus the IPC channel back to the renderer closed) which means we
can't unblock the renderer waiting on the IPC response.

Instead we cache the IPC reply message and while waiting on Java to come
back to us, if we detect that the RenderFrame has been deleted, send a
reponse back before the IPC channel is closed.

BUG=408188

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

Cr-Commit-Position: refs/heads/master@{#292631}
parent 530acace
Hide whitespace changes
Inline Side-by-side
Showing with 155 additions and 31 deletions
android_webview/javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java 0 → 100644
View file @ 48334b78
// 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.android_webview.test;
import android.test.suitebuilder.annotation.SmallTest;
import org.chromium.android_webview.AwContents;
import org.chromium.base.test.util.Feature;
import org.chromium.content.browser.JavascriptInterface;
/**
* Test suite for the WebView specific JavaBridge features.
*/
public class AwJavaBridgeTest extends AwTestBase {
private TestAwContentsClient mContentsClient = new TestAwContentsClient();
private AwTestContainerView mTestContainerView;
@Override
protected void setUp() throws Exception {
super.setUp();
mTestContainerView = createAwTestContainerViewOnMainSync(mContentsClient);
}
@SmallTest
@Feature({"AndroidWebView", "Android-JavaBridge"})
public void testDestroyFromJavaObject() throws Throwable {
final String HTML = "<html>Hello World</html>";
final TestAwContentsClient client2 = new TestAwContentsClient();
final AwTestContainerView view2 = createAwTestContainerViewOnMainSync(client2);
final AwContents awContents = mTestContainerView.getAwContents();
class Test {
@JavascriptInterface
public void destroy() {
try {
runTestOnUiThread(new Runnable() {
@Override
public void run() {
awContents.destroy();
}
});
// Destroying one AwContents from within the JS callback should still
// leave others functioning.
loadDataSync(view2.getAwContents(), client2.getOnPageFinishedHelper(),
HTML, "text/html", false);
} catch (Throwable t) {
throw new RuntimeException(t);
}
}
}
enableJavaScriptOnUiThread(awContents);
runTestOnUiThread(new Runnable() {
@Override
public void run() {
awContents.addPossiblyUnsafeJavascriptInterface(new Test(), "test", null);
}
});
loadDataSync(awContents, mContentsClient.getOnPageFinishedHelper(), HTML,
"text/html", false);
// Ensure the JS interface object is there, and invoke the test method.
assertEquals("\"function\"", executeJavaScriptAndWaitForResult(
awContents, mContentsClient, "typeof test.destroy"));
awContents.evaluateJavaScript("test.destroy()", null);
client2.getOnPageFinishedHelper().waitForCallback(
client2.getOnPageFinishedHelper().getCallCount());
}
}
content/browser/android/java/gin_java_bridge_dispatcher_host.cc
View file @ 48334b78
......@@ -56,6 +56,7 @@ GinJavaBridgeDispatcherHost::GinJavaBridgeDispatcherHost(
}
GinJavaBridgeDispatcherHost::~GinJavaBridgeDispatcherHost() {
DCHECK(pending_replies_.empty());
}
void GinJavaBridgeDispatcherHost::RenderFrameCreated(
......@@ -70,6 +71,15 @@ void GinJavaBridgeDispatcherHost::RenderFrameCreated(
void GinJavaBridgeDispatcherHost::RenderFrameDeleted(
RenderFrameHost* render_frame_host) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
IPC::Message* reply_msg = TakePendingReply(render_frame_host);
if (reply_msg != NULL) {
base::ListValue result;
result.Append(base::Value::CreateNullValue());
IPC::WriteParam(reply_msg, result);
IPC::WriteParam(reply_msg, kGinJavaBridgeRenderFrameDeleted);
render_frame_host->Send(reply_msg);
}
RemoveHolder(render_frame_host,
GinJavaBoundObject::ObjectMap::iterator(&objects_),
objects_.size());
......@@ -352,17 +362,6 @@ bool GinJavaBridgeDispatcherHost::IsValidRenderFrameHost(
return helper->rfh_found();
}
void GinJavaBridgeDispatcherHost::SendReply(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (IsValidRenderFrameHost(render_frame_host)) {
render_frame_host->Send(reply_msg);
} else {
delete reply_msg;
}
}
void GinJavaBridgeDispatcherHost::OnGetMethods(
RenderFrameHost* render_frame_host,
GinJavaBoundObject::ObjectID object_id,
......@@ -381,22 +380,26 @@ void GinJavaBridgeDispatcherHost::OnGetMethods(
render_frame_host->Send(reply_msg);
return;
}
DCHECK(!HasPendingReply(render_frame_host));
pending_replies_[render_frame_host] = reply_msg;
base::PostTaskAndReplyWithResult(
g_background_thread.Get().message_loop()->message_loop_proxy(),
FROM_HERE,
base::Bind(&GinJavaBoundObject::GetMethodNames, object),
base::Bind(&GinJavaBridgeDispatcherHost::SendMethods,
AsWeakPtr(),
render_frame_host,
reply_msg));
render_frame_host));
}
void GinJavaBridgeDispatcherHost::SendMethods(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
const std::set<std::string>& method_names) {
IPC::Message* reply_msg = TakePendingReply(render_frame_host);
if (!reply_msg) {
return;
}
IPC::WriteParam(reply_msg, method_names);
SendReply(render_frame_host, reply_msg);
render_frame_host->Send(reply_msg);
}
void GinJavaBridgeDispatcherHost::OnHasMethod(
......@@ -413,22 +416,26 @@ void GinJavaBridgeDispatcherHost::OnHasMethod(
render_frame_host->Send(reply_msg);
return;
}
DCHECK(!HasPendingReply(render_frame_host));
pending_replies_[render_frame_host] = reply_msg;
base::PostTaskAndReplyWithResult(
g_background_thread.Get().message_loop()->message_loop_proxy(),
FROM_HERE,
base::Bind(&GinJavaBoundObject::HasMethod, object, method_name),
base::Bind(&GinJavaBridgeDispatcherHost::SendHasMethodReply,
AsWeakPtr(),
render_frame_host,
reply_msg));
render_frame_host));
}
void GinJavaBridgeDispatcherHost::SendHasMethodReply(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
bool result) {
IPC::Message* reply_msg = TakePendingReply(render_frame_host);
if (!reply_msg) {
return;
}
IPC::WriteParam(reply_msg, result);
SendReply(render_frame_host, reply_msg);
render_frame_host->Send(reply_msg);
}
void GinJavaBridgeDispatcherHost::OnInvokeMethod(
......@@ -449,6 +456,8 @@ void GinJavaBridgeDispatcherHost::OnInvokeMethod(
render_frame_host->Send(reply_msg);
return;
}
DCHECK(!HasPendingReply(render_frame_host));
pending_replies_[render_frame_host] = reply_msg;
scoped_refptr<GinJavaMethodInvocationHelper> result =
new GinJavaMethodInvocationHelper(
make_scoped_ptr(new GinJavaBoundObjectDelegate(object))
......@@ -466,32 +475,37 @@ void GinJavaBridgeDispatcherHost::OnInvokeMethod(
&GinJavaBridgeDispatcherHost::ProcessMethodInvocationResult,
AsWeakPtr(),
render_frame_host,
reply_msg,
result));
}
void GinJavaBridgeDispatcherHost::ProcessMethodInvocationResult(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
scoped_refptr<GinJavaMethodInvocationHelper> result) {
if (result->HoldsPrimitiveResult()) {
IPC::Message* reply_msg = TakePendingReply(render_frame_host);
if (!reply_msg) {
return;
}
IPC::WriteParam(reply_msg, result->GetPrimitiveResult());
IPC::WriteParam(reply_msg, result->GetInvocationError());
SendReply(render_frame_host, reply_msg);
render_frame_host->Send(reply_msg);
} else {
ProcessMethodInvocationObjectResult(render_frame_host, reply_msg, result);
ProcessMethodInvocationObjectResult(render_frame_host, result);
}
}
void GinJavaBridgeDispatcherHost::ProcessMethodInvocationObjectResult(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
scoped_refptr<GinJavaMethodInvocationHelper> result) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
if (!IsValidRenderFrameHost(render_frame_host)) {
delete reply_msg;
// In this case, we must've already sent the reply when the render frame
// was destroyed.
DCHECK(!HasPendingReply(render_frame_host));
return;
}
base::ListValue wrapped_result;
if (!result->GetObjectResult().is_null()) {
GinJavaBoundObject::ObjectID returned_object_id;
......@@ -504,10 +518,15 @@ void GinJavaBridgeDispatcherHost::ProcessMethodInvocationObjectResult(
render_frame_host);
}
wrapped_result.Append(
GinJavaBridgeValue::CreateObjectIDValue(returned_object_id).release());
GinJavaBridgeValue::CreateObjectIDValue(
returned_object_id).release());
} else {
wrapped_result.Append(base::Value::CreateNullValue());
}
IPC::Message* reply_msg = TakePendingReply(render_frame_host);
if (!reply_msg) {
return;
}
IPC::WriteParam(reply_msg, wrapped_result);
IPC::WriteParam(reply_msg, result->GetInvocationError());
render_frame_host->Send(reply_msg);
......@@ -527,4 +546,28 @@ void GinJavaBridgeDispatcherHost::OnObjectWrapperDeleted(
}
}
IPC::Message* GinJavaBridgeDispatcherHost::TakePendingReply(
RenderFrameHost* render_frame_host) {
if (!IsValidRenderFrameHost(render_frame_host)) {
DCHECK(!HasPendingReply(render_frame_host));
return NULL;
}
PendingReplyMap::iterator it = pending_replies_.find(render_frame_host);
// There may be no pending reply if we're called from RenderFrameDeleted and
// we already sent the reply through the regular route.
if (it == pending_replies_.end()) {
return NULL;
}
IPC::Message* reply_msg = it->second;
pending_replies_.erase(it);
return reply_msg;
}
bool GinJavaBridgeDispatcherHost::HasPendingReply(
RenderFrameHost* render_frame_host) const {
return pending_replies_.find(render_frame_host) != pending_replies_.end();
}
} // namespace content
content/browser/android/java/gin_java_bridge_dispatcher_host.h
View file @ 48334b78
......@@ -76,20 +76,15 @@ class GinJavaBridgeDispatcherHost
GinJavaBoundObject::ObjectID object_id);
bool IsValidRenderFrameHost(RenderFrameHost* render_frame_host);
void SendReply(RenderFrameHost* render_frame_host, IPC::Message* reply_msg);
void SendMethods(RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
const std::set<std::string>& method_names);
void SendHasMethodReply(RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
bool result);
void ProcessMethodInvocationResult(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
scoped_refptr<GinJavaMethodInvocationHelper> result);
void ProcessMethodInvocationObjectResult(
RenderFrameHost* render_frame_host,
IPC::Message* reply_msg,
scoped_refptr<GinJavaMethodInvocationHelper> result);
GinJavaBoundObject::ObjectID AddObject(
const base::android::JavaRef<jobject>& object,
......@@ -101,6 +96,8 @@ class GinJavaBridgeDispatcherHost
void RemoveHolder(RenderFrameHost* holder,
const GinJavaBoundObject::ObjectMap::iterator& from,
size_t count);
bool HasPendingReply(RenderFrameHost* render_frame_host) const;
IPC::Message* TakePendingReply(RenderFrameHost* render_frame_host);
// Every time a GinJavaBoundObject backed by a real Java object is
// created/destroyed, we insert/remove a strong ref to that Java object into
......@@ -114,6 +111,13 @@ class GinJavaBridgeDispatcherHost
typedef std::map<std::string, GinJavaBoundObject::ObjectID> NamedObjectMap;
NamedObjectMap named_objects_;
// Keep track of pending calls out to Java such that we can send a synchronous
// reply to the renderer waiting on the response should the RenderFrame be
// destroyed while the reply is pending.
// Only used on the UI thread.
typedef std::map<RenderFrameHost*, IPC::Message*> PendingReplyMap;
PendingReplyMap pending_replies_;
DISALLOW_COPY_AND_ASSIGN(GinJavaBridgeDispatcherHost);
};
......
content/common/android/gin_java_bridge_errors.cc
View file @ 48334b78
......@@ -25,6 +25,8 @@ const char* GinJavaBridgeErrorToString(GinJavaBridgeError error) {
case kGinJavaBridgeNonAssignableTypes:
return "The type of the object passed to the method is incompatible "
"with the type of method's argument";
case kGinJavaBridgeRenderFrameDeleted:
return "RenderFrame has been deleted";
}
NOTREACHED();
return "Unknown error";
......
content/common/android/gin_java_bridge_errors.h
View file @ 48334b78
......@@ -17,6 +17,7 @@ enum GinJavaBridgeError {
kGinJavaBridgeAccessToObjectGetClassIsBlocked,
kGinJavaBridgeJavaExceptionRaised,
kGinJavaBridgeNonAssignableTypes,
kGinJavaBridgeRenderFrameDeleted,
};
CONTENT_EXPORT const char* GinJavaBridgeErrorToString(GinJavaBridgeError error);
......
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