Make JavaBridgeDispatcherHost not be a RenderViewHostObserver anymore.

The only reason it was inheriting from it was to to dispatch an IPC from the renderer. Dispatch the IPC in WebContentsImpl instead, since that already knows about the java bridge code and send it to JavaBridgeDispatcherHostManager. I did this instead of exposing WebContentsImpl::message_source_ as no one else needs this yet, and I'm nervous of exposing it in case of misuse.

This also fixes usage of RenderProcessHost and RenderViewHost on the background java thread, which is not thread-safe, and a possible use-after-free with RenderViewHost.

This is part of the work in removing RenderViewHostObserver.

BUG=306569
R=steveblock@chromium.org, torne@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@231856 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 69 insertions, 89 deletions

diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host.cc b/content/browser/renderer_host/java/java_bridge_dispatcher_host.cc
index 95a1c6a..ece3056 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host.cc
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host.cc
@@ -18,7 +18,7 @@
 #include "third_party/WebKit/public/web/WebBindings.h"
 
 #if !defined(OS_ANDROID)
-#error "JavaBridge currently only supports OS_ANDROID"
+#error "JavaBridge only supports OS_ANDROID"
 #endif
 
 namespace content {
@@ -50,8 +50,7 @@ base::LazyInstance<JavaBridgeThread> g_background_thread =
 
 JavaBridgeDispatcherHost::JavaBridgeDispatcherHost(
     RenderViewHost* render_view_host)
-    : RenderViewHostObserver(render_view_host),
-      is_renderer_initialized_(false) {
+    : render_view_host_(render_view_host) {
 }
 
 JavaBridgeDispatcherHost::~JavaBridgeDispatcherHost() {
@@ -65,11 +64,8 @@ void JavaBridgeDispatcherHost::AddNamedObject(const string16& name,
   NPVariant_Param variant_param;
   CreateNPVariantParam(object, &variant_param);
 
-  if (!is_renderer_initialized_) {
-    is_renderer_initialized_ = true;
-    Send(new JavaBridgeMsg_Init(routing_id()));
-  }
-  Send(new JavaBridgeMsg_AddNamedObject(routing_id(), name, variant_param));
+  Send(new JavaBridgeMsg_AddNamedObject(
+      render_view_host_->GetRoutingID(), name, variant_param));
 }
 
 void JavaBridgeDispatcherHost::RemoveNamedObject(const string16& name) {
@@ -78,28 +74,12 @@ void JavaBridgeDispatcherHost::RemoveNamedObject(const string16& name) {
   // removed, the proxy object will delete its NPObjectProxy, which will cause
   // the NPObjectStub to be deleted, which will drop its reference to the
   // original NPObject.
-  Send(new JavaBridgeMsg_RemoveNamedObject(routing_id(), name));
-}
-
-bool JavaBridgeDispatcherHost::Send(IPC::Message* msg) {
-  return RenderViewHostObserver::Send(msg);
+  Send(new JavaBridgeMsg_RemoveNamedObject(
+      render_view_host_->GetRoutingID(), name));
 }
 
-void JavaBridgeDispatcherHost::RenderViewHostDestroyed(
-    RenderViewHost* render_view_host) {
-  // Base implementation deletes the object. This class is ref counted, with
-  // refs held by the JavaBridgeDispatcherHostManager and base::Bind, so that
-  // behavior is unwanted.
-}
-
-bool JavaBridgeDispatcherHost::OnMessageReceived(const IPC::Message& msg) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(JavaBridgeDispatcherHost, msg)
-    IPC_MESSAGE_HANDLER_DELAY_REPLY(JavaBridgeHostMsg_GetChannelHandle,
-                                    OnGetChannelHandle)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
+void JavaBridgeDispatcherHost::RenderViewDeleted() {
+  render_view_host_ = NULL;
 }
 
 void JavaBridgeDispatcherHost::OnGetChannelHandle(IPC::Message* reply_msg) {
@@ -108,6 +88,15 @@ void JavaBridgeDispatcherHost::OnGetChannelHandle(IPC::Message* reply_msg) {
       base::Bind(&JavaBridgeDispatcherHost::GetChannelHandle, this, reply_msg));
 }
 
+void JavaBridgeDispatcherHost::Send(IPC::Message* msg) {
+  if (render_view_host_) {
+    render_view_host_->Send(msg);
+    return;
+  }
+
+  delete msg;
+}
+
 void JavaBridgeDispatcherHost::GetChannelHandle(IPC::Message* reply_msg) {
   // The channel creates the channel handle based on the renderer ID we passed
   // to GetJavaBridgeChannelHost() and, on POSIX, the file descriptor used by
@@ -115,7 +104,11 @@ void JavaBridgeDispatcherHost::GetChannelHandle(IPC::Message* reply_msg) {
   JavaBridgeHostMsg_GetChannelHandle::WriteReplyParams(
       reply_msg,
       channel_->channel_handle());
-  Send(reply_msg);
+
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&JavaBridgeDispatcherHost::Send, this, reply_msg));
 }
 
 void JavaBridgeDispatcherHost::CreateNPVariantParam(NPObject* object,
@@ -139,16 +132,17 @@ void JavaBridgeDispatcherHost::CreateNPVariantParam(NPObject* object,
   g_background_thread.Get().message_loop()->PostTask(
       FROM_HERE,
       base::Bind(&JavaBridgeDispatcherHost::CreateObjectStub, this, object,
-                 route_id));
+                 render_view_host_->GetProcess()->GetID(), route_id));
 }
 
 void JavaBridgeDispatcherHost::CreateObjectStub(NPObject* object,
+                                                int render_process_id,
                                                 int route_id) {
   DCHECK_EQ(g_background_thread.Get().message_loop(),
             base::MessageLoop::current());
   if (!channel_.get()) {
     channel_ = JavaBridgeChannelHost::GetJavaBridgeChannelHost(
-        render_view_host()->GetProcess()->GetID(),
+        render_process_id,
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
   }
 
diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host.h b/content/browser/renderer_host/java/java_bridge_dispatcher_host.h
index 6d44c39..94ee632 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host.h
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host.h
@@ -10,11 +10,14 @@
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
 #include "content/child/npapi/npobject_stub.h"
-#include "content/public/browser/render_view_host_observer.h"
 
 class RouteIDGenerator;
 struct NPObject;
 
+namespace IPC {
+class Message;
+}
+
 namespace content {
 class NPChannelBase;
 class RenderViewHost;
@@ -25,11 +28,10 @@ struct NPVariant_Param;
 // proxy object is created in the renderer. An instance of this class exists
 // for each RenderViewHost.
 class JavaBridgeDispatcherHost
-    : public base::RefCountedThreadSafe<JavaBridgeDispatcherHost>,
-      public RenderViewHostObserver {
+    : public base::RefCountedThreadSafe<JavaBridgeDispatcherHost> {
  public:
   // We hold a weak pointer to the RenderViewhost. It must outlive this object.
-  JavaBridgeDispatcherHost(RenderViewHost* render_view_host);
+  explicit JavaBridgeDispatcherHost(RenderViewHost* render_view_host);
 
   // Injects |object| into the main frame of the corresponding RenderView. A
   // proxy object is created in the renderer and when the main frame's window
@@ -43,28 +45,23 @@ class JavaBridgeDispatcherHost
   void AddNamedObject(const string16& name, NPObject* object);
   void RemoveNamedObject(const string16& name);
 
-  // RenderViewHostObserver overrides:
-  // The IPC macros require this to be public.
-  virtual bool Send(IPC::Message* msg) OVERRIDE;
-  virtual void RenderViewHostDestroyed(
-      RenderViewHost* render_view_host) OVERRIDE;
+  // Since this object is ref-counted, it might outlive render_view_host_.
+  void RenderViewDeleted();
+
+  void OnGetChannelHandle(IPC::Message* reply_msg);
 
  private:
   friend class base::RefCountedThreadSafe<JavaBridgeDispatcherHost>;
   virtual ~JavaBridgeDispatcherHost();
 
-  // RenderViewHostObserver override:
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
-
-  // Message handlers
-  void OnGetChannelHandle(IPC::Message* reply_msg);
+  void Send(IPC::Message* msg);
 
   void GetChannelHandle(IPC::Message* reply_msg);
   void CreateNPVariantParam(NPObject* object, NPVariant_Param* param);
-  void CreateObjectStub(NPObject* object, int route_id);
+  void CreateObjectStub(NPObject* object, int render_process_id, int route_id);
 
   scoped_refptr<NPChannelBase> channel_;
-  bool is_renderer_initialized_;
+  RenderViewHost* render_view_host_;
   std::vector<base::WeakPtr<NPObjectStub> > stubs_;
 
   DISALLOW_COPY_AND_ASSIGN(JavaBridgeDispatcherHost);
diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
index 5625d05..a3822a0 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.cc
@@ -82,6 +82,11 @@ void JavaBridgeDispatcherHostManager::RemoveNamedObject(const string16& name) {
   }
 }
 
+void JavaBridgeDispatcherHostManager::OnGetChannelHandle(
+    RenderViewHost* render_view_host, IPC::Message* reply_msg) {
+  instances_[render_view_host]->OnGetChannelHandle(reply_msg);
+}
+
 void JavaBridgeDispatcherHostManager::RenderViewCreated(
     RenderViewHost* render_view_host) {
   // Creates a JavaBridgeDispatcherHost for the specified RenderViewHost and
@@ -99,6 +104,9 @@ void JavaBridgeDispatcherHostManager::RenderViewCreated(
 
 void JavaBridgeDispatcherHostManager::RenderViewDeleted(
     RenderViewHost* render_view_host) {
+  if (!instances_.count(render_view_host))  // Needed for tests.
+    return;
+  instances_[render_view_host]->RenderViewDeleted();
   instances_.erase(render_view_host);
 }
 
diff --git a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
index e2bb7dc..e3d629a 100644
--- a/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
+++ b/content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h
@@ -37,6 +37,9 @@ class JavaBridgeDispatcherHostManager
   void AddNamedObject(const string16& name, NPObject* object);
   void RemoveNamedObject(const string16& name);
 
+  void OnGetChannelHandle(RenderViewHost* render_view_host,
+                          IPC::Message* reply_msg);
+
   // Every time a JavaBoundObject backed by a real Java object is
   // created/destroyed, we insert/remove a strong ref to that Java object into
   // this set so that it doesn't get garbage collected while it's still
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index f089c4a..ed29f97 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -87,6 +87,8 @@
 
 #if defined(OS_ANDROID)
 #include "content/browser/android/date_time_chooser_android.h"
+#include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
+#include "content/common/java_bridge_messages.h"
 #include "content/public/browser/android/content_view_core.h"
 #endif
 
@@ -95,10 +97,6 @@
 #include "ui/gl/io_surface_support_mac.h"
 #endif
 
-#if defined(OS_ANDROID)
-#include "content/browser/renderer_host/java/java_bridge_dispatcher_host_manager.h"
-#endif
-
 // Cross-Site Navigations
 //
 // If a WebContentsImpl is told to navigate to a different web site (as
@@ -527,6 +525,8 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
                         OnFindMatchRectsReply)
     IPC_MESSAGE_HANDLER(ViewHostMsg_OpenDateTimeDialog,
                         OnOpenDateTimeDialog)
+    IPC_MESSAGE_HANDLER_DELAY_REPLY(JavaBridgeHostMsg_GetChannelHandle,
+                                    OnJavaBridgeGetChannelHandle)
 #endif
     IPC_MESSAGE_HANDLER(ViewHostMsg_MediaNotification, OnMediaNotification)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -1050,9 +1050,6 @@ void WebContentsImpl::Init(const WebContents::CreateParams& params) {
 #if defined(OS_ANDROID)
   java_bridge_dispatcher_host_manager_.reset(
       new JavaBridgeDispatcherHostManager(this));
-#endif
-
-#if defined(OS_ANDROID)
   date_time_chooser_.reset(new DateTimeChooserAndroid());
 #endif
 }
@@ -2310,6 +2307,11 @@ void WebContentsImpl::OnOpenDateTimeDialog(
                                  value.step);
 }
 
+void WebContentsImpl::OnJavaBridgeGetChannelHandle(IPC::Message* reply_msg) {
+  java_bridge_dispatcher_host_manager_->OnGetChannelHandle(
+      message_source_, reply_msg);
+}
+
 #endif
 
 void WebContentsImpl::OnCrashedPlugin(const base::FilePath& plugin_path,
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 5f226c4..6a21e07 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -636,6 +636,7 @@ class CONTENT_EXPORT WebContentsImpl
 
   void OnOpenDateTimeDialog(
       const ViewHostMsg_DateTimeDialogValue_Params& value);
+  void OnJavaBridgeGetChannelHandle(IPC::Message* reply_msg);
 #endif
   void OnCrashedPlugin(const base::FilePath& plugin_path,
                        base::ProcessId plugin_pid);
diff --git a/content/common/java_bridge_messages.h b/content/common/java_bridge_messages.h
index 62923c7..7f2d8c1 100644
--- a/content/common/java_bridge_messages.h
+++ b/content/common/java_bridge_messages.h
@@ -16,9 +16,6 @@
 
 // Messages for handling Java objects injected into JavaScript -----------------
 
-// Sent from browser to renderer to initialize the Java Bridge.
-IPC_MESSAGE_ROUTED0(JavaBridgeMsg_Init)
-
 // Sent from browser to renderer to add a Java object with the given name.
 IPC_MESSAGE_ROUTED2(JavaBridgeMsg_AddNamedObject,
                     string16 /* name */,
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 3f79c62..8fcfd89 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -861,7 +861,6 @@ RenderViewImpl::RenderViewImpl(RenderViewImplParams* params)
       devtools_agent_(NULL),
       accessibility_mode_(AccessibilityModeOff),
       renderer_accessibility_(NULL),
-      java_bridge_dispatcher_(NULL),
       mouse_lock_dispatcher_(NULL),
 #if defined(OS_ANDROID)
       body_background_color_(SK_ColorWHITE),
@@ -987,6 +986,7 @@ void RenderViewImpl::Initialize(RenderViewImplParams* params) {
 
 #if defined(OS_ANDROID)
   media_player_manager_.reset(new RendererMediaPlayerManager());
+  new JavaBridgeDispatcher(this);
 #endif
 
   // The next group of objects all implement RenderViewObserver, so are deleted
@@ -1457,11 +1457,12 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_SetHistoryLengthAndPrune,
                         OnSetHistoryLengthAndPrune)
     IPC_MESSAGE_HANDLER(ViewMsg_EnableViewSourceMode, OnEnableViewSourceMode)
-#if defined(OS_ANDROID)
-    IPC_MESSAGE_HANDLER(JavaBridgeMsg_Init, OnJavaBridgeInit)
-#endif
     IPC_MESSAGE_HANDLER(ViewMsg_SetAccessibilityMode, OnSetAccessibilityMode)
     IPC_MESSAGE_HANDLER(ViewMsg_DisownOpener, OnDisownOpener)
+    IPC_MESSAGE_HANDLER(ViewMsg_ReleaseDisambiguationPopupDIB,
+                        OnReleaseDisambiguationPopupDIB)
+    IPC_MESSAGE_HANDLER(ViewMsg_WindowSnapshotCompleted,
+                        OnWindowSnapshotCompleted)
 #if defined(OS_ANDROID)
     IPC_MESSAGE_HANDLER(InputMsg_ActivateNearestFindResult,
                         OnActivateNearestFindResult)
@@ -1481,10 +1482,8 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_SetWindowVisibility, OnSetWindowVisibility)
     IPC_MESSAGE_HANDLER(ViewMsg_WindowFrameChanged, OnWindowFrameChanged)
 #endif
-    IPC_MESSAGE_HANDLER(ViewMsg_ReleaseDisambiguationPopupDIB,
-                        OnReleaseDisambiguationPopupDIB)
-    IPC_MESSAGE_HANDLER(ViewMsg_WindowSnapshotCompleted,
-                        OnWindowSnapshotCompleted)
+    // Adding a new message? Add platform independent ones first, then put the
+    // platform specific ones at the end.
 
     // Have the super handle all other messages.
     IPC_MESSAGE_UNHANDLED(handled = RenderWidget::OnMessageReceived(message))
@@ -5316,6 +5315,7 @@ void RenderViewImpl::OnMediaPlayerActionAt(const gfx::Point& location,
 }
 
 void RenderViewImpl::OnOrientationChangeEvent(int orientation) {
+  // Screen has rotated. 0 = default (portrait), 90 = one turn right, and so on.
   FOR_EACH_OBSERVER(RenderViewObserver,
                     observers_,
                     OrientationChangeEvent(orientation));
@@ -6434,13 +6434,6 @@ void RenderViewImpl::OnEnableViewSourceMode() {
   main_frame->enableViewSourceMode(true);
 }
 
-#if defined(OS_ANDROID)
-void RenderViewImpl::OnJavaBridgeInit() {
-  DCHECK(!java_bridge_dispatcher_);
-  java_bridge_dispatcher_ = new JavaBridgeDispatcher(this);
-}
-#endif
-
 void RenderViewImpl::OnDisownOpener() {
   if (!webview())
     return;
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
index 1830ba5..53b291f 100644
--- a/content/renderer/render_view_impl.h
+++ b/content/renderer/render_view_impl.h
@@ -924,7 +924,6 @@ class CONTENT_EXPORT RenderViewImpl
   void OnSetEditCommandsForNextKeyEvent(const EditCommands& edit_commands);
   void OnUndo();
   void OnUnselect();
-
   void OnAllowBindings(int enabled_bindings_flags);
   void OnAllowScriptToClose(bool script_can_close);
   void OnCancelDownload(int32 download_id);
@@ -975,10 +974,7 @@ class CONTENT_EXPORT RenderViewImpl
       const base::FilePath& local_directory_name);
   void OnMediaPlayerActionAt(const gfx::Point& location,
                              const WebKit::WebMediaPlayerAction& action);
-
-  // Screen has rotated. 0 = default (portrait), 90 = one turn right, and so on.
   void OnOrientationChangeEvent(int orientation);
-
   void OnPluginActionAt(const gfx::Point& location,
                         const WebKit::WebPluginAction& action);
   void OnMoveOrResizeStarted();
@@ -1015,16 +1011,12 @@ class CONTENT_EXPORT RenderViewImpl
   void OnUpdateTargetURLAck();
   void OnUpdateTimezone();
   void OnUpdateWebPreferences(const WebPreferences& prefs);
-
   void OnZoom(PageZoom zoom);
   void OnZoomFactor(PageZoom zoom, int zoom_center_x, int zoom_center_y);
-
   void OnEnableViewSourceMode();
-
-  void OnJavaBridgeInit();
-
   void OnDisownOpener();
-
+  void OnWindowSnapshotCompleted(const int snapshot_id,
+      const gfx::Size& size, const std::vector<unsigned char>& png);
 #if defined(OS_ANDROID)
   void OnActivateNearestFindResult(int request_id, float x, float y);
   void OnFindMatchRects(int current_version);
@@ -1046,10 +1038,6 @@ class CONTENT_EXPORT RenderViewImpl
                             const gfx::Rect& view_frame);
 #endif
 
-  void OnWindowSnapshotCompleted(const int snapshot_id,
-      const gfx::Size& size, const std::vector<unsigned char>& png);
-
-
   // Adding a new message handler? Please add it in alphabetical order above
   // and put it in the same position in the .cc file.
 
@@ -1424,9 +1412,6 @@ class CONTENT_EXPORT RenderViewImpl
   // AccessibilityModeOff.
   RendererAccessibility* renderer_accessibility_;
 
-  // Java Bridge dispatcher attached to this view; lazily initialized.
-  JavaBridgeDispatcher* java_bridge_dispatcher_;
-
   // Mouse Lock dispatcher attached to this view.
   MouseLockDispatcher* mouse_lock_dispatcher_;