Browser Plugin: New Implementation (Browser Side)

content/browser/browser_plugin/browser_plugin_embedder.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_embedder.h"
+
+#include <set>
+
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/time.h"
+#include "content/browser/browser_plugin/browser_plugin_embedder_helper.h"
+#include "content/browser/browser_plugin/browser_plugin_guest.h"
+#include "content/browser/browser_plugin/browser_plugin_guest_helper.h"
+#include "content/browser/browser_plugin/browser_plugin_host_factory.h"
+#include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/renderer_host/render_widget_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
+#include "content/common/browser_plugin_messages.h"
+#include "content/common/view_messages.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.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_view_host.h"
+#include "content/public/browser/render_widget_host_view.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_view.h"
+#include "content/public/common/url_constants.h"
+#include "ui/gfx/size.h"
+
+namespace content {
+
+// static
+BrowserPluginHostFactory* BrowserPluginEmbedder::factory_ = NULL;
+
+BrowserPluginEmbedder::BrowserPluginEmbedder(
+    WebContentsImpl* web_contents,
+    RenderViewHost* render_view_host)
+    : WebContentsObserver(web_contents),
+      render_view_host_(render_view_host) {
+  // Listen to visibility changes so that an embedder hides its guests
+  // as well.
+  registrar_.Add(this,
+                 NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
+                 Source<WebContents>(web_contents));
+
+  new BrowserPluginEmbedderHelper(this, render_view_host);

[awong, 2012/09/09 18:08:09]

Comment about ownership.

[lazyboy, 2012/09/10 16:30:17]

Done.

+}
+
+BrowserPluginEmbedder::~BrowserPluginEmbedder() {
+  // Destroy guests that are managed by the current embedder, for more details
+  // see BrowserPluginEmbedder::DidCommitProvisionalLoadForFrame().
+  DestroyGuests();
+}
+
+// static
+BrowserPluginEmbedder* BrowserPluginEmbedder::Create(
+    WebContentsImpl* web_contents,
+    content::RenderViewHost* render_view_host) {
+  if (factory_) {
+    return factory_->CreateBrowserPluginEmbedder(web_contents,
+                                                 render_view_host);
+  }
+  return new BrowserPluginEmbedder(web_contents, render_view_host);
+}
+
+BrowserPluginGuest* BrowserPluginEmbedder::GetGuestByInstanceID(
+    int instance_id) const {
+  ContainerInstanceMap::const_iterator it =
+      guests_by_instance_id_.find(instance_id);
+  if (it != guests_by_instance_id_.end())
+    return it->second;
+  return NULL;
+}
+
+void BrowserPluginEmbedder::AddGuest(int instance_id,
+                                     BrowserPluginGuest* guest,
+                                     int64 frame_id) {
+  DCHECK(guests_by_instance_id_.find(instance_id) ==
+        guests_by_instance_id_.end());
+  guests_by_instance_id_[instance_id] = guest;
+  guest_web_contents_container_[guest->web_contents()] = frame_id;
+}
+
+void BrowserPluginEmbedder::NavigateGuest(RenderViewHost* render_view_host,
+                                          int instance_id,
+                                          int64 frame_id,
+                                          const std::string& src,
+                                          const gfx::Size& size) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  WebContentsImpl* guest_web_contents = NULL;
+  GURL url(src);
+  if (!guest) {
+    // The SiteInstance of a given guest is based on the fact that it's a guest
+    // in addition to which platform application the guest belongs to, rather
+    // than the URL that the guest is being navigated to.

[awong, 2012/09/09 18:08:09]

This comment seems to make more sense in CreateGuest.

[lazyboy, 2012/09/10 16:30:17]

Hmmm, I also have this comment in CreateGuest, removed from here.

[awong, 2012/09/10 19:21:07]

Still seems to be here...

[lazyboy, 2012/09/10 23:40:57]

Missed it somehow :(
Done.

+    std::string host = render_view_host->GetSiteInstance()->GetSite().host();

[awong, 2012/09/09 18:08:09]

const std::string& host

[lazyboy, 2012/09/10 16:30:17]

Done.

+    guest_web_contents = static_cast<WebContentsImpl*>(
+        WebContents::CreateGuest(web_contents()->GetBrowserContext(),
+                                 host,
+                                 instance_id));
+
+    guest = guest_web_contents->GetBrowserPluginGuest();
+    guest->set_embedder_render_process_host(
+        render_view_host->GetProcess());
+
+    guest_web_contents->GetMutableRendererPrefs()->
+        throttle_input_events = false;
+    AddGuest(instance_id, guest, frame_id);
+    guest_web_contents->SetDelegate(guest);
+  } else {
+    guest_web_contents = static_cast<WebContentsImpl*>(guest->web_contents());
+  }
+  guest_web_contents->GetController().LoadURL(url,
+                                              Referrer(),
+                                              PAGE_TRANSITION_AUTO_SUBFRAME,
+                                              std::string());
+  if (!size.IsEmpty())
+    guest_web_contents->GetView()->SizeContents(size);
+}
+
+void BrowserPluginEmbedder::UpdateRectACK(int instance_id,
+                                          int message_id,
+                                          const gfx::Size& size) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  if (guest)
+    guest->UpdateRectACK(message_id, size);
+}
+
+void BrowserPluginEmbedder::ResizeGuest(int instance_id,
+                                        TransportDIB* damage_buffer,
+                                        int width,
+                                        int height,
+                                        bool resize_pending,
+                                        float scale_factor) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  if (!guest)
+    return;
+  WebContentsImpl* guest_web_contents =
+      static_cast<WebContentsImpl*>(guest->web_contents());
+  guest->SetDamageBuffer(damage_buffer, gfx::Size(width, height), scale_factor);
+  if (!resize_pending)
+    guest_web_contents->GetView()->SizeContents(gfx::Size(width, height));
+}
+
+void BrowserPluginEmbedder::SetFocus(int instance_id,
+                                     bool focused) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  if (guest)
+    guest->SetFocus(focused);
+}
+
+void BrowserPluginEmbedder::DestroyGuests() {
+  STLDeleteContainerPairFirstPointers(guest_web_contents_container_.begin(),
+                                      guest_web_contents_container_.end());

[awong, 2012/09/09 18:08:09]

Does this need to call guest->Destroy()?

[lazyboy, 2012/09/10 16:30:17]

guest->Destroy() would actually do the same thing, delete their web_contents().
Maybe
foreach(a in guests_by_instance_id_)
  a->Destroy();
guest_web_contents_container_.clear(), guests_by_instance_id_.clear();

is better implementation for this method?

+  guest_web_contents_container_.clear();
+  guests_by_instance_id_.clear();
+}
+
+void BrowserPluginEmbedder::HandleInputEvent(int instance_id,
+                                             RenderViewHost* render_view_host,
+                                             const gfx::Rect& guest_rect,
+                                             const WebKit::WebInputEvent& event,
+                                             IPC::Message* reply_message) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  if (guest)
+    guest->HandleInputEvent(render_view_host, guest_rect, event, reply_message);
+}
+
+void BrowserPluginEmbedder::DestroyGuestByInstanceID(int instance_id) {
+  BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
+  if (guest) {
+    WebContents* guest_web_contents = guest->web_contents();
+
+    GuestWebContentsMap::iterator guest_it = guest_web_contents_container_.find(
+        guest_web_contents);
+    DCHECK(guest_it != guest_web_contents_container_.end());
+
+    guest_web_contents_container_.erase(guest_it);
+    guests_by_instance_id_.erase(instance_id);
+
+    guest->Destroy();

[awong, 2012/09/09 18:08:09]

Do we need to delete guest here?

Maybe we should have a map of linked_ptrs?

[lazyboy, 2012/09/10 16:30:17]

guest->Destroy() would delete the web_contents() associated with the guest,
since the guest is a WCObserver, it would delete the guest as well.

[awong, 2012/09/10 19:21:07]

Ah, I see now.  It's weird to have 2 mechanisms for managing destruction of the
same set of pointers.  Makes tracking ownership hard. Also, in the future, if
someone wanted to add some behavior on destruction of a guest, there's a higher
chance that you'd 

AFAICT, this is only place that class guest->Destroy().  Maybe we should
completely remove the Destroy() method and just delete guest_web_contents
manually?

[lazyboy, 2012/09/10 23:40:57]

Agreed, I actually thought of destroying the guests in a consistent manner, but
forgot. (Although I was thinking the otherway, always calling Destroy())
Done.

+  }
+}
+
+void BrowserPluginEmbedder::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.
+  //
+  // Subframe's guest(s) clean up is not handled here. They are handled by
+  // either the lifetime of WebContents associated directly with the guest(s) or
+  // by the lifetime of their embedders. Consider the following cases:
+  //
+  // Case 1: (Subframe's guest clean up managed by embedder) A WebContents WC1
+  // embeds a guest (therefore it is an embedder E1), call this guest G1. G1
+  // points to a page that has an iframe. Call the iframe's WebContents WC2. Now
+  // the iframe has a guest of its own in it, lets call it G2, which makes WC2
+  // an embedder as well, call it E2. Now when
+  // DidCommitProvisionalLoadForFrame() is called, G2 won't be cleaned up by the
+  // code below since it doesn't directly belong to the top level frame. However
+  // when the WebContents WC2 gets destroyed, it destroys the embedder E2
+  // associated with it, eventually destroying all of its guests (G2).
+  //
+  // Case 2: (Subframe's guest cleanup managed by WebContents) A page has an
+  // iframe which points to a page that has a guest G1. Therefore this makes top
+  // level page's WebContents WC1 an embedder E1. Lets say the WebContents
+  // associated with G1 is WC2. When DidCommitProvisionalLoadForFrame() occurs,
+  // G1 is not a guest within the top level frame that called commit, so it
+  // won't be cleaned up with the code below; However since WC2 goes away, this
+  // eventually destroys G1 since G1 is an observer for WC2.

