[Android] Show autofill popup after animation.

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 7f38d6c98c43cf5910a9efe523420224fe8b32d7..557237f543ae696824dfad612df7c8f22ace88b7 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1503,11 +1503,15 @@ void RenderViewImpl::OnScrollFocusedEditableNodeIntoRect(
   }
 
   blink::WebElement element = GetFocusedElement();
+  bool will_animate = false;
   if (!element.isNull() && IsEditableNode(element)) {
     rect_for_scrolled_focused_editable_node_ = rect;
     has_scrolled_focused_editable_node_into_rect_ = true;
-    webview()->scrollFocusedNodeIntoRect(rect);
+    will_animate = webview()->scrollFocusedNodeIntoRect(rect);
   }
+
+  FOR_EACH_OBSERVER(RenderViewObserver, observers_,

[aelias_OOO_until_Jul13, 2014/11/22 03:38:11]

It's brittle to tie the autofill showing up at all to an optional
scroll-into-view action that might be suppressed before we ever get here.  And
it also leads you to adding "#if !defined(OS_ANDROID)" in
AutofillAgent::FocusedNodeChanged, which is something we prefer to avoid.

Please make autofill observer notified based on a mandatory event that happens
on every form field focus instead (might need to introduce something like
ViewMsg_FocusChangeComplete for that).  And store will_animate_ in a member
variable that defaults to false in this method.

[please use gerrit instead, 2014/11/24 20:22:31]

FocusChangeComplete event is a good idea. On Android this can be invoked either
from here or from DidCompletePageScaleAnimation. I am working on figuring out
where to issue this event on desktop platforms. Let me know if you have
suggestions.

[aelias_OOO_until_Jul13, 2014/11/25 01:26:34]

To be clear, what I'm suggesting is that we should have the sequence with
guaranteed ordering:
1. Focus
2. ScrollIntoView (optional)
3. FocusComplete
4. DidCompletePageScaleAnimation (optional)

The first three events are very close to each other in time.  On ScrollIntoView,
we would set a flag if a page scale animation was issued, and on FocusComplete,
we would either show autofill immediately if that flag is false, or wait until
DidCompletePageScaleAnimation if true.  This means we wouldn't need any
platform-specific logic aside from not triggering the page scale animation.

I don't know the focus code well enough to offer a very specific suggestion of
how it should be added, although I think it would probably be in the browser
process, who sends messages to trigger the first two -- we could send another
message right after that.

[please use gerrit instead, 2014/12/15 18:39:55]

The step #2 (FocusComplete) is not necessary, because there's already an event
that happens at the very end: OnHandledGestureEvent/OnHandledClickEvent/etc.
Also, we don't have all of the required information on whether
DidCompletePageScaleAnimation will happen at the end of the focus event in
renderer, because browser makes some decisions as well. The solution is for the
browser to let the renderer know whether it will initiate the page scale
animation or not. See
https://docs.google.com/document/d/1-5bhgEYhjHeJaPBYZZTLA0goc13epj1r_7bU-zFRP...
for a diagram.

+                    WillAnimatePageScale(will_animate));
 }
 
 void RenderViewImpl::OnSetEditCommandsForNextKeyEvent(
@@ -3848,6 +3852,11 @@ void RenderViewImpl::InstrumentWillComposite() {
   webview()->devToolsAgent()->willComposite();
 }
 
+void RenderViewImpl::DidCompletePageScaleAnimation() {
+  FOR_EACH_OBSERVER(RenderViewObserver, observers_,
+                    DidCompletePageScaleAnimation());
+}
+
 void RenderViewImpl::SetScreenMetricsEmulationParameters(
     float device_scale_factor,
     const gfx::Point& root_layer_offset,