Browser Plugin: New Implementation (Browser Side)

content/browser/browser_plugin/browser_plugin_guest_helper.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_GUEST_HELPER_H_
+#define CONTENT_BROWSER_BROWSER_PLUGIN_BROWSER_PLUGIN_GUEST_HELPER_H_
+
+#include <string>
+
+#include "base/compiler_specific.h"
+#include "content/browser/browser_plugin/browser_plugin_guest.h"
+#include "content/public/browser/render_view_host_observer.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "ipc/ipc_channel_handle.h"
+#include "ipc/ipc_sync_message.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
+#include "ui/surface/transport_dib.h"
+#include "webkit/glue/webcursor.h"
+
+namespace gfx {
+class Size;
+}
+
+struct BrowserPluginHostMsg_ResizeGuest_Params;
+struct ViewHostMsg_UpdateRect_Params;
+
+namespace content {
+
+class RenderViewHost;
+
+// Helper for browser plugin guest.
+//
+// It overrides different WebContents messages that require special treatment
+// for a WebContents to act as a guest. All functionality is handled by its
+// delegate. This class exists so we have separation of messages requiring
+// special handling, which can be move to a message filter (IPC thread) for

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

How likely is this future optimization? If we don't have concrete plans to
actually do this soon, speculatively adding such a class just adds more
maintenance burden cause you have to handle more delegates.

If we keep this class, add a comment about the expected lifetime of this object
(who owns it, does how we know guest outlives it, etc.)

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

This optimization is something we would like to have soon I believe, specially
for input events since we have unnecessary thread hops.

Added comment about expected lifetime.

+// future optimization.
+class BrowserPluginGuestHelper : public RenderViewHostObserver {
+ public:
+  BrowserPluginGuestHelper(BrowserPluginGuest* guest,
+                           RenderViewHost* render_view_host);
+  virtual ~BrowserPluginGuestHelper();
+
+ private:
+  // Message handlers
+  void OnUpdateRect(const ViewHostMsg_UpdateRect_Params& params);
+  void OnHandleInputEventAck(WebKit::WebInputEvent::Type event_type,
+    bool processed);
+  void OnTakeFocus(bool reverse);
+  void OnShowWidget(int route_id, const gfx::Rect& initial_pos);
+  void OnSetCursor(const WebCursor& cursor);
+
+  // RenderViewHostObserver implementation.
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;

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

Please go through all the classes and move overrides out of private.

The reason I'm so picky about this is cause you're actually really protecting
access to the method.  People modifying it in the future may incorrectly assume
that since it's private, it's only usable by this class which is wrong. 
Example:

BrowserPluginGuest* r = new  BrowserPluginGuestHelper();
r->OnMessageReceved();  // compile failure.
static_cast<RenderViewHostObserver*>(r)->OnMessageReceved(); // works on
upcast??

this is just odd.

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

Fixed.

+
+  BrowserPluginGuest* guest_;
+  // A scoped container for notification registries.
+  NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginGuestHelper);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_BROWSER_PLUGIN_BROWSER_PLUGIN_GUEST_HELPER_H_