Browser Plugin: New Implementation (Browser Side)

content/browser/browser_plugin/browser_plugin_guest.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_guest.h"
+
+#include "base/time.h"
+#include "content/browser/browser_plugin/browser_plugin_guest_helper.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/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/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/web_contents_observer.h"
+#include "content/public/common/result_codes.h"
+#include "ui/gfx/size.h"
+
+namespace content {
+
+namespace {
+const int kGuestHangTimeout = 5000;
+}
+
+BrowserPluginGuest::BrowserPluginGuest(int instance_id,
+                                       WebContentsImpl* web_contents,
+                                       RenderViewHost* render_view_host)
+                                       : WebContentsObserver(web_contents),
+                                         embedder_render_process_host_(NULL),
+                                         instance_id_(instance_id),
+                                         damage_buffer_(NULL),
+                                         pending_update_counter_(0) {
+  new BrowserPluginGuestHelper(this, render_view_host);

[Fady Samuel, 2012/08/25 05:28:32]

The lifetime of the helper is not equal to the lifetime of the
BrowserPluginGuest. The helper should be created on
NOTIFICATION_WEB_CONTENTS_RENDER_VIEW_HOST_CREATED as before, see
http://codereview.chromium.org/10560022/patch/59102/53234

We need to create a new helper every time we get a new RenderViewHost
(cross-process navigation for example).

[lazyboy, 2012/08/25 05:57:09]

Ah, that's why it was like that, I see.
Now the problem is, at the time BrowserPluginGuest gets created, we would/might
already have the render_view_host created (since we don't create
BrowserPluginGuest at the very beginning of WebContentsImpl anymore) and we
might not get such notification (at least the first RVH creation one) at all?

[lazyboy, 2012/08/27 15:42:51]

So my assumption was:
embedder loads guest a.com, creates guest, gweb_contents, g_RVH
embedder navigates the same guest to b.com, modifies existing gweb_contents to
LoadURL b.com, *this would create a new RVH other than g_RVH?

I don't see any notification saying RENDER_VIEW_HOST_CREATED in this case (both
in this code and your old patch)? When does a web_contents create (another) new
RVH?

[Charlie Reis, 2012/08/27 20:23:59]

As I mentioned separately, we don't yet do any cross-process swaps for
navigations in guests.  It might happen in the future, but I'm more concerned
about this on the embedder side than the guest side for now.

[lazyboy, 2012/08/28 19:07:14]

When we switch to new RVH (for embedder case), would we not get a RenderViewGone
call in BrowserPluginEmbedder? Right now we DestroyGuests() from there, but
maybe we can just remove the whole embedder instance from WebContents, that way
next time we get a browser_plugin from renderer process, we would create a new
one (with correct RVH).

[Charlie Reis, 2012/08/29 00:11:04]

Sounds like you're talking about the embedder here, so let's move that
discussion to BrowserPluginEmbedder.

For BrowserPluginGuest, we'll need a CHECK or DCHECK somewhere to ensure that we
don't swap RVHs.  Maybe that's something like listening for
NOTIFICATION_WEB_CONTENTS_SWAPPED?

+}
+
+BrowserPluginGuest::~BrowserPluginGuest() {
+  if (damage_buffer_)
+    delete damage_buffer_;
+}
+
+bool BrowserPluginGuest::TakeFocus(bool reverse) {
+  embedder_render_process_host()->Send(
+      new BrowserPluginMsg_AdvanceFocus(instance_id(), reverse));
+  return true;
+}
+
+void BrowserPluginGuest::RendererUnresponsive(WebContents* source) {
+  base::ProcessHandle process_handle =
+      web_contents()->GetRenderProcessHost()->GetHandle();
+  base::KillProcess(process_handle, RESULT_CODE_HUNG, false);

[Charlie Reis, 2012/08/27 20:23:59]

I don't think we've resolved this yet.  I understand that we don't want the
guest to block the embedder, but automatically killing the guest if it goes
unresponsive for a bit isn't a good long term plan.

Darin suggested treating the guest as a black hole for input events so that we
don't block on it, or we could let them bubble up asynchronously (at the risk of
some odd behavior).

