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

conflicts in:
  content/common/android/gin_java_bridge_errors.cc
  content/common/android/gin_java_bridge_errors.h

Bug: 16840290

Original description:
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
Committed:
https://chromium.googlesource.com/chromium/src/+/5d3e001f79c137b2ba0f5b0e9489abea616f3431

Change-Id: I7f1b28e297059eb69c5686316b47d926ea8a3d4f
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java
new file mode 100644
index 0000000..6a15244
--- /dev/null
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java
@@ -0,0 +1,74 @@
+// 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());
+    }
+}
diff --git a/content/browser/android/java/gin_java_bridge_dispatcher_host.cc b/content/browser/android/java/gin_java_bridge_dispatcher_host.cc
index 6526381..c961dd9 100644
--- a/content/browser/android/java/gin_java_bridge_dispatcher_host.cc
+++ b/content/browser/android/java/gin_java_bridge_dispatcher_host.cc
@@ -56,6 +56,7 @@
 }
 
 GinJavaBridgeDispatcherHost::~GinJavaBridgeDispatcherHost() {
+  DCHECK(pending_replies_.empty());
 }
 
 void GinJavaBridgeDispatcherHost::RenderFrameCreated(
@@ -70,6 +71,15 @@
 
 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());
@@ -348,17 +358,6 @@
   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,
@@ -377,22 +376,26 @@
     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(
@@ -409,22 +412,26 @@
     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(
@@ -445,6 +452,8 @@
     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))
@@ -462,32 +471,37 @@
               &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;
@@ -500,10 +514,15 @@
                                      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);
@@ -523,4 +542,28 @@
   }
 }
 
+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
diff --git a/content/browser/android/java/gin_java_bridge_dispatcher_host.h b/content/browser/android/java/gin_java_bridge_dispatcher_host.h
index 615c2b0..48fcbb5 100644
--- a/content/browser/android/java/gin_java_bridge_dispatcher_host.h
+++ b/content/browser/android/java/gin_java_bridge_dispatcher_host.h
@@ -76,20 +76,15 @@
                               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 @@
   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 @@
   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);
 };
 
diff --git a/content/common/android/gin_java_bridge_errors.cc b/content/common/android/gin_java_bridge_errors.cc
index 7a80801..b25d25c 100644
--- a/content/common/android/gin_java_bridge_errors.cc
+++ b/content/common/android/gin_java_bridge_errors.cc
@@ -22,6 +22,8 @@
       return "Access to java.lang.Object.getClass is blocked";
     case kGinJavaBridgeJavaExceptionRaised:
       return "Java exception was raised during method invocation";
+    case kGinJavaBridgeRenderFrameDeleted:
+      return "RenderFrame has been deleted";
   }
   NOTREACHED();
   return "Unknown error";
diff --git a/content/common/android/gin_java_bridge_errors.h b/content/common/android/gin_java_bridge_errors.h
index 75a8970..c73973a 100644
--- a/content/common/android/gin_java_bridge_errors.h
+++ b/content/common/android/gin_java_bridge_errors.h
@@ -16,6 +16,7 @@
   kGinJavaBridgeMethodNotFound,
   kGinJavaBridgeAccessToObjectGetClassIsBlocked,
   kGinJavaBridgeJavaExceptionRaised,
+  kGinJavaBridgeRenderFrameDeleted,
 };
 
 CONTENT_EXPORT const char* GinJavaBridgeErrorToString(GinJavaBridgeError error);