Browser Plugin: browser process side changes

content/browser/browser_plugin/browser_plugin_host.cc

+// Copyright (c) 2012 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/browser_plugin/browser_plugin_host.h"
+
+#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/common/browser_plugin_messages.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_widget_host_view.h"
+#include "content/public/browser/site_instance.h"
+#include "content/public/browser/web_contents_view.h"
+#include "ppapi/proxy/ppapi_messages.h"
+
+namespace content {
+
+BrowserPluginHost::BrowserPluginHost(
+    WebContentsImpl* web_contents)
+    : WebContentsObserver(web_contents),
+      embedder_(NULL),
+      instance_id_(0),
+      pending_render_view_host_(NULL) {
+  // Listen to visibility changes so that an embedder hides its guests
+  // as well.
+  registrar_.Add(this,
+                 NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED,
+                 Source<RenderViewHost>(
+                     web_contents->GetRenderViewHost()));

[Charlie Reis, 2012/05/18 20:35:24]

Just want to sanity check that we know this is the right RenderViewHost.  Which
web_contents is this, and are we certain that it hasn't changed its RVH already?

[Fady Samuel, 2012/05/21 14:30:17]

Using the new NOTIFICATION_WEB_CONTENTS_VISIBLITY_CHANGE.

+}
+
+BrowserPluginHost::~BrowserPluginHost() {
+}
+
+BrowserPluginHost* BrowserPluginHost::GetGuestByContainerID(int container_id) {
+  ContainerInstanceMap::const_iterator it =
+      guests_by_container_id_.find(container_id);
+  if (it != guests_by_container_id_.end())
+    return it->second;
+  return NULL;
+}
+
+void BrowserPluginHost::RegisterContainerInstance(
+    int container_id,
+    BrowserPluginHost* observer) {
+  DCHECK(guests_by_container_id_.find(container_id) ==
+      guests_by_container_id_.end());
+  guests_by_container_id_[container_id] = observer;
+}
+
+void BrowserPluginHost::OnCrossProcessNavigation(RenderViewHost* dest_rvh) {
+  if (web_contents()->GetRenderViewHost() != dest_rvh) {
+    pending_render_view_host_ = dest_rvh;
+  }
+}
+
+
+bool BrowserPluginHost::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(BrowserPluginHost, message)
+    IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_ConnectToChannel,
+                        OnConnectToChannel)
+    IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_NavigateFromGuest,
+                        OnNavigateFromGuest)
+    IPC_MESSAGE_HANDLER(BrowserPluginHostMsg_MapInstance,
+                        OnMapInstance)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+void BrowserPluginHost::NavigateGuestFromEmbedder(
+    RenderViewHost* render_view_host,
+    int container_instance_id,
+    long long frame_id,
+    const std::string& src,
+    const gfx::Size& size) {
+  BrowserPluginHost* guest_observer =
+      GetGuestByContainerID(container_instance_id);
+  WebContentsImpl* guest_web_contents =
+      guest_observer ?
+          static_cast<WebContentsImpl*>(guest_observer->web_contents()): NULL;
+  if (!guest_observer) {
+    GURL url(src);
+    guest_web_contents =
+        static_cast<WebContentsImpl*>(
+            WebContents::Create(
+                web_contents()->GetBrowserContext(),
+                render_view_host->GetSiteInstance(),
+                MSG_ROUTING_NONE,
+                NULL, // base WebContents
+                NULL  // session storage namespace
+            ));
+    guest_observer =
+        guest_web_contents->browser_plugin_host();
+    guest_observer->set_embedder(static_cast<WebContentsImpl*>(web_contents()));
+    guest_observer->set_instance_id(container_instance_id);
+    guest_observer->set_url(url);

[Charlie Reis, 2012/05/18 20:35:24]

Does the observer need to know the url?  The fewer things that keep it as state
the better.

[Fady Samuel, 2012/05/21 14:30:17]

Done. Initial size will go away in a subsequent patch too.

+    guest_observer->set_initial_size(size);
+    RegisterContainerInstance(container_instance_id, guest_observer);
+    AddGuest(guest_web_contents, frame_id);
+  }
+
+  guest_observer->web_contents()->SetDelegate(guest_observer);
+  guest_observer->web_contents()->GetController().LoadURL(
+      guest_observer->url(),
+      Referrer(),
+      PAGE_TRANSITION_AUTO_SUBFRAME,
+      std::string());
+}
+
+void BrowserPluginHost::OnNavigateFromGuest(
+    PP_Instance instance,
+    const std::string& src) {
+  DCHECK(embedder());
+  GURL url(src);
+  web_contents()->GetController().LoadURL(
+      url,
+      Referrer(),
+      PAGE_TRANSITION_AUTO_SUBFRAME,
+      std::string());
+}
+
+// TODO(fsamuel): This handler is all kinds of bad and could be racy.

[Charlie Reis, 2012/05/18 20:35:24]

Please add more of an explanation why this is racy: the message could be coming
from either the current or pending RVH, whether or not there is a pending one.

I want to make sure we don't forget about this TODO, so please file a bug for it
and include the number here as well.

[Fady Samuel, 2012/05/21 14:30:17]

I don't think that's the issue. The problem here isn't where the message is
coming from (it's always coming from the embedder) but rather, where the message
is going to: it needs to go to the new render process that we're navigating to.
Between the time we start a new pending navigation and call MapInstance, it may
be the case that the pending_render_view_host is no longer valid, I think. I'm
really not sure what problems might arise here, but I'll file the bug to get rid
of this potentially buggy way of handling things.

+// The correct solution is probably to send
+// "BrowserPluginMsg_CompleteNavigation" over the pepper channel to the guest
+// and then have the guest send the browser "BrowserPluginHostMsg_ResizeGuest"
+// to resize appropriately.
+void BrowserPluginHost::OnMapInstance(int container_instance_id,
+                                      PP_Instance instance) {
+  BrowserPluginHost* guest_observer =
+      GetGuestByContainerID(container_instance_id);
+  WebContentsImpl* guest_web_contents =
+    static_cast<WebContentsImpl*>(guest_observer->web_contents());
+  RenderViewHost* rvh = guest_observer->pending_render_view_host() ?
+      guest_observer->pending_render_view_host() :
+          guest_web_contents->GetRenderViewHost();
+
+  guest_web_contents->GetView()->SizeContents(guest_observer->initial_size());
+
+  rvh->Send(new BrowserPluginMsg_CompleteNavigation(
+                rvh->GetRoutingID(),
+                instance));
+}
+
+void BrowserPluginHost::OnConnectToChannel(
+    const IPC::ChannelHandle& channel_handle) {
+  DCHECK(embedder());
+  // Tell the BrowserPlugin in the embedder that we're done and that it can
+  // begin using the guest renderer.
+  embedder()->GetRenderProcessHost()->Send(

[Charlie Reis, 2012/05/18 20:35:24]

This looks unsafe as well, as does the GetPendingSiteInstance call below.  Can
we pass in the actual RenderProcessHost(s) or their IDs instead?  Sorry to be
strict on this, but I've spent much too long debugging races from problems like
this.

[Fady Samuel, 2012/05/21 14:30:17]

Yes, the guest BrowserPluginHost can keep track of the embedder's
RenderProcessHost on creation in NavigateGuestFromEmbedder.

+      new BrowserPluginMsg_LoadGuest(
+          instance_id(),
+          //embedder_web_contents->GetRenderProcessHost()->GetID(),

[Charlie Reis, 2012/05/18 20:35:24]

Remove commented code.

[Fady Samuel, 2012/05/21 14:30:17]

Done.

+          web_contents()->GetPendingSiteInstance()->GetProcess()->
+              GetID(),
+          channel_handle));
+}
+
+void BrowserPluginHost::AddGuest(WebContentsImpl* guest, int64 frame_id) {
+  guests_[guest] = frame_id;
+}
+
+void BrowserPluginHost::RemoveGuest(WebContentsImpl* guest) {
+  guests_.erase(guest);
+}
+
+void BrowserPluginHost::DestroyGuests() {
+  for (GuestMap::const_iterator it = guests_.begin();
+       it != guests_.end(); ++it) {
+    WebContentsImpl* web_contents = it->first;
+    delete web_contents;
+  }
+  guests_.clear();
+  guests_by_container_id_.clear();
+}
+
+void BrowserPluginHost::DidCommitProvisionalLoadForFrame(
+    int64 frame_id,
+    bool is_main_frame,
+    const GURL& url,
+    PageTransition transition_type,
+    RenderViewHost* render_view_host) {
+  // Clean-up guests that lie in the frame that we're navigating.
+  typedef std::set<WebContentsImpl*> GuestSet;
+  GuestSet guests_to_delete;
+  for (GuestMap::const_iterator it = guests_.begin();
+       it != guests_.end(); ++it) {
+    WebContentsImpl* web_contents = it->first;
+    if (it->second == frame_id) {
+      guests_to_delete.insert(web_contents);
+    }
+  }
+  for (GuestSet::const_iterator it = guests_to_delete.begin();
+       it != guests_to_delete.end(); ++it) {
+    delete *it;
+    guests_.erase(*it);
+  }
+}
+
+void BrowserPluginHost::RenderViewDeleted(RenderViewHost* render_view_host) {
+  // TODO(fsamuel): For some reason ToT hangs when killing a guest, this wasn't
+  // the case earlier. Presumably, when a guest is killed/crashes, one of
+  // RenderViewDeleted, RenderViewGone or WebContentsDestroyed will get called.
+  // At that point, we should remove reference to this BrowserPluginHost
+  // from its embedder, in addition to destroying its guests, to ensure
+  // that we do not attempt a double delete.
+  DestroyGuests();
+}
+
+void BrowserPluginHost::RenderViewGone(base::TerminationStatus status) {
+  DestroyGuests();
+}
+
+void BrowserPluginHost::WebContentsDestroyed(WebContents* web_contents) {
+  DestroyGuests();
+}
+
+void BrowserPluginHost::Observe(
+    int type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  DCHECK(type == NOTIFICATION_RENDER_WIDGET_VISIBILITY_CHANGED);
+  bool visible = *Details<bool>(details).ptr();
+
+  // If the embedder is hidden we need to hide the guests as well.
+  for (GuestMap::const_iterator it = guests_.begin();
+       it != guests_.end(); ++it) {
+    WebContentsImpl* web_contents = it->first;
+    if (visible)
+      web_contents->ShowContents();
+    else
+      web_contents->HideContents();
+  }
+}
+
+}  // namespace content