Remove RenderViewImpl::GetFocusedElement.

content/renderer/render_view_impl.cc

Index: content/renderer/render_view_impl.cc
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index b530435b68a160285e3da2a5f6e236a977fa07cd..f1375059ef59eb4323a7aa824f543a202268095a 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1408,15 +1408,12 @@ void RenderViewImpl::OnScrollFocusedEditableNodeIntoRect(
     return;
   }
 
-  blink::WebElement element = GetFocusedElement();
-  bool will_animate = false;
-  if (!element.isNull() && element.isEditable()) {
-    rect_for_scrolled_focused_editable_node_ = rect;
-    has_scrolled_focused_editable_node_into_rect_ = true;
-    will_animate = webview()->scrollFocusedNodeIntoRect(rect);
-  }
+  if (!webview()->scrollFocusedEditableElementIntoRect(rect))
+    return;
 
-  if (!will_animate)
+  rect_for_scrolled_focused_editable_node_ = rect;
+  has_scrolled_focused_editable_node_into_rect_ = true;
+  if (!compositor()->hasPendingPageScaleAnimation())

[enne (OOO), 2016/05/18 19:30:29]

I don't know this code at all, but what's the goal of this check? It looks like
if you have a pending page scale animation, then eventually you'll get a
RenderViewImpl::DidCompletePageScaleAnimation which will call
FocusChangeComplete.  So, I'm guessing you're avoiding the call here because
it'll get done later?

I feel like there's a race here, potentially.  There could be an active page
scale animation, but one that's already been committed and so it not pending. 
Then you'd get FocusChangeComplete here and then another one later when the page
scale animation completes.

Is that what you want? What are you trying to do here?

[dglazkov, 2016/05/18 19:50:34]

I am hoping not to make any functional changes -- this change here is just avoid
using passing bools instead of just querying asking the source. 

In the function above, the WebViewImpl::scrollFocusedEditableElementIntoRect may
or may (indirectly) invoke LayerTreeHost::StartPageScaleAnimation. The check in
question is just to see if that happened. There shouldn't be a race in this
case.

However, the new code could be behaving differently is
LayerTreeHost::pending_page_scale_animation_ is already set, and
WebViewImpl::scrollFocusedEditableElementIntoRect ends up _not_ calling
LayerTreeHost::StartPageScaleAnimation. I need to better understand if that's
possible and if the new behavior would be a good idea.

[dglazkov, 2016/05/19 04:12:31]

Oh, I think you meant the code that was there before. Yeah, I can see that this
is really skeezy. I should probably look into it.

For this CL, would you like me to leave this alone and revert to using the bool?
Let me know.

     GetWidget()->FocusChangeComplete();
 }
 
@@ -2325,19 +2322,6 @@ void RenderViewImpl::DidStopLoading() {
   main_render_frame_->didStopLoading();
 }
 
-blink::WebElement RenderViewImpl::GetFocusedElement() const {
-  if (!webview())
-    return WebElement();
-  WebFrame* focused_frame = webview()->focusedFrame();
-  if (focused_frame) {
-    WebDocument doc = focused_frame->document();
-    if (!doc.isNull())
-      return doc.focusedElement();
-  }
-
-  return WebElement();
-}
-
 void RenderViewImpl::OnSetPageScale(float page_scale_factor) {
   if (!webview())
     return;