OOPIF: Route keyboard events to focused frame in the browser process.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 6b1407b78e0ace939218a24a5478b0c23d48f1c3..765168619c929bd78174d518c2d2c1a9ed0d4653 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1590,6 +1590,18 @@ void WebContentsImpl::ReplicatePageFocus(bool is_focused) {
   frame_tree_.ReplicatePageFocus(is_focused);
 }
 
+RenderWidgetHostImpl* WebContentsImpl::GetFocusedRenderWidgetHost() {

[alexmos, 2015/10/23 23:40:16]

I wasn't sure whether this logic should go into RenderWidgetHostInputEventRouter
instead.  I kept it here for now, because RenderWidgetHostInputEventRouter
seemed more oriented toward mouse events and hittesting, and because it would've
required a frame_host DEPS exception, since it's in renderer_host but would need
to access FrameTree and FrameTreeNode for this code.

[kenrb, 2015/10/28 21:07:05]

I was thinking about this also, but I can't think of a good reason to try to
bring them together. This is fine.

+  RenderWidgetHostImpl* target_rwh;
+  FrameTreeNode* focused_frame = frame_tree_.GetFocusedFrame();
+  if (!focused_frame) {
+    target_rwh = GetMainFrame()->GetRenderWidgetHost();
+  } else {
+    target_rwh = RenderWidgetHostImpl::From(
+        focused_frame->current_frame_host()->GetView()->GetRenderWidgetHost());
+  }
+  return target_rwh;

[alexmos, 2015/10/23 23:40:16]

I thought about whether this should check AreCrossProcessFramesPossible and
return early if not, similarly to what's done for mouse events.  But the code
here should work regardless and just always get the root frame's RWH without
--site-per-process, so I thought that there isn't much benefit to adding the
check, other than getting to the root RWH a bit faster.

[kenrb, 2015/10/28 21:07:05]

Will this affect browser plugin? Currently that still does input event
forwarding, so if the code here short-circuits it then it might break something.
That was one of the reasons I disabled browser process hit testing for
non-site-per-process (the other being that I didn't want bugs to cause trunk
regressions).

[alexmos, 2015/10/29 05:26:29]

I've tried it on a sample app with <webview>, and keyboard events still worked,
both with and without --site-per-process.  I've dug into this a little, and so
far my understanding is that it shouldn't be affected.  The guest will not show
up in the FrameTree of the embedder WebContents, so we will still send the
keyboard event to the embedder's main frame RWH.  On the renderer side, the
embedder forwards the event for the guest back to browser process from
BrowserPlugin::handleInputEvent, which will reach
RenderWidgetHostViewGuest::OnHandleInputEvent.  Since I didn't modify any
keyboard forwarding there, the event should be forwarded directly to the guest's
RWH as before.

That said, I'm noticing that two WebViewTests related to interstitials are
failing on the site isolation bot with the latest patchset (they passed in
previous patchset).  I'm looking into those.

For the regular cases, my reasoning was that without
AreCrossProcessFramesPossible, calling GetView() on any frame in a FrameTree
will just walk up the parent chain to the main frame and return its RVHI/RWHI
(since it's the only frame that has a render_widget_host_).  I guess there's no
reason to do this walk without AreCrossProcessFramesPossible, so I added an
early escape to return the main frame's RWH.

+}
+
 void WebContentsImpl::EnterFullscreenMode(const GURL& origin) {
   // This method is being called to enter renderer-initiated fullscreen mode.
   // Make sure any existing fullscreen widget is shut down first.