Remove RenderViewImpl::GetFocusedElement.

content/renderer/render_view_impl.cc

 // 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.
 
 #include "content/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <memory>
 
@@ skipping 1390 matching lines @@
 }
 
 void RenderViewImpl::OnScrollFocusedEditableNodeIntoRect(
     const gfx::Rect& rect) {
   if (has_scrolled_focused_editable_node_into_rect_ &&
       rect == rect_for_scrolled_focused_editable_node_) {
     GetWidget()->FocusChangeComplete();
     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();
 }
 
 void RenderViewImpl::OnSetEditCommandsForNextKeyEvent(
     const EditCommands& edit_commands) {
   edit_commands_ = edit_commands;
 }
 
 void RenderViewImpl::OnSetHistoryOffsetAndLength(int history_offset,
                                                  int history_length) {
@@ skipping 888 matching lines @@
 }
 
 void RenderViewImpl::DidStartLoading() {
   main_render_frame_->didStartLoading(true);
 }
 
 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;
   webview()->setPageScaleFactor(page_scale_factor);
 }
 
 void RenderViewImpl::OnZoom(PageZoom zoom) {
   if (!webview())  // Not sure if this can happen, but no harm in being safe.
     return;
 
@@ skipping 1052 matching lines @@
         return render_frame->focused_pepper_plugin();
     }
     frame = frame->traverseNext(false);
   }
 
   return nullptr;
 }
 #endif
 
 }  // namespace content