Fix crash when calling WebView.removeJavascriptInterface

It may crash to call WebView.addJavascriptInterface followed
by WebView.removeJavascriptInterface, when WebContents is
switching RenderFrameHosts due to WebView.lodUrl.

How it the crash happen:
1) WebView.loadUrl
2) Create a new RenderFrameHost in browser process and a new
   RenderFrame in render process, and now the RenderFrameHost
   is in pending status
3) GinJavaBridgeDispatcherHost:RenderFrameCreated is called
   to add javascript interface object to new created
   RenderFrameHost/RenderFrame.
4) WebView.addJavascriptInterface(object, "test_name") is called,
   but only RenderFrameHosts in FrameTree is got notified to
   add "test_name", the pending RenderFrameHost is not notified
5) WebContents commit the pending RenderFrameHost, now it is in
   the FrameTree
6) WebView.removeJavascriptInterface is called, and the new
   committed RenderFrameHost is got notified. the peer
   GinJavaBridgeDispatcher in render process is notified,
   and GinJavaBridgeDispatcher::OnRemoveNamedObject is called.
   but GinJavaBridgeDispatcher of the RenderFrame never has
   a entry "test_name" stored in named_objects_, so it's crashed

How to fix it:
The solution is to also notify pending RenderFrameHosts to avoid
missing them when they are created but not committed

Bug: 1087806
Change-Id: Iecc1223d1f2b4f6791cdb9e3393db874bf766a5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2237387
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776666}

AUTHORS

@@ -1047,6 +1047,7 @@ Xinghua Cao <xinghua.cao@intel.com>
 Xu Samuel <samuel.xu@intel.com>
 Xu Xing <xing.xu@intel.com>
 Xuefei Ren <xrenishere@gmail.com>
+Xuehui Xie <xuehui.xxh@alibaba-inc.com>
 Xueqing Huang <huangxueqing@xiaomi.com>
 Xun Sun <xun.sun@intel.com>
 Xunran Ding <xunran.ding@samsung.com>

content/browser/android/java/gin_java_bridge_dispatcher_host.cc

@@ -11,6 +11,7 @@
 #include "content/browser/android/java/gin_java_bound_object_delegate.h"
 #include "content/browser/android/java/gin_java_bridge_message_filter.h"
 #include "content/browser/android/java/java_bridge_thread.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/android/gin_java_bridge_value.h"
 #include "content/common/android/hash_set.h"
 #include "content/common/gin_java_bridge_messages.h"
