<webview>: Correctly shift focus between WebContents.

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 b3b4e822ba2b0739870141f28e0dc31c20681428..741ab032ce27de51413c719bc79928ad87ffef41 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -482,9 +482,9 @@ WebContentsImpl::~WebContentsImpl() {
     entry.second->CloseAllBindings();
 
   WebContentsImpl* outermost = GetOutermostWebContents();
-  if (GetFocusedWebContents() == this && this != outermost) {
+  if (this != outermost && ContainsOrIsFocusedWebContents()) {
     // If the current WebContents is in focus, unset it.
-    outermost->node_->SetFocusedWebContents(outermost);
+    outermost->SetAsFocusedWebContentsIfNecessary();

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

This will now end up calling 
  old_contents->GetMainFrame()->GetRenderWidgetHost()->SetPageFocus(false)

where old_contents is the contents being destroyed.  This SetPageFocus isn't
really necessary, but I also wonder if the main frame RFH and its RWH are still
around or if we've deleted them by now, in which case this would be UAF?

This really only needs to do two things:
outermost->node_->SetFocusedWebContents(outermost);
outermost->GetMainFrame()->GetRenderWidgetHost()->SetPageFocus(true);

[alexmos, 2016/11/17 00:09:20]

To follow up, looks like the destruction will happen as ~WebContentsImpl, then
~RFH, then ~RWH.  So I think your original call is fine.

[avallee, 2016/11/17 00:20:12]

Ack, not changing. I could always add a !is_being_destroyed in
SetAsFocusedWebContents().

   }
 
   for (FrameTreeNode* node : frame_tree_.Nodes()) {
@@ -1870,6 +1870,10 @@ RenderWidgetHostImpl* WebContentsImpl::GetFocusedRenderWidgetHost(
   return RenderWidgetHostImpl::From(view->GetRenderWidgetHost());
 }
 
+RenderWidgetHostImpl* WebContentsImpl::GetRenderWidgetHostWithPageFocus() {
+  return GetFocusedWebContents()->GetMainFrame()->GetRenderWidgetHost();
+}
+
 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.
@@ -4223,6 +4227,17 @@ WebContentsImpl* WebContentsImpl::GetFocusedWebContents() {
   return outermost->node_->focused_web_contents();
 }
 
+bool WebContentsImpl::ContainsOrIsFocusedWebContents() {
+  for (WebContentsImpl* focused_contents = GetFocusedWebContents();
+       focused_contents;
+       focused_contents = focused_contents->GetOuterWebContents()) {
+    if (focused_contents == this)
+      return true;
+  }
+
+  return false;
+}
+
 WebContentsImpl* WebContentsImpl::GetOutermostWebContents() {
   WebContentsImpl* root = this;
   while (root->GetOuterWebContents())
@@ -4642,29 +4657,32 @@ void WebContentsImpl::EnsureOpenerProxiesExist(RenderFrameHost* source_rfh) {
   }
 }
 
+void WebContentsImpl::SetAsFocusedWebContentsIfNecessary() {
+  // Only change focus if we are not currently focused.
+  WebContentsImpl* old_contents = GetFocusedWebContents();
+  if (old_contents == this)
+    return;
+
+  // Send a page level blur to the old contents so that it displays inactive UI
+  // and focus this contents to activate it.
+  if (old_contents)
+    old_contents->GetMainFrame()->GetRenderWidgetHost()->SetPageFocus(false);
+  GetMainFrame()->GetRenderWidgetHost()->SetPageFocus(true);
+  GetOutermostWebContents()->node_->SetFocusedWebContents(this);
+}
+
 void WebContentsImpl::SetFocusedFrame(FrameTreeNode* node,
                                       SiteInstance* source) {
+  // The PDF plugin still runs as a BrowserPlugin and must go through the
+  // input redirection mechanism. It must not become focused direcly.
   if (!GuestMode::IsCrossProcessFrameGuest(this) && browser_plugin_guest_) {
     frame_tree_.SetFocusedFrame(node, source);
     return;
   }
 
-  // 1. Find old focused frame and unfocus it.
-  // 2. Focus the new frame in the current FrameTree.
-  // 3. Set current WebContents as focused.
-  WebContentsImpl* old_focused_contents = GetFocusedWebContents();
-  if (old_focused_contents != this) {
-    // Focus is moving between frame trees, unfocus the frame in the old tree.
-    old_focused_contents->frame_tree_.SetFocusedFrame(nullptr, source);
-    GetOutermostWebContents()->node_->SetFocusedWebContents(this);
-  }
+  SetAsFocusedWebContentsIfNecessary();
 
   frame_tree_.SetFocusedFrame(node, source);
-
-  // TODO(avallee): Remove this once page focus is fixed.
-  RenderWidgetHostImpl* rwh = node->current_frame_host()->GetRenderWidgetHost();
-  if (rwh && old_focused_contents != this)
-    rwh->Focus();
 }
 
 bool WebContentsImpl::DidAddMessageToConsole(int32_t level,
@@ -4694,6 +4712,22 @@ void WebContentsImpl::OnUserInteraction(
     rdh->OnUserGesture();
 }
 
+void WebContentsImpl::FocusOwningWebContents(
+    RenderWidgetHostImpl* render_widget_host) {
+  // The PDF plugin still runs as a BrowserPlugin and must go through the
+  // input redirection mechanism. It must not become focused direcly.
+  if (!GuestMode::IsCrossProcessFrameGuest(this) && browser_plugin_guest_)
+    return;
+
+  RenderWidgetHostImpl* focused_widget =
+      GetFocusedRenderWidgetHost(render_widget_host);
+
+  if (focused_widget != render_widget_host &&
+      focused_widget->delegate() != render_widget_host->delegate()) {
+    SetAsFocusedWebContentsIfNecessary();
+  }
+}
+
 void WebContentsImpl::OnIgnoredUIEvent() {
   // Notify observers.
   for (auto& observer : observers_)