Android WebView: flush input events during onVSync

content/browser/renderer_host/render_widget_host_view_android.cc

Index: content/browser/renderer_host/render_widget_host_view_android.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index dd65887f78db5d47ad01b2adc5b5da168d243a18..46c0427313cf0370f34ae29ab51b92739ccea826 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -811,6 +811,12 @@ void RenderWidgetHostViewAndroid::ShowDisambiguationPopup(
 
 scoped_ptr<SyntheticGestureTarget>
 RenderWidgetHostViewAndroid::CreateSyntheticGestureTarget() {
+  DCHECK(content_view_core_);
+  if (using_synchronous_compositor_) {

[boliu, 2014/08/19 22:54:39]

&& !observing_root_window_

[hush (inactive), 2014/08/19 23:24:36]

Done.

+    content_view_core_->GetWindowAndroid()->AddObserver(this);
+    observing_root_window_ = true;
+  }
+
   return scoped_ptr<SyntheticGestureTarget>(new SyntheticGestureTargetAndroid(
       host_, content_view_core_->CreateTouchEventSynthesizer()));
 }
@@ -1529,6 +1535,9 @@ void RenderWidgetHostViewAndroid::OnVSync(base::TimeTicks frame_time,
     host_->FlushInput();
   }
 
+  if (using_synchronous_compositor_)

[boliu, 2014/08/19 22:54:39]

Can we also remove this as observer if there are no more synthetic gestures? Is
there an easy way to detect that?

[hush (inactive), 2014/08/19 23:24:36]

I'm not sure you can do that...
You can't know if there are any more synthetic gestures, because they are coming
from javascript console and could be typed by the user any time when the user
connects to chrome DevTools.

Also there's no need to remove observer when the synthetic gesture target is
destructed because SyntheticGestureController does not change its GestureTarget
once it is initialized and SyntheticGestureController only gets destructed when
the render process exited
(https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...)
and webview is gone at that point so there is no need to remove observers.


On 2014/08/19 22:54:39, boliu wrote:

[jdduke (slow), 2014/08/19 23:30:38]

|DidFlushInput| will be called when synthetic *events* have been fully flushed,
however, the synthetic gesture may yet be active, and there's not really a great
way of determining this from RWHVAndroid. Keep in mind that synthetic tests are
designed to run in synthetic environments, so I'm not sure it's critical that we
"clean up" after ourselves here.

Rather than putting this early out here, why not have a simple
|vsync_subscribers_| bit field that allows for input, transient BeginFrame and
permanent BeginFrame subscriber types? That should handle the WebView case
naturally, as WebView should never request BeginFrame under any circumstances,
no? Perhaps we have a helper method |SubscribeToVsync(SubscriberType)| that
requests the vsync and updates the flags as appropriate.

[boliu, 2014/08/19 23:37:47]

Never mind then. Maybe SyntheticGestureController should have a call to clean
things up, but no need to do it in this CL.
Right, webview should never request begin frame here.
This is outside my knowledge now, so I guess just do what jared says.

What's transient BeginFrame??

[jdduke (slow), 2014/08/19 23:41:33]

There are a couple one-off cases where we "kick start" BeginFrame without
actually permanently subscribing to it, e.g., on touch start, on |WasShown()|
and when we swap out the ContentViewCore.

Hmm, why does every touch event request a vsync update? That smells a bit, I
would think it's only the initial touchstart or touchmove..

[hush (inactive), 2014/08/20 00:16:14]

So there are 3 VSync Subscriber Types:
input: only flushes input OnVSync, never request beginFrame.
transient BeginFrame: requests beginFrame once during the first OnVSync. Clear
the corresponding bit in |vsync_subscribers_| after that.
permanent BeginFrame: always requests beginFrame during each OnVSync.


On 2014/08/19 23:41:33, jdduke wrote:

[jdduke (slow), 2014/08/20 00:21:33]

Yup, that sounds about right. This way we can also enforce subscription validity
with DCHECK (i.e., synchronous_compositor_ should never coincide with BeginFrame
subscriptions).

+    return;
+
   TRACE_EVENT0("cc", "RenderWidgetHostViewAndroid::SendBeginFrame");
   base::TimeTicks display_time = frame_time + vsync_period;