@@ -208,7 +209,12 @@ void GinJavaBridgeDispatcherHost::AddNamedObject(
   // notified the observers about new RenderFrame, it is necessary to ensure
   // here that all render frame IDs are registered with the filter.
   InstallFilterAndRegisterAllRoutingIds();
-  web_contents()->SendToAllFrames(
+  // We should include pending RenderFrameHosts, otherwise they will miss the
+  // chance when calling add or remove methods when they are created but not
+  // committed. See: http://crbug.com/1087806
+  WebContentsImpl* web_contents_impl =
+      static_cast<WebContentsImpl*>(web_contents());
+  web_contents_impl->SendToAllFramesIncludingPending(
       new GinJavaBridgeMsg_AddNamedObject(MSG_ROUTING_NONE, name, object_id));
 }
 
@@ -234,7 +240,12 @@ void GinJavaBridgeDispatcherHost::RemoveNamedObject(
   // hold it. All the transient objects and removed named objects will be purged
   // during the cleansing caused by DocumentAvailableInMainFrame event.
 
-  web_contents()->SendToAllFrames(
+  // We should include pending RenderFrameHosts, otherwise they will miss the
+  // chance when calling add or remove methods when they are created but not
+  // committed. See: http://crbug.com/1087806
+  WebContentsImpl* web_contents_impl =
+      static_cast<WebContentsImpl*>(web_contents());
+  web_contents_impl->SendToAllFramesIncludingPending(
       new GinJavaBridgeMsg_RemoveNamedObject(MSG_ROUTING_NONE, copied_name));
 }
 

content/browser/web_contents/web_contents_impl.cc

@@ -322,6 +322,40 @@ bool AnyInnerWebContents(WebContents* web_contents, const Functor& f) {
   return std::any_of(inner_contents.begin(), inner_contents.end(), f);
 }
 
+std::vector<RenderFrameHost*> GetAllFramesImpl(FrameTree& frame_tree,
+                                               bool include_pending) {
+  std::vector<RenderFrameHost*> frame_hosts;
+  for (FrameTreeNode* node : frame_tree.Nodes()) {
+    frame_hosts.push_back(node->current_frame_host());
+    if (include_pending) {
+      RenderFrameHostImpl* pending_frame_host =
+          node->render_manager()->speculative_frame_host();
+      if (pending_frame_host)
+        frame_hosts.push_back(pending_frame_host);
+    }
+  }
+  return frame_hosts;
+}
+
+int SendToAllFramesImpl(FrameTree& frame_tree,
+                        bool include_pending,
+                        IPC::Message* message) {
+  int number_of_messages = 0;
+  std::vector<RenderFrameHost*> frame_hosts =
+      GetAllFramesImpl(frame_tree, include_pending);
+  for (RenderFrameHost* rfh : frame_hosts) {
+    if (!rfh->IsRenderFrameLive())
+      continue;
+
+    ++number_of_messages;
+    IPC::Message* message_copy = new IPC::Message(*message);
+    message_copy->set_routing_id(rfh->GetRoutingID());
+    rfh->Send(message_copy);
+  }
+  delete message;
+  return number_of_messages;
+}
+
 }  // namespace
 
 CreatedWindow::CreatedWindow() = default;
@@ -1050,25 +1084,19 @@ void WebContentsImpl::ForEachFrame(
 }
 
 std::vector<RenderFrameHost*> WebContentsImpl::GetAllFrames() {
-  std::vector<RenderFrameHost*> frame_hosts;
-  for (FrameTreeNode* node : frame_tree_.Nodes())
-    frame_hosts.push_back(node->current_frame_host());
-  return frame_hosts;
+  return GetAllFramesImpl(frame_tree_, /*include_pending=*/false);
+}
+
+std::vector<RenderFrameHost*> WebContentsImpl::GetAllFramesIncludingPending() {
+  return GetAllFramesImpl(frame_tree_, /*include_pending=*/true);
 }
 
 int WebContentsImpl::SendToAllFrames(IPC::Message* message) {
-  int number_of_messages = 0;
-  for (RenderFrameHost* rfh : GetAllFrames()) {
-    if (!rfh->IsRenderFrameLive())
-      continue;
+  return SendToAllFramesImpl(frame_tree_, /*include_pending=*/false, message);
+}
 
-    ++number_of_messages;
-    IPC::Message* message_copy = new IPC::Message(*message);
-    message_copy->set_routing_id(rfh->GetRoutingID());
-    rfh->Send(message_copy);
-  }
-  delete message;
-  return number_of_messages;
+int WebContentsImpl::SendToAllFramesIncludingPending(IPC::Message* message) {
+  return SendToAllFramesImpl(frame_tree_, /*include_pending=*/true, message);
 }
 
 void WebContentsImpl::SendPageMessage(IPC::Message* msg) {

content/browser/web_contents/web_contents_impl.h

@@ -1212,6 +1212,12 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   // WebContentsDelegate.
   void SystemDragEnded(RenderWidgetHost* source_rwh);
 
+  // They are similar to functions GetAllFrames() and SendToAllFrames() in
+  // WebContents interface, but also include pendings frames. See bug:
+  // http://crbug.com/1087806
+  std::vector<RenderFrameHost*> GetAllFramesIncludingPending();
+  int SendToAllFramesIncludingPending(IPC::Message* message);
+
  private:
   friend class WebContentsObserver;
   friend class WebContents;  // To implement factory methods.