Use RWH instead of RWHV to know whether a frame is focus.

content/browser/frame_host/render_frame_host_impl.cc

 // Copyright 2013 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/frame_host/render_frame_host_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
@@ skipping 1576 matching lines @@
 #if defined(OS_ANDROID)
   // The Android-specific service registry has a reference to
   // |service_registry_| and thus must be torn down first.
   service_registry_android_.reset();
 #endif
 
   service_registry_.reset();
 }
 
 bool RenderFrameHostImpl::IsFocused() {
-  return GetView()->HasFocus() &&
+  return RenderWidgetHostImpl::From(
+            GetView()->GetRenderWidgetHost())->is_focused() &&

[Charlie Reis, 2015/02/02 18:03:55]

This is pretty awkward.  Ken, this is the kind of thing I was fearing due to
RFHI::GetRenderWidgetHost() returning nullptr in some cases.  Can we just fix
that method and use it here?

[mlamouri (slow - plz ping), 2015/02/02 18:21:15]

Yes, GetRenderWidgetHost() doesn't behave like I would expect and Ken, for the
other patches said that doing this was fine. I would be okay in changing that
call but it's already being used in other places. I would gladly add a TODO and
point to a bug if we consider changing that.

[kenrb, 2015/02/02 18:28:17]

I agree it is awkward. I am okay with it because I think it should be an
exceptional case.

I am somewhat suspicious of anything that needs to get an ancestral
RenderWidgetHost from the RenderFrameHost. I believe you probably want the
RenderWidgetHostView in that case, which is why he checked focus state on the
view in the earlier patch. I talked to Mounir about this and he says it fails
tests, but he isn't exactly clear on the circumstances in which the
RenderWidgetHost and RenderWidgetHostView are reporting different values for
is_focused()/HasFocus(), but since the RWH seems to behave more reliably, this
change was needed.

[Charlie Reis, 2015/02/02 18:34:49]

Ok, please file a bug and add a TODO.  I think this is very important for the
RenderFrameHostImpl API, but it doesn't have to get fixed in this CL.

[Charlie Reis, 2015/02/02 18:34:49]

This is exactly what's bothering me about this CL.

We don't seem to understand why these cases differ, and why this method is
needed.  I'm inclined to not approve this until then.

          frame_tree_->GetFocusedFrame() &&
          (frame_tree_->GetFocusedFrame() == frame_tree_node() ||
           frame_tree_->GetFocusedFrame()->IsDescendantOf(frame_tree_node()));
 }
 
 void RenderFrameHostImpl::UpdateCrossProcessIframeAccessibility(
     const std::map<int32, int>& node_to_frame_routing_id_map) {
   for (const auto& iter : node_to_frame_routing_id_map) {
     // This is the id of the accessibility node that has a child frame.
     int32 node_id = iter.first;
@@ skipping 163 matching lines @@
 void RenderFrameHostImpl::DidUseGeolocationPermission() {
   RenderFrameHost* top_frame = frame_tree_node()->frame_tree()->GetMainFrame();
   GetContentClient()->browser()->RegisterPermissionUsage(
       PERMISSION_GEOLOCATION,
       delegate_->GetAsWebContents(),
       GetLastCommittedURL().GetOrigin(),
       top_frame->GetLastCommittedURL().GetOrigin());
 }
 
 }  // namespace content