Browser Plugin: New Implementation (Browser Side)

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index c70cbd09bbadece534bc5c460a622594275429d3..6a6a7fb2161aa6d3cd676d44679063770e845027 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -15,6 +15,8 @@
 #include "base/sys_info.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
+#include "content/browser/browser_plugin/browser_plugin_embedder.h"
+#include "content/browser/browser_plugin/browser_plugin_guest.h"
 #include "content/browser/browser_plugin/old/old_browser_plugin_host.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/debugger/devtools_manager_impl.h"
@@ -34,6 +36,7 @@
 #include "content/browser/web_contents/interstitial_page_impl.h"
 #include "content/browser/web_contents/navigation_entry_impl.h"
 #include "content/browser/webui/web_ui_impl.h"
+#include "content/common/browser_plugin_messages.h"
 #include "content/common/intents_messages.h"
 #include "content/common/ssl_status_serialization.h"
 #include "content/common/view_messages.h"
@@ -686,6 +689,8 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
                         OnSetSelectedColorInColorChooser)
     IPC_MESSAGE_HANDLER(ViewHostMsg_PepperPluginHung, OnPepperPluginHung)
     IPC_MESSAGE_HANDLER(ViewHostMsg_WebUISend, OnWebUISend)
+    IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_NavigateGuest,
+                        OnBrowserPluginNavigateGuest)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
   message_source_ = NULL;
@@ -2178,6 +2183,33 @@ void WebContentsImpl::OnWebUISend(const GURL& source_url,
     delegate_->WebUISend(this, source_url, name, args);
 }
 
+void WebContentsImpl::OnBrowserPluginNavigateGuest(int instance_id,
+                                                   int64 frame_id,
+                                                   std::string src,
+                                                   gfx::Size size) {
+  // This is the first 'navigate' to a browser plugin, before WebContents has/is
+  // an 'Embedder'; subsequent navigate messages for this WebContents will
+  // be handled by the BrowserPluginEmbedderHelper of the embedder itself (this
+  // also means any message from browser plugin renderer prior to NavigateGuest
+  // which is not NavigateGuest will be ignored). Therefore
+  // browser_plugin_embedder_ should not be set.
+  DCHECK(!browser_plugin_embedder_.get());
+
+  // TODO(lazyboy, rjkroege): This might trigger a race condition if we receive
+  // NavigateGuest message in WebContents via IO thread (by capturing it via

[Fady Samuel, 2012/08/27 15:35:00]

Why handle NavigateGuest on the I/O thread?

[lazyboy, 2012/08/28 19:07:14]

If we can do this in MessageFilter (running in IO) then we can install or not
install the filter based on current mode of the web_contents, (e.g. no such
filter if we are not in platform apps), that way the messages don't arrive at
all in WebContents; maybe this is not necessary, wdyt? (see comment below too
please).

[Charlie Reis, 2012/08/29 00:11:04]

Yeah, a race is worse than an unnecessary filter, in my opinion.

[lazyboy, 2012/08/31 00:38:04]

I've removed the todo since moving this to a filter will introduce race
condition.

+  // MesssageFilter) for the following case: if first two 'NavigateGuest'
+  // messages are around the same time (i.e. both gets routed to WebContents
+  // before BrowserPluginEmbedderHelper has a chance to intercept the second
+  // 'NavigateGuest' message).

[Charlie Reis, 2012/08/27 20:23:59]

I agree, this sounds fragile.  Maybe it's better for the guest to first
initialize the BrowserPluginEmbedder (on the UI thread) before sending
NavigateGuest, and only handle NavigateGuest in the BrowserPluginEmbedder?

(Or, as Fady asks, why make it possible to process this on the IO thread?)

[lazyboy, 2012/08/28 19:07:14]

(See comments above too), it sounds like it's better to keep it on UI thread?
I can remove the todo.

+  browser_plugin_embedder_.reset(
+      new content::BrowserPluginEmbedder(this, GetRenderViewHost()));
+  browser_plugin_embedder_->NavigateGuest(GetRenderViewHost(),
+                                          instance_id,
+                                          frame_id,
+                                          src,
+                                          size);
+}
+
 // Notifies the RenderWidgetHost instance about the fact that the page is
 // loading, or done loading and calls the base implementation.
 void WebContentsImpl::SetIsLoading(bool is_loading,
@@ -3148,3 +3180,19 @@ void WebContentsImpl::GetBrowserPluginEmbedderInfo(
                      embedder_process_id);
   }
 }
+
+content::BrowserPluginGuest* WebContentsImpl::AddBrowserPluginGuest(

[Fady Samuel, 2012/08/27 15:35:00]

I think SetBrowserPluginGuest might be a better name because a WebContents is
either a guest or its not, it does not keep track of more than one guest.

[lazyboy, 2012/08/28 19:07:14]

Done.

+    int instance_id) {
+  browser_plugin_guest_.reset(new content::BrowserPluginGuest(
+      instance_id, this, GetRenderViewHost()));
+  return browser_plugin_guest_.get();
+}
+
+content::BrowserPluginGuest* WebContentsImpl::GetBrowserPluginGuest() {
+  return browser_plugin_guest_.get();
+}
+
+content::BrowserPluginEmbedder* WebContentsImpl::
+    GetBrowserPluginEmbedder() {
+  return browser_plugin_embedder_.get();
+}