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

content/browser/android/java/gin_java_bridge_dispatcher_host.cc

Index: content/browser/android/java/gin_java_bridge_dispatcher_host.cc
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 74ccaa21aaf6b0352edd3d971c6d1630524292ae..6f076ebda2d0738cd28b6ed8e3187f86dbb6e1db 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(
 }
 
 GinJavaBridgeDispatcherHost::~GinJavaBridgeDispatcherHost() {
+  DCHECK(pending_replies_.empty());
 }
 
 void GinJavaBridgeDispatcherHost::RenderFrameCreated(
@@ -70,6 +71,18 @@ void GinJavaBridgeDispatcherHost::RenderFrameCreated(
 
 void GinJavaBridgeDispatcherHost::RenderFrameDeleted(
     RenderFrameHost* render_frame_host) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  PendingReplyMap::const_iterator it =
+      pending_replies_.find(render_frame_host);
+  if (it != pending_replies_.end()) {
+    IPC::Message* reply_msg = it->second;
+    base::ListValue result;
+    result.Append(base::Value::CreateNullValue());
+    IPC::WriteParam(reply_msg, result);
+    IPC::WriteParam(reply_msg, kGinJavaBridgeRenderFrameDeleted);
+    render_frame_host->Send(reply_msg);
+    pending_replies_.erase(render_frame_host);
+  }
   RemoveHolder(render_frame_host,
                GinJavaBoundObject::ObjectMap::iterator(&objects_),
                objects_.size());
@@ -356,11 +369,17 @@ 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;
+
+  if (!IsValidRenderFrameHost(render_frame_host)) {
+    // In this case, we must've already sent the reply when the redner frame
+    // was destroyed.
+    DCHECK(pending_replies_.find(render_frame_host) == pending_replies_.end());

[boliu, 2014/08/27 17:47:34]

delete reply_msg here I think?

[benm (inactive), 2014/08/27 17:53:34]

I don't think so, as in this case we should have sent the reply already in
RenderFrameDeleted.

[boliu, 2014/08/27 17:55:56]

Hmm? I mean |reply_msg| argument to this function, not what's in
|pending_replies_|.

[benm (inactive), 2014/08/27 17:57:50]

They are one and the same I believe. the param here is put into the callback
(e.g. see line 412)

[boliu, 2014/08/27 18:23:48]

Oh god. That's just freaking confusing. That means SendReply, and probably a
bunch of other places no longer needs to take the reply_msg arg.

[benm (inactive), 2014/08/27 18:30:02]

Mmm, yes that's true. We can always get it from the map when we want it.

+    return;
   }
+
+  DCHECK(pending_replies_.find(render_frame_host) != pending_replies_.end());
+  render_frame_host->Send(reply_msg);
+  pending_replies_.erase(render_frame_host);
 }
 
 void GinJavaBridgeDispatcherHost::OnGetMethods(
@@ -381,6 +400,8 @@ void GinJavaBridgeDispatcherHost::OnGetMethods(
     render_frame_host->Send(reply_msg);
     return;
   }
+  DCHECK(pending_replies_.find(render_frame_host) == pending_replies_.end());
+  pending_replies_[render_frame_host] = reply_msg;
   base::PostTaskAndReplyWithResult(
       g_background_thread.Get().message_loop()->message_loop_proxy(),
       FROM_HERE,
@@ -413,6 +434,8 @@ void GinJavaBridgeDispatcherHost::OnHasMethod(
     render_frame_host->Send(reply_msg);
     return;
   }
+  DCHECK(pending_replies_.find(render_frame_host) == pending_replies_.end());
+  pending_replies_[render_frame_host] = reply_msg;
   base::PostTaskAndReplyWithResult(
       g_background_thread.Get().message_loop()->message_loop_proxy(),
       FROM_HERE,
@@ -449,6 +472,8 @@ void GinJavaBridgeDispatcherHost::OnInvokeMethod(
     render_frame_host->Send(reply_msg);
     return;
   }
+  DCHECK(pending_replies_.find(render_frame_host) == pending_replies_.end());
+  pending_replies_[render_frame_host] = reply_msg;
   scoped_refptr<GinJavaMethodInvocationHelper> result =
       new GinJavaMethodInvocationHelper(
           make_scoped_ptr(new GinJavaBoundObjectDelegate(object))
@@ -488,10 +513,14 @@ void GinJavaBridgeDispatcherHost::ProcessMethodInvocationObjectResult(
     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(pending_replies_.find(render_frame_host) == pending_replies_.end());

[boliu, 2014/08/27 17:47:34]

delete reply_msg?

[benm (inactive), 2014/08/27 17:53:34]

same reply as above.

     return;
   }
+
   base::ListValue wrapped_result;
   if (!result->GetObjectResult().is_null()) {
     GinJavaBoundObject::ObjectID returned_object_id;
@@ -504,13 +533,14 @@ 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::WriteParam(reply_msg, wrapped_result);
   IPC::WriteParam(reply_msg, result->GetInvocationError());
-  render_frame_host->Send(reply_msg);
+  SendReply(render_frame_host, reply_msg);
 }
 
 void GinJavaBridgeDispatcherHost::OnObjectWrapperDeleted(