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

content/browser/android/java/gin_java_bridge_dispatcher_host.cc

 // 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.
 
 #include "content/browser/android/java/gin_java_bridge_dispatcher_host.h"
 
 #include "base/android/java_handler_thread.h"
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/lazy_instance.h"
@@ skipping 38 matching lines @@
     WebContents* web_contents,
     jobject retained_object_set)
     : WebContentsObserver(web_contents),
       retained_object_set_(base::android::AttachCurrentThread(),
                            retained_object_set),
       allow_object_contents_inspection_(true) {
   DCHECK(retained_object_set);
 }
 
 GinJavaBridgeDispatcherHost::~GinJavaBridgeDispatcherHost() {
+  DCHECK(pending_replies_.empty());
 }
 
 void GinJavaBridgeDispatcherHost::RenderFrameCreated(
     RenderFrameHost* render_frame_host) {
   for (NamedObjectMap::const_iterator iter = named_objects_.begin();
        iter != named_objects_.end();
        ++iter) {
     render_frame_host->Send(new GinJavaBridgeMsg_AddNamedObject(
         render_frame_host->GetRoutingID(), iter->first, iter->second));
   }
 }
 
 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());
 }
 
 GinJavaBoundObject::ObjectID GinJavaBridgeDispatcherHost::AddObject(
     const base::android::JavaRef<jobject>& object,
     const base::android::JavaRef<jclass>& safe_annotation_clazz,
     bool is_named,
     RenderFrameHost* holder) {
@@ skipping 266 matching lines @@
       new IsValidRenderFrameHostHelper(render_frame_host);
   web_contents()->ForEachFrame(
       base::Bind(&IsValidRenderFrameHostHelper::OnFrame, helper));
   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;
+
+  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(
     RenderFrameHost* render_frame_host,
     GinJavaBoundObject::ObjectID object_id,
     IPC::Message* reply_msg) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(render_frame_host);
   if (!allow_object_contents_inspection_) {
     IPC::WriteParam(reply_msg, std::set<std::string>());
     render_frame_host->Send(reply_msg);
     return;
   }
   scoped_refptr<GinJavaBoundObject> object(*objects_.Lookup(object_id));
   if (!object) {
     LOG(ERROR) << "WebView: Unknown object: " << object_id;
     IPC::WriteParam(reply_msg, std::set<std::string>());
     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,
       base::Bind(&GinJavaBoundObject::GetMethodNames, object),
       base::Bind(&GinJavaBridgeDispatcherHost::SendMethods,
                  AsWeakPtr(),
                  render_frame_host,
                  reply_msg));
 }
 
@@ skipping 12 matching lines @@
     IPC::Message* reply_msg) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(render_frame_host);
   scoped_refptr<GinJavaBoundObject> object(*objects_.Lookup(object_id));
   if (!object) {
     LOG(ERROR) << "WebView: Unknown object: " << object_id;
     IPC::WriteParam(reply_msg, false);
     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,
       base::Bind(&GinJavaBoundObject::HasMethod, object, method_name),
       base::Bind(&GinJavaBridgeDispatcherHost::SendHasMethodReply,
                  AsWeakPtr(),
                  render_frame_host,
                  reply_msg));
 }
 
@@ skipping 16 matching lines @@
   scoped_refptr<GinJavaBoundObject> object(*objects_.Lookup(object_id));
   if (!object) {
     LOG(ERROR) << "WebView: Unknown object: " << object_id;
     base::ListValue result;
     result.Append(base::Value::CreateNullValue());
     IPC::WriteParam(reply_msg, result);
     IPC::WriteParam(reply_msg, kGinJavaBridgeUnknownObjectId);
     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))
               .PassAs<GinJavaMethodInvocationHelper::ObjectDelegate>(),
           method_name,
           arguments);
   result->Init(this);
   g_background_thread.Get()
       .message_loop()
       ->message_loop_proxy()
@@ skipping 19 matching lines @@
   } else {
     ProcessMethodInvocationObjectResult(render_frame_host, reply_msg, 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(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;
     if (FindObjectId(result->GetObjectResult(), &returned_object_id)) {
       (*objects_.Lookup(returned_object_id))->AddHolder(render_frame_host);
     } else {
       returned_object_id = AddObject(result->GetObjectResult(),
                                      result->GetSafeAnnotationClass(),
                                      false,
                                      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(
     RenderFrameHost* render_frame_host,
     GinJavaBoundObject::ObjectID object_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(render_frame_host);
   if (objects_.Lookup(object_id)) {
     GinJavaBoundObject::ObjectMap::iterator iter(&objects_);
     while (!iter.IsAtEnd() && iter.GetCurrentKey() != object_id)
       iter.Advance();
     DCHECK(!iter.IsAtEnd());
     RemoveHolder(render_frame_host, iter, 1);
   }
 }
 
 }  // namespace content