Are either of those feasible?  At the very least, we need a TODO and a heads up
to the stability team that this could cause kills, but I'd like to avoid having
the kill here if possible.

[lazyboy, 2012/08/28 19:07:14]

I talked to rjkroege@ abt this, it seems:
a. would make a page's event handling behave significantly differently if it is
an embedder.
b. it's very hard to make them async (webkit related).
Rob will comment more.

Also which part of killing is the concern (to educate myself): would it report
more kills from chrome in general and reduce some stability score of chrome?

+}
+
+void BrowserPluginGuest::SetDamageBuffer(
+    TransportDIB* damage_buffer, const gfx::Size& size, float scale_factor) {
+  if (damage_buffer_)
+    delete damage_buffer_;
+  damage_buffer_ = damage_buffer;
+  damage_buffer_size_ = size;
+  damage_buffer_scale_factor_ = scale_factor;
+}
+
+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'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.
+  if (params.view_size.width() == damage_buffer_size().width() &&
+      params.view_size.height() == damage_buffer_size().height() &&
+      params.scale_factor == damage_buffer_scale_factor()) {
+    TransportDIB* dib = render_view_host->GetProcess()->
+        GetTransportDIB(params.bitmap);
+    if (dib) {
+      void* guest_memory = dib->memory();
+      void* embedder_memory = damage_buffer_->memory();
+      int size = std::min(dib->size(), damage_buffer_->size());
+      memcpy(embedder_memory, guest_memory, size);
+    }
+  }
+  DCHECK(embedder_render_process_host());
+  BrowserPluginMsg_UpdateRect_Params relay_params;
+  relay_params.bitmap_rect = params.bitmap_rect;
+  relay_params.dx = params.dx;
+  relay_params.dy = params.dy;
+  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);
+
+  gfx::Size param_size = gfx::Size(params.view_size.width(),
+                                   params.view_size.height());
+  content::NotificationService::current()->Notify(
+       // TODO(lazyboy): Rename notification variable if this stays.
+       NOTIFICATION_BROWSER_PLUGIN_UPDATE_RECT,
+       content::Source<BrowserPluginGuest>(this),
+       content::Details<const gfx::Size>(&param_size));
+
+  embedder_render_process_host()->Send(
+      new BrowserPluginMsg_UpdateRect(instance_id(),
+                                      message_id,
+                                      relay_params));
+}
+
+void BrowserPluginGuest::UpdateRectACK(int message_id, const gfx::Size& size) {
+  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)
+    return;
+  pending_updates_.Remove(message_id);
+  render_view_host->Send(
+      new ViewMsg_UpdateRect_ACK(render_view_host->GetRoutingID()));
+  if (!size.IsEmpty())
+    render_view_host->GetView()->SetSize(size);
+}
+
+void BrowserPluginGuest::HandleInputEvent(RenderViewHost* render_view_host,
+                                          const gfx::Rect& guest_rect,
+                                          const WebKit::WebInputEvent& event,
+                                          IPC::Message* reply_message) {
+  guest_rect_ = guest_rect;
+  RenderViewHostImpl* guest_rvh = static_cast<RenderViewHostImpl*>(
+      web_contents()->GetRenderViewHost());
+  IPC::Message* message = new ViewMsg_HandleInputEvent(
+      guest_rvh->GetRoutingID());
+
+  // Copy the WebInputEvent and modify the event type. The guest expects
+  // WebInputEvent::RawKeyDowns and not KeyDowns.
+  scoped_array<char> input_buffer(new char[event.size]);
+  memcpy(input_buffer.get(), &event, event.size);
+  WebKit::WebInputEvent* input_event =
+      reinterpret_cast<WebKit::WebInputEvent*>(input_buffer.get());
+  if (event.type == WebKit::WebInputEvent::KeyDown)
+    input_event->type = WebKit::WebInputEvent::RawKeyDown;
+
+  message->WriteData(input_buffer.get(), event.size);
+  // TODO(fsamuel): What do we need to do here? This is for keyboard shortcuts.
+  if (input_event->type == WebKit::WebInputEvent::RawKeyDown)
+    message->WriteBool(false);
+  guest_rvh->Send(message);
+
+  DCHECK(!pending_input_event_reply_.get());
+  pending_input_event_reply_.reset(reply_message);
+  guest_rvh->StartHangMonitorTimeout(
+      base::TimeDelta::FromMilliseconds(kGuestHangTimeout));
+}
+
+void BrowserPluginGuest::HandleInputEventAck(RenderViewHost* render_view_host,
+                                             bool handled) {
+  DCHECK(pending_input_event_reply_.get());
+  IPC::Message* reply_message = pending_input_event_reply_.release();
+  BrowserPluginHostMsg_HandleInputEvent::WriteReplyParams(reply_message,
+                                                          handled,
+                                                          cursor_);
+  embedder_render_process_host()->Send(reply_message);
+  RenderViewHostImpl* guest_rvh =
+      static_cast<RenderViewHostImpl*>(render_view_host);
+  guest_rvh->StopHangMonitorTimeout();
+}
+
+void BrowserPluginGuest::SetFocus(bool focused) {
+  RenderViewHost* render_view_host = web_contents()->GetRenderViewHost();
+  render_view_host->Send(
+      new ViewMsg_SetFocus(render_view_host->GetRoutingID(), 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_rect_.origin());
+  static_cast<WebContentsImpl*>(web_contents())->ShowCreatedWidget(route_id,
+                                                                   screen_pos);
+}
+
+void BrowserPluginGuest::SetCursor(const WebCursor& cursor) {
+  cursor_ = cursor;
+}
+
+void BrowserPluginGuest::Destroy() {
+  WebContents* guest_web_contents = web_contents();
+  DCHECK(guest_web_contents);
+  delete guest_web_contents;
+}
+
+void BrowserPluginGuest::DidCommitProvisionalLoadForFrame(
+    int64 frame_id,
+    bool is_main_frame,
+    const GURL& url,
+    PageTransition transition_type,
+    RenderViewHost* render_view_host) {
+  // Inform its embedder of the updated URL.
+  // TODO(creis, fsamuel): Ensure this is safe/secure.

[Charlie Reis, 2012/08/27 20:23:59]

What is this referring to?

[lazyboy, 2012/08/28 19:07:14]

I have no context on this one, it's from Fady's CL, Fady can you comment?
Thanks.

[Fady Samuel, 2012/08/29 08:16:20]

At some point Charlie expressed a concern that passing the guest's URL to the
embedder might be unsafe. I'm really not sure. Charlie, if this code looks OK to
you, I'm OK with removing this TODO.

[Charlie Reis, 2012/08/29 18:08:12]

I can't find the context for this.  Now that we assume the embedder has control
over the guest's content, I don't see the risk of providing the guest's URL to
the embedder.  I think we can remove this TODO.

[lazyboy, 2012/08/29 21:24:44]

Removed.

+  DCHECK(embedder_render_process_host());
+  if (is_main_frame) {
+    embedder_render_process_host()->Send(
+        new BrowserPluginMsg_DidNavigate(instance_id(), url));
+  }
+}
+
+void BrowserPluginGuest::RenderViewGone(
+    base::TerminationStatus status) {
+  DCHECK(embedder_render_process_host());
+  if (pending_input_event_reply_.get()) {
+    IPC::Message* reply_message = pending_input_event_reply_.release();
+    BrowserPluginHostMsg_HandleInputEvent::WriteReplyParams(reply_message,
+                                                            false,
+                                                            cursor_);
+    embedder_render_process_host()->Send(reply_message);
+  }
+  embedder_render_process_host()->Send(
+      new BrowserPluginMsg_GuestCrashed(instance_id()));
+  IDMap<RenderViewHost>::const_iterator iter(&pending_updates_);
+  while (!iter.IsAtEnd()) {
+    pending_updates_.Remove(iter.GetCurrentKey());
+    iter.Advance();
+  }
+  content::NotificationService::current()->Notify(
+       // TODO(lazyboy): Rename notification variable if this stays.

[Charlie Reis, 2012/08/27 20:23:59]

nit: Wrong indent.

[lazyboy, 2012/08/28 19:07:14]

Done.

+      NOTIFICATION_BROWSER_PLUGIN_GUEST_CRASHED,
+      content::Source<BrowserPluginGuest>(this),
+      content::NotificationService::NoDetails());
+}
+
+}  // namespace content