Browser Plugin: Simplify BrowserPluginGuestHelper

Browser Plugin: Simplify BrowserPluginGuestHelper

Since BrowserPlugin was first implemented, support for cross-process navigation
was dropped but BrowserPlugin*Helper classes maintain a lot of complexity that
was no longer necessary. This CL attempts to reduce some of this complexity
by simplifying BrowserPluginGuestHelper. It is only needed now to intercept
ViewHostMsg_* before arriving to RenderViewHost. It is still a separate class
because RenderViewHostObserver's lifetime is managed by RenderViewHost and
BrowserPluginGuest's lifetime is managed by WebContentsImpl.

This refactor/simplification is a prerequisite to introducing a
BrowserPluginGuestObserver class. Further changes will come in a subsequent
CL to simplify the message routing from

BrowserPluginEmbedderHelper --> BrowserPluginEmbedder --> BrowserPluginGuest

BUG=166165
Test=BrowserPluginHostTest.*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173804

[Charlie Reis, 2012-12-17 22:40:25]

LGTM.  Does it make sense to mention that cross-process navigations aren't
supported in the class-level comment of BrowserPluginGuest (or somewhere
similar)?

[commit-bot: I haz the power, 2012-12-18 02:54:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11606005/7001

[commit-bot: I haz the power, 2012-12-18 02:54:41]

Presubmit check for 11606005-7001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/common/browser_plugin_messages.h

Presubmit checks took 1.5s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[Fady Samuel, 2012-12-18 03:11:14]

Hi Cris,

Could you please review browser_plugin_messages.h

Thanks,

Fady

[Cris Neckar, 2012-12-18 19:26:34]

new code lgtm.. working with Fady on pre-existing issues

[commit-bot: I haz the power, 2012-12-18 19:27:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/11606005/7001

[commit-bot: I haz the power, 2012-12-18 22:44:44]

Failed to apply patch for
content/browser/browser_plugin/browser_plugin_guest.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/browser_plugin/browser_plugin_guest.cc
  Hunk #6 FAILED at 348.
  Hunk #7 succeeded at 446 (offset -10 lines).
  Hunk #8 succeeded at 464 (offset -10 lines).
  Hunk #9 succeeded at 498 (offset -10 lines).
  Hunk #10 succeeded at 562 (offset -10 lines).
  Hunk #11 succeeded at 577 (offset -10 lines).
  1 out of 11 hunks FAILED -- saving rejects to file
content/browser/browser_plugin/browser_plugin_guest.cc.rej

Patch:       content/browser/browser_plugin/browser_plugin_guest.cc
Index: content/browser/browser_plugin/browser_plugin_guest.cc
diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc
b/content/browser/browser_plugin/browser_plugin_guest.cc
index
c8f511642d9e3f3476b553a76a6d3c7ee2ad843b..0a66b07c95d386ffd6af42028898e5b6dd76c6bc
100644
--- a/content/browser/browser_plugin/browser_plugin_guest.cc
+++ b/content/browser/browser_plugin/browser_plugin_guest.cc
@@ -15,6 +15,7 @@
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/browser_plugin_messages.h"
 #include "content/common/content_constants_internal.h"
+#include "content/common/drag_messages.h"
 #include "content/common/view_messages.h"
 #include "content/port/browser/render_view_host_delegate_view.h"
 #include "content/public/browser/notification_service.h"
@@ -53,7 +54,6 @@ BrowserPluginGuest::BrowserPluginGuest(
       remote_damage_buffer_handle_(0),
 #endif
       damage_buffer_scale_factor_(1.0f),
-      pending_update_counter_(0),
       guest_hang_timeout_(
           base::TimeDelta::FromMilliseconds(kHungRendererDelayMs)),
       focused_(params.focused),
@@ -112,14 +112,6 @@ void BrowserPluginGuest::Observe(int type,
   }
 }
 
-bool BrowserPluginGuest::ViewTakeFocus(bool reverse) {
-  SendMessageToEmbedder(
-      new BrowserPluginMsg_AdvanceFocus(embedder_routing_id(),
-                                        instance_id(),
-                                        reverse));
-  return true;
-}
-
 void BrowserPluginGuest::Go(int relative_index) {
   web_contents()->GetController().GoToOffset(relative_index);
 }
@@ -173,13 +165,6 @@ bool BrowserPluginGuest::ShouldFocusPageAfterCrash() {
   return false;
 }
 
-void BrowserPluginGuest::SetIsAcceptingTouchEvents(bool accept) {
-  SendMessageToEmbedder(
-      new BrowserPluginMsg_ShouldAcceptTouchEvents(embedder_routing_id(),
-                                                   instance_id(),
-                                                   accept));
-}
-
 void BrowserPluginGuest::SetVisibility(bool embedder_visible, bool visible) {
   visible_ = visible;
   if (embedder_visible && visible)
@@ -242,17 +227,6 @@ void BrowserPluginGuest::SetSize(
   Resize(embedder_web_contents_->GetRenderViewHost(), resize_guest_params);
 }
 
-void BrowserPluginGuest::UpdateDragCursor(WebKit::WebDragOperation operation) {
-  RenderViewHostImpl* embedder_render_view_host =
-      static_cast<RenderViewHostImpl*>(
-          embedder_web_contents_->GetRenderViewHost());
-  CHECK(embedder_render_view_host);
-  RenderViewHostDelegateView* view =
-      embedder_render_view_host->GetDelegate()->GetDelegateView();
-  if (view)
-    view->UpdateDragCursor(operation);
-}
-
 WebContents* BrowserPluginGuest::GetWebContents() {
   return web_contents();
 }
@@ -374,85 +348,12 @@ bool BrowserPluginGuest::InAutoSizeBounds(const gfx::Size&
size) const {
       size.height() <= max_auto_size_.height();
 }
 
-void BrowserPluginGuest::UpdateRect(
-    RenderViewHost* render_view_host,
-    const ViewHostMsg_UpdateRect_Params& params) {
-  // This handler is only of interest to us for the 2D software rendering path.
-  // needs_ack should always be true for the 2D path.
-  // TODO(fsamuel): Do we need to do something different in the 3D case?
-  if (!params.needs_ack)
-    return;
-
-  // Only copy damage if the guest is in autosize mode and the guest's view
size
-  // is less than the maximum size or the guest's view size is equal to the
-  // damage buffer's size and the guest's scale factor is equal to the damage
-  // buffer's scale factor.
-  // The scaling change can happen due to asynchronous updates of the DPI on a
-  // resolution change.
-  if (((auto_size_enabled_ && InAutoSizeBounds(params.view_size)) ||
-      (params.view_size.width() == damage_view_size().width() &&
-       params.view_size.height() == damage_view_size().height())) &&
-       params.scale_factor == damage_buffer_scale_factor()) {
-    TransportDIB* dib = render_view_host->GetProcess()->
-        GetTransportDIB(params.bitmap);
-    if (dib) {
-#if defined(OS_WIN)
-      size_t guest_damage_buffer_size = params.bitmap_rect.width() *
-                                        params.bitmap_rect.height() * 4;
-      size_t embedder_damage_buffer_size = damage_buffer_size_;
-#else
-      size_t guest_damage_buffer_size = dib->size();
-      size_t embedder_damage_buffer_size = damage_buffer_->size();
-#endif
-      void* guest_memory = dib->memory();
-      void* embedder_memory = damage_buffer_->memory();
-      size_t size = std::min(guest_damage_buffer_size,
-                             embedder_damage_buffer_size);
-      memcpy(embedder_memory, guest_memory, size);
-    }
-  }
-  BrowserPluginMsg_UpdateRect_Params relay_params;
-#if defined(OS_MACOSX)
-  relay_params.damage_buffer_identifier = damage_buffer_->id();
-#elif defined(OS_WIN)
-  // On Windows, the handle used locally differs from the handle received from
-  // the embedder process, since we duplicate the remote handle.
-  relay_params.damage_buffer_identifier = remote_damage_buffer_handle_;
-#else
-  relay_params.damage_buffer_identifier = damage_buffer_->handle();
-#endif
-  relay_params.bitmap_rect = params.bitmap_rect;
-  relay_params.scroll_delta = params.scroll_delta;
-  relay_params.scroll_rect = params.scroll_rect;
-  relay_params.copy_rects = params.copy_rects;
-  relay_params.view_size = params.view_size;
-  relay_params.scale_factor = params.scale_factor;
-  relay_params.is_resize_ack = ViewHostMsg_UpdateRect_Flags::is_resize_ack(
-      params.flags);
-
-  // We need to send the ACK to the same render_view_host that issued
-  // the UpdateRect. We keep track of this correspondence via a message_id.
-  int message_id = pending_update_counter_++;
-  pending_updates_.AddWithID(render_view_host, message_id);
-
-  SendMessageToEmbedder(new BrowserPluginMsg_UpdateRect(embedder_routing_id(),
-                                                        instance_id(),
-                                                        message_id,
-                                                        relay_params));
-}
-
 void BrowserPluginGuest::UpdateRectACK(
-    int message_id,
     const BrowserPluginHostMsg_AutoSize_Params& auto_size_params,
     const BrowserPluginHostMsg_ResizeGuest_Params& resize_guest_params) {
-  RenderViewHost* render_view_host = pending_updates_.Lookup(message_id);
-  // If the guest has crashed since it sent the initial ViewHostMsg_UpdateRect
-  // then the pending_updates_ map will have been cleared.
-  if (render_view_host) {
-    pending_updates_.Remove(message_id);
-    render_view_host->Send(
-        new ViewMsg_UpdateRect_ACK(render_view_host->GetRoutingID()));
-  }
+  RenderViewHost* render_view_host = web_contents()->GetRenderViewHost();
+  render_view_host->Send(
+      new ViewMsg_UpdateRect_ACK(render_view_host->GetRoutingID()));
   SetSize(auto_size_params, resize_guest_params);
 }
 
@@ -482,13 +383,6 @@ void BrowserPluginGuest::HandleInputEvent(RenderViewHost*
render_view_host,
   guest_rvh->StartHangMonitorTimeout(guest_hang_timeout_);
 }
 
-void BrowserPluginGuest::HandleInputEventAck(RenderViewHost* render_view_host,
-                                             bool handled) {
-  RenderViewHostImpl* guest_rvh =
-      static_cast<RenderViewHostImpl*>(render_view_host);
-  guest_rvh->StopHangMonitorTimeout();
-}
-
 void BrowserPluginGuest::Stop() {
   web_contents()->Stop();
 }
@@ -507,38 +401,6 @@ void BrowserPluginGuest::SetFocus(bool focused) {
   Send(new ViewMsg_SetFocus(routing_id(), focused));
 }
 
-void BrowserPluginGuest::ShowWidget(RenderViewHost* render_view_host,
-                                    int route_id,
-                                    const gfx::Rect& initial_pos) {
-  gfx::Rect screen_pos(initial_pos);
-  screen_pos.Offset(guest_screen_rect_.OffsetFromOrigin());
-  static_cast<WebContentsImpl*>(web_contents())->ShowCreatedWidget(route_id,
-                                                                   screen_pos);
-}
-
-#if defined(OS_MACOSX)
-void BrowserPluginGuest::ShowPopup(RenderViewHost* render_view_host,
-                                   const ViewHostMsg_ShowPopup_Params& params)
{
-  gfx::Rect translated_bounds(params.bounds);
-  translated_bounds.Offset(guest_window_rect_.OffsetFromOrigin());
-  BrowserPluginPopupMenuHelper popup_menu_helper(
-      embedder_web_contents_->GetRenderViewHost(), render_view_host);
-  popup_menu_helper.ShowPopupMenu(translated_bounds,
-                                  params.item_height,
-                                  params.item_font_size,
-                                  params.selected_item,
-                                  params.popup_items,
-                                  params.right_aligned,
-                                  params.allow_multiple_selection);
-}
-#endif
-
-void BrowserPluginGuest::SetCursor(const WebCursor& cursor) {
-  SendMessageToEmbedder(new BrowserPluginMsg_SetCursor(embedder_routing_id(),
-                                                       instance_id(),
-                                                       cursor));
-}
-
 void BrowserPluginGuest::DidStartProvisionalLoadForFrame(
     int64 frame_id,
     int64 parent_frame_id,
@@ -573,6 +435,10 @@ void BrowserPluginGuest::DidFai…
(message too large)