Browser Plugin: New Implementation (Browser Side)

content/browser/browser_plugin/browser_plugin_host.h

+// 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.
+
+#ifndef CONTENT_BROWSER_BROWSER_PLUGIN_BROWSER_PLUGIN_HOST_H__
+#define CONTENT_BROWSER_BROWSER_PLUGIN_BROWSER_PLUGIN_HOST_H__
+
+#include <map>
+
+#include "base/compiler_specific.h"
+#include "base/id_map.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/web_contents_delegate.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "ppapi/c/pp_instance.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
+#include "ui/gfx/rect.h"
+#include "ui/gfx/size.h"
+#include "ui/surface/transport_dib.h"
+#include "webkit/glue/webcursor.h"
+
+class WebContentsImpl;
+struct BrowserPluginHostMsg_PostMessage_Params;
+struct ViewHostMsg_UpdateRect_Params;
+
+namespace IPC {
+struct ChannelHandle;
+}
+
+namespace content {
+
+class BrowserPluginHost;
+class RenderProcessHost;
+class RenderWidgetHostView;
+
+typedef std::map<WebContents*, int64> GuestMap;
+typedef std::map<int, BrowserPluginHost*> ContainerInstanceMap;
+
+// A BrowserPluginHost object is used by both embedders and guests.

[Charlie Reis, 2012/08/13 17:50:14]

I still think having the BrowserPluginHost act as both embedder and guest makes
this code hard to follow.  First, it's not clear which parts of the API are for
which role.  Second, there's a 1:1 relationship between BrowserPlugin and
BrowserPluginHost for guests (like most Foo/FooHost classes), but it sounds like
a single embedder is supposed to manage multiple guests.

Is there a clean way to split this?  Or at least very clearly delineate which
parts of the class are for which role?

I still don't understand why it's useful for BrowserPluginHost to serve both
roles (without apparently knowing which role it is in).  This makes its API
harder to understand.

+// The primary purpose of BrowserPluginHost is to allow an embedder
+// to manage the lifetime of its guests. It cleans up its guests
+// on navigation, crashes, and "hides" guests when it hides.
+// For the guest, BrowserPluginHost keeps track of its embedder,
+// and its BrowserPlugin instance ID.
+class BrowserPluginHost : public WebContentsObserver,
+                          public NotificationObserver,
+                          public WebContentsDelegate {
+ public:
+  // BrowserPluginHost is owned by a WebContentsImpl. Here it takes in its
+  // owner. The owner can be either a guest or embedder WebContents.
+  explicit BrowserPluginHost(WebContentsImpl* web_contents);
+
+  virtual ~BrowserPluginHost();
+
+  // This is called on navigation of the browser plugin. On the first
+  // navigation, it creates a new WebContentsImpl for the guest, associates
+  // it with its embedder, and navigates it to the src URL. On subsequent
+  // navigations, it calls up the guest's existign WebContents and navigates
+  // it.
+  void NavigateOrCreateGuest(RenderViewHost* render_view_host,

[Charlie Reis, 2012/08/13 17:50:14]

I had asked this to be renamed to NavigateGuest in
https://chromiumcodereview.appspot.com/10830072/diff/102/content/common/brows....
 The comments make it clear that it creates the guest if necessary.

[Fady Samuel, 2012/08/13 21:30:08]

Ohh sorry, it looks like I missed addressing that particular comment last time.
Sorry about that. Fixing now.

+                             int instance_id,
+                             long long frame_id,
+                             const std::string& src,
+                             const gfx::Size& size);
+
+  // This is called when the browser plugin's container has resized in the
+  // embedder. In additon to specifying the new size, and new scale factor,
+  // it also passes a TransportDIB that is shared with the embedder.
+  // The browser process is free to write to this buffer whenever it receives
+  // damage from the guest because if it receives damage from the guest,
+  // then the previous damage has already been accessed and ACK'ed by the
+  // embedder.
+  void ResizeGuest(int instance_id,
+                   TransportDIB* damage_buffer,
+                   int width,
+                   int height,
+                   bool resize_pending,
+                   float scale_factor);
+
+  void UpdateRect(RenderViewHost* render_view_host,
+                  const ViewHostMsg_UpdateRect_Params& params);
+
+  // Called on the guest BrowserPluginHost.
+  void UpdateRectACK(int message_id, const gfx::Size& size);
+  void HandleInputEvent(RenderViewHost* render_view_host,
+                        const gfx::Rect& guest_rect,
+                        const WebKit::WebInputEvent& event,
+                        IPC::Message* reply_message);
+  void HandleInputEventAck(RenderViewHost* render_view_host, bool handled);
+  void SetFocus(bool focused);
+  void ShowWidget(RenderViewHost* render_view_host,
+                  int route_id,
+                  const gfx::Rect& initial_pos);
+  void SetCursor(const WebCursor& cursor);
+
+  RenderProcessHost* embedder_render_process_host() const {
+    return embedder_render_process_host_;

[Charlie Reis, 2012/08/13 17:50:14]

Why do we need to cache this?  Is it present for both embedders and guests?

[Fady Samuel, 2012/08/13 21:30:08]

This is used to talk back to the embedder. 

+  }
+  int instance_id() const { return instance_id_; }

[Charlie Reis, 2012/08/13 17:50:14]

This is important enough that it needs a comment indicating what it represents.

+  void SetDamageBuffer(TransportDIB* damage_buffer,
+                       const gfx::Size& size,
+                       float scale_factor);
+  TransportDIB* damage_buffer() const { return damage_buffer_; }
+  const gfx::Size& damage_buffer_size() const { return damage_buffer_size_; }
+  float damage_buffer_scale_factor() const {
+      return damage_buffer_scale_factor_;
+  }
+
+  const ContainerInstanceMap& guests_for_testing() const {
+    return guests_by_instance_id_;
+  }
+
+ protected:
+  friend class BrowserPluginHostHelper;
+  friend class BrowserPluginHostTest;
+
+  // Get a guest BrowserPluginHost by its container ID specified
+  // in BrowserPlugin.
+  BrowserPluginHost* GetGuestByInstanceID(int container_id) const;

[Charlie Reis, 2012/08/13 17:50:14]

Why is it called ByInstanceID if you're passing in a container ID?  How do those
differ?

[Fady Samuel, 2012/08/13 21:30:08]

I used to call this container_id then I renamed it to instance_id. I fixed this.

+
+  // An embedder BrowserPluginHost keeps track of
+  // its guests so that if it navigates away, its associated RenderView
+  // crashes or it is hidden, it takes appropriate action on the guest.
+  void AddGuest(int instance_id, BrowserPluginHost* guest, int64 frame_id);
+
+  void set_embedder_render_process_host(
+      RenderProcessHost* embedder_render_process_host) {
+    embedder_render_process_host_ = embedder_render_process_host;
+  }
+  void set_instance_id(int instance_id) { instance_id_ = instance_id; }

[Charlie Reis, 2012/08/13 17:50:14]

Can this change over time?  I would recommend setting it on construction rather
than having a period of time where you might not have one.

+
+  void DestroyGuests();
+  // Destroy a specific guest if possible. This is a no-op if the guest id is
+  // unknown as we may already have removed it for a different reason.
+  void DestroyGuestByInstanceID(int instance_id);
+
+  // WebContentsDelegate implementation.
+  virtual bool TakeFocus(bool reverse) OVERRIDE;
+  virtual void RendererUnresponsive(WebContents* source) OVERRIDE;
+
+  // WebContentsObserver implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+  // Used to monitor frame navigation to cleanup guests when a frame navigates
+  // away from the browser plugin it's hosting.
+  virtual void DidCommitProvisionalLoadForFrame(
+      int64 frame_id,
+      bool is_main_frame,
+      const GURL& url,
+      PageTransition transition_type,
+      RenderViewHost* render_view_host) OVERRIDE;
+  virtual void RenderViewDeleted(RenderViewHost* render_view_host) OVERRIDE;
+  virtual void RenderViewGone(base::TerminationStatus status) OVERRIDE;
+
+  // NotificationObserver method override.
+  virtual void Observe(int type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) OVERRIDE;
+
+  // A scoped container for notification registries.
+  NotificationRegistrar registrar_;
+  RenderProcessHost* embedder_render_process_host_;
+  std::string embedder_channel_name_;
+  // An identifier that uniquely identifies a browser plugin container
+  // within an embedder.
+  int instance_id_;
+  TransportDIB* damage_buffer_;
+  gfx::Size damage_buffer_size_;
+  float damage_buffer_scale_factor_;
+  GuestMap guests_;
+  ContainerInstanceMap guests_by_instance_id_;
+
+  IDMap<RenderViewHost> pending_updates_;
+  int pending_update_counter_;
+  scoped_ptr<IPC::Message> pending_input_event_reply_;
+  gfx::Rect guest_rect_;
+  WebCursor cursor_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginHost);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_BROWSER_PLUGIN_OLD_BROWSER_PLUGIN_HOST_H_