[awong, 2012/09/09 18:08:09]

Add a line explaining that the following code is repsonsible for deleting the
top-level frame which will eventually invoke one of the two cases below covering
all guests in the whole frame tree.

[lazyboy, 2012/09/10 16:30:17]

Done.

+  typedef std::set<WebContents*> GuestSet;
+  GuestSet guests_to_delete;
+  for (GuestWebContentsMap::const_iterator it =
+      guest_web_contents_container_.begin();

[awong, 2012/09/09 18:08:09]

indentation. off by 5 spaces.

[lazyboy, 2012/09/10 16:30:17]

Ah, hit this case, what happens in this case when the expressions in for() is
long (I couldn't seem to find it in style guide):

for (my_long_var ... =
    this_is_continuation_4_space_indentation;
     second_expression_regular_(_indent; third...)
or just indent everything 4 spaces or 5 spaces?

[awong, 2012/09/10 19:21:07]

Yeah, it's confusing.  My understanding of the style guide is it should be:

 for (my_long_var ... =
          this_is_continuation_4_space_indentation;
      second_expression_regular_(_indent; third...)

Since, without a line break, the next line is supposed to line up with the "m"
in "my_long_var", when you do the line continuation you are supposed to indent 4
more spaces from "where you should have been."

BTW, if the style guide doesn't state anything, the rule is to look at the
codebase for the prevailing pattern.  Ofcourse, having just written that, I went
and tried searching for

   for\ \(.*\=$ lang:^c++$

on

   http://code.google.com/p/chromium/source/search

and consistency on this is...lacking.  Google3 is a little bit more consistent
so I would indent 4 more spaces.

[lazyboy, 2012/09/10 23:40:57]

Somehow I was always thinking of indenting 4 sp from the beginning of the line
:S.
Done.

+       it != guest_web_contents_container_.end(); ++it) {
+    guests_to_delete.insert(it->first);
+  }
+  for (GuestSet::const_iterator it = guests_to_delete.begin();
+       it != guests_to_delete.end(); ++it) {
+    int instance_id = static_cast<WebContentsImpl*>(*it)->
+        GetBrowserPluginGuest()->instance_id();
+    DestroyGuestByInstanceID(instance_id);
+  }

[awong, 2012/09/09 18:08:09]

Shouldn't this just call DestroyGuests()?

Also, this code is nlogn, though I'm guessing we don't have enough guests to
care.

[lazyboy, 2012/09/10 16:30:17]

This got broken between patch #12 and #13 it seems, fixed, guests_to_delete_
does not contain all the ids in this embedder, only the ones belonging to
|frame_id|

+}
+
+void BrowserPluginEmbedder::RenderViewDeleted(
+    RenderViewHost* render_view_host) {
+  DestroyGuests();
+}
+
+void BrowserPluginEmbedder::RenderViewGone(base::TerminationStatus status) {
+  DestroyGuests();
+}
+
+void BrowserPluginEmbedder::WebContentsVisibilityChanged(bool visible) {
+  // If the embedder is hidden we need to hide the guests as well.
+  for (GuestWebContentsMap::const_iterator it =
+       guest_web_contents_container_.begin();

[awong, 2012/09/09 18:08:09]

indentation

[lazyboy, 2012/09/10 16:30:17]

See question above, thanks.

+       it != guest_web_contents_container_.end(); ++it) {
+    WebContents* web_contents = it->first;
+    if (visible)
+      web_contents->WasShown();
+    else
+      web_contents->WasHidden();
+  }
+}
+
+void BrowserPluginEmbedder::PluginDestroyed(int instance_id) {
+  DestroyGuestByInstanceID(instance_id);
+}
+
+void BrowserPluginEmbedder::Observe(int type,
+                                    const NotificationSource& source,
+                                    const NotificationDetails& details) {
+  switch (type) {
+    case NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED: {
+      bool visible = *Details<bool>(details).ptr();
+      WebContentsVisibilityChanged(visible);
+      break;
+    }
+    default:
+      NOTREACHED() << "Unexpected notification type: " << type;
+  }
+}
+
+}  // namespace content