<webview>: Correctly shift focus between WebContents.

content/browser/renderer_host/render_widget_host_delegate.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_RENDERER_HOST_RENDER_WIDGET_HOST_DELEGATE_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_DELEGATE_H_
 
 #include <stdint.h>
 
 #include <vector>
@@ skipping 127 matching lines @@
   // RenderWidgetHost to this function to determine who should ultimately
   // consume the event.  This facilitates keyboard event routing with
   // out-of-process iframes, where multiple RenderWidgetHosts may be involved
   // in rendering a page, yet keyboard events all arrive at the main frame's
   // RenderWidgetHostView.  When a main frame's RenderWidgetHost is passed in,
   // the function returns the focused frame that should consume keyboard
   // events. In all other cases, the function returns back |receiving_widget|.
   virtual RenderWidgetHostImpl* GetFocusedRenderWidgetHost(
       RenderWidgetHostImpl* receiving_widget);
 
+  // Get the RenderWidgetHost that should receive page level focus events. This
+  // will be the widget that is rendering the main frame of the currently
+  // focused WebContents.

[Charlie Reis, 2016/11/16 20:28:23]

I'm having trouble understanding this vs GetFocusedRenderWidgetHost, since I'm
not familiar with page level focus events.  Why is knowing the focused widget
the wrong thing?  Can you elaborate in the comment?

[avallee, 2016/11/16 21:18:10]

See the <webview> focus explainer for extra details.
https://docs.google.com/document/d/1ysPHsls5CFRhIfW19p8uYRXtVpSwKt6ZY39IX_6AnjY/

The point is that in a guest view case for example <webview> either the webview
is active or the embedder is. Each is considered a different page, they don't
really have insight into each other's content. ekaramad has a tabbed <webview>
browser and when switching from tab to tab, the old webview should receive
events to tell it that the page is no longer focused, and the new page will
receive focus.


This is used by RenderWidgetHostImpl::Focus/Blur to forward the event to the
RenderWidgetHostImpl that is currently "active" or have page focus.

The main frame widget for a single WebContents in a tree of WebContents will be
focused.

Knowing the focused widget is the wrong thing since this might be the widget for
some oopif but in the same WebContents, the single WebContents case is already
handled and works.

[Charlie Reis, 2016/11/16 21:51:19]

Ok.  I think more of this needs to come across in the comment in the code, since
these methods don't sound like they're specific to the inner/outer WebContents
case.  Maybe the idea from web_contents_impl.h of having a larger explanation
for these focus methods would help here as well?

[alexmos, 2016/11/16 23:29:03]

+1 to making all relevant comments emphasize that the main use case is for
inner/outer WebContents.

[Charlie Reis, 2016/11/17 00:49:47]

Looks like this was overlooked.

+  virtual RenderWidgetHostImpl* GetRenderWidgetHostWithPageFocus();
+
+  // In cases with multiple RenderWidgetHosts involved in rendering a page, only
+  // one widget should be focused and active. This ensures that
+  // |render_widget_host| is focused and that its owning WebContents is also
+  // focused.
+  virtual void EnsureOwningContentsIsFocused(

[Charlie Reis, 2016/11/16 20:28:23]

We should try to be more direct in the name, since it's not clear what action
"Ensure" is taking.  Would FocusOwningWebContents be accurate?

[avallee, 2016/11/16 21:18:10]

Good idea. Done.

+      RenderWidgetHostImpl* render_widget_host) {}
+
   // Notification that the renderer has become unresponsive. The
   // delegate can use this notification to show a warning to the user.
   virtual void RendererUnresponsive(RenderWidgetHostImpl* render_widget_host,
                                     RendererUnresponsiveType type) {}
 
   // Notification that a previously unresponsive renderer has become
   // responsive again. The delegate can use this notification to end the
   // warning shown to the user.
   virtual void RendererResponsive(RenderWidgetHostImpl* render_widget_host) {}
 
@@ skipping 61 matching lines @@
   // Allow the delegate to handle the cursor update. Returns true if handled.
   virtual bool OnUpdateDragCursor();
 
  protected:
   virtual ~RenderWidgetHostDelegate() {}
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_DELEGATE_H_