Add mobile optimized to TouchEvent Timeout For Android WebView

Add mobile optimized to TouchEvent Timeout For Android WebView

Due to an unfortunate factoring oversight, the InputRouter was never informed of the
mobile-optimized status from frame metadata.  This means that the touch event
timeout was globally applied for Android WebView.

This patch forwards the notification appropriately to the input router, so
that the timeout is limited to when viewing oversized ("desktop") pages in
WebView (just like in Chrome for Android).

BUG=645337
Committed: https://crrev.com/e68beb5d5b4092c21ff77c0b9db69c4729bc99b9
Cr-Commit-Position: refs/heads/master@{#417869}

[johnstonli, 2016-09-02 04:01:45]

Description was changed from

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

BUG=
==========

to

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

BUG=nobug

Android Chrome set mobile optimized to Touchevent Timeout,but Android WebView do
not ,so apply it
==========

[johnstonli, 2016-09-02 04:01:45]

johnstonli@tencent.com changed reviewers:
+ jamesxwei@tencent.com, rjkroege@chromium.org, sievers@chromium.org

[johnstonli, 2016-09-02 04:04:27]

Add mobile optimized to TouchEvent Timeout For Android WebView.
Android Chrome set mobile optimized to Touchevent Timeout,but Android WebView do
not ,so apply it

[johnstonli, 2016-09-08 07:28:48]

johnstonli@tencent.com changed reviewers:
+ sadrul@chromium.org, tdresser@chromium.org
- sievers@chromium.org

[tdresser, 2016-09-08 12:37:31]

tdresser@chromium.org changed reviewers:
+ aelias@chromium.org
- tdresser@chromium.org

[tdresser, 2016-09-08 12:38:03]

tdresser@chromium.org changed reviewers:
+ tdresser@chromium.org

[tdresser, 2016-09-08 12:38:03]

Passing review over to aelias@, who has more WebView context.

[rjkroege, 2016-09-08 22:30:51]

On 2016/09/08 12:38:03, tdresser wrote:
> Passing review over to aelias@, who has more WebView context.

I should pass too for the same reason.

[aelias_OOO_until_Jul13, 2016-09-09 01:23:40]

As far as I can tell this patch does nothing, the added line is redundant with a
similar one in RenderWidgetHostImpl::OnSwapCompositorFrame() called right
before.  Also, all the code in both these files is run both in Android WebView
and Chrome.

[aelias_OOO_until_Jul13, 2016-09-09 01:26:43]

I'm inferring you may have a particular WebView app that has a touch timeout
where you don't expect it's necessary.  I believe this patch wouldn't fix that
if so.  If you can file a bug and attach WebView-using app APK exhibiting the
problem, we could look into it.

[johnstonli, 2016-09-09 01:50:16]

On 2016/09/09 01:23:40, aelias wrote:
> As far as I can tell this patch does nothing, the added line is redundant with
a
> similar one in RenderWidgetHostImpl::OnSwapCompositorFrame() called right
> before.  Also, all the code in both these files is run both in Android WebView
> and Chrome.

Android Chrome can execute RenderWidgetHostImpl::OnSwapCompositorFrame,but
Android WebView will not

[johnstonli, 2016-09-09 01:55:21]

On 2016/09/09 01:26:43, aelias wrote:
> I'm inferring you may have a particular WebView app that has a touch timeout
> where you don't expect it's necessary.  I believe this patch wouldn't fix that
> if so.  If you can file a bug and attach WebView-using app APK exhibiting the
> problem, we could look into it.

some native android webview app,when multiple android webview execture
touchevent,because touchevent timeout,
normal touchend change to touchcancel,some js lib such as polymer which has tap
event,it will not to happed

[aelias_OOO_until_Jul13, 2016-09-09 01:59:07]

On 2016/09/09 at 01:50:16, johnstonli wrote:
> Android Chrome can execute RenderWidgetHostImpl::OnSwapCompositorFrame,but
Android WebView will not

Ah, right, good catch.  That's an unfortunate oversight.

Could you move the additional call into
RenderWidgetHostViewAndroid::SynchronousFrameMetadata()?  That's the
WebView-specific call.

[johnstonli, 2016-09-09 02:01:31]

On 2016/09/09 01:59:07, aelias wrote:
> On 2016/09/09 at 01:50:16, johnstonli wrote:
> > Android Chrome can execute RenderWidgetHostImpl::OnSwapCompositorFrame,but
> Android WebView will not
> 
> Ah, right, good catch.  That's an unfortunate oversight.
> 
> Could you move the additional call into
> RenderWidgetHostViewAndroid::SynchronousFrameMetadata()?  That's the
> WebView-specific call.

Android Chrome execute CompositorOutputSurface::SwapBuffers,then send
ViewHostMsg_SwapCompositorFrame,
but Android WebView execute SynchronousCompositorOutputSurface::SwapBuffers,and
not send message.so apply this patch

[johnstonli, 2016-09-09 02:08:34]

On 2016/09/09 02:01:31, johnstonli wrote:
> On 2016/09/09 01:59:07, aelias wrote:
> > On 2016/09/09 at 01:50:16, johnstonli wrote:
> > > Android Chrome can execute RenderWidgetHostImpl::OnSwapCompositorFrame,but
> > Android WebView will not
> > 
> > Ah, right, good catch.  That's an unfortunate oversight.
> > 
> > Could you move the additional call into
> > RenderWidgetHostViewAndroid::SynchronousFrameMetadata()?  That's the
> > WebView-specific call.
> 
> Android Chrome execute CompositorOutputSurface::SwapBuffers,then send
> ViewHostMsg_SwapCompositorFrame,
> but Android WebView execute
SynchronousCompositorOutputSurface::SwapBuffers,and
> not send message.so apply this patch

OK,i will move this modify to
RenderWidgetHostViewAndroid::SynchronousFrameMetadata

[johnstonli, 2016-09-09 02:40:58]

On 2016/09/09 02:08:34, johnstonli wrote:
> On 2016/09/09 02:01:31, johnstonli wrote:
> > On 2016/09/09 01:59:07, aelias wrote:
> > > On 2016/09/09 at 01:50:16, johnstonli wrote:
> > > > Android Chrome can execute
RenderWidgetHostImpl::OnSwapCompositorFrame,but
> > > Android WebView will not
> > > 
> > > Ah, right, good catch.  That's an unfortunate oversight.
> > > 
> > > Could you move the additional call into
> > > RenderWidgetHostViewAndroid::SynchronousFrameMetadata()?  That's the
> > > WebView-specific call.
> > 
> > Android Chrome execute CompositorOutputSurface::SwapBuffers,then send
> > ViewHostMsg_SwapCompositorFrame,
> > but Android WebView execute
> SynchronousCompositorOutputSurface::SwapBuffers,and
> > not send message.so apply this patch
> 
> OK,i will move this modify to
> RenderWidgetHostViewAndroid::SynchronousFrameMetadata

patch2 set move sync mobile optimized to
RenderWidgetHostViewAndroid::SynchronousFrameMetadata

[johnstonli, 2016-09-09 02:45:29]

On 2016/09/09 01:59:07, aelias wrote:
> On 2016/09/09 at 01:50:16, johnstonli wrote:
> > Android Chrome can execute RenderWidgetHostImpl::OnSwapCompositorFrame,but
> Android WebView will not
> 
> Ah, right, good catch.  That's an unfortunate oversight.
> 
> Could you move the additional call into
> RenderWidgetHostViewAndroid::SynchronousFrameMetadata()?  That's the
> WebView-specific call.

add patch2 to move the additional call into
RenderWidgetHostViewAndroid::SynchronousFrameMetadata

[aelias_OOO_until_Jul13, 2016-09-09 03:04:43]

Description was changed from

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

BUG=nobug

Android Chrome set mobile optimized to Touchevent Timeout,but Android WebView do
not ,so apply it
==========

to

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

Android Chrome set mobile optimized to Touchevent Timeout,but Android WebView do
not ,so apply it 

BUG=645337
==========

[aelias_OOO_until_Jul13, 2016-09-09 03:06:07]

OK, aside from comment below, I filed a bug on this.  I'd like to discuss the
possibility of globally disabling the timeout on WebView instead, so let's hold
off this patch until that's decided.  Thanks for discovering this problem and
offering to fix it, it's really useful.

https://codereview.chromium.org/2304043002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2304043002/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:1034:
host_->host_->input_router()->NotifySiteIsMobileOptimized(
Why double-host_ here but not in the if() statement?  Does this compile?

[johnstonli, 2016-09-09 03:28:41]

On 2016/09/09 03:06:07, aelias wrote:
> OK, aside from comment below, I filed a bug on this.  I'd like to discuss the
> possibility of globally disabling the timeout on WebView instead, so let's
hold
> off this patch until that's decided.  Thanks for discovering this problem and
> offering to fix it, it's really useful.
> 
>
https://codereview.chromium.org/2304043002/diff/20001/content/browser/rendere...
> File content/browser/renderer_host/render_widget_host_view_android.cc (right):
> 
>
https://codereview.chromium.org/2304043002/diff/20001/content/browser/rendere...
> content/browser/renderer_host/render_widget_host_view_android.cc:1034:
> host_->host_->input_router()->NotifySiteIsMobileOptimized(
> Why double-host_ here but not in the if() statement?  Does this compile?

I am so sorry,it can compile another chrome project,but this commit chrome
project code is copy from another project.so this happend!

[johnstonli, 2016-09-09 03:39:10]

The CQ bit was checked by johnstonli@tencent.com

[johnstonli, 2016-09-09 03:39:11]

The CQ bit was unchecked by johnstonli@tencent.com

[boliu, 2016-09-09 16:12:28]

fwiw you could code this in a way that prevents future accidents like this

factor out the common part of RenderWidgetHostImpl::OnSwapCompositorFrame into a
separate helper method, and have RWHVA call it. Then add a comment in
OnSwapCompositorFrame that webview doesn't go through that path.

[aelias_OOO_until_Jul13, 2016-09-09 22:07:09]

Description was changed from

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

Android Chrome set mobile optimized to Touchevent Timeout,but Android WebView do
not ,so apply it 

BUG=645337
==========

to

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

Due to an unfortunate factoring oversight, the InputRouter was never informed of
the
mobile-optimized status from frame metadata.  This means that the touch event
timeout was globally applied for Android WebView.

This patch forwards the notification appropriately to the input router, so
that the timeout is limited to when viewing oversized ("desktop") pages in
WebView (just like in Chrome for Android).

BUG=645337
==========

[aelias_OOO_until_Jul13, 2016-09-09 22:09:24]

On 2016/09/09 at 16:12:28, boliu wrote:
> fwiw you could code this in a way that prevents future accidents like this
> 
> factor out the common part of RenderWidgetHostImpl::OnSwapCompositorFrame into
a separate helper method, and have RWHVA call it. Then add a comment in
OnSwapCompositorFrame that webview doesn't go through that path.

There's already a shared helper
RenderWidgetHostViewAndroid::OnFrameMetadataUpdated, but if we move it there
then it doesn't end up called on non-Android (which is a bit of a hypothetical
problem since it's no-op on non-Android today, but it could eventually be used
elsewhere).  And I don't think it's really worth introducing a second helper
method for this loose end.  Anyway, this will likely be revisited soon since
passive event listener intervention might replace this one, and we can decide on
the final factoring then.  lgtm as is.

[aelias_OOO_until_Jul13, 2016-09-09 22:10:05]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2016-09-09 22:10:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 22:10:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-09 22:10:46]

The author johnstonli@tencent.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[johnstonli, 2016-09-12 02:53:58]

The CQ bit was checked by johnstonli@tencent.com

[commit-bot: I haz the power, 2016-09-12 02:54:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 02:58:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-12 02:59:00]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[johnstonli, 2016-09-12 03:16:54]

The CQ bit was checked by johnstonli@tencent.com

[commit-bot: I haz the power, 2016-09-12 03:17:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 03:24:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-12 03:24:31]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[johnstonli, 2016-09-12 03:28:52]

The CQ bit was checked by johnstonli@tencent.com

[johnstonli, 2016-09-12 03:28:52]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org
Link to the patchset: https://codereview.chromium.org/2304043002/#ps60001
(title: "Fix line ending with white spaces")

[commit-bot: I haz the power, 2016-09-12 03:29:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 04:03:03]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-12 04:04:49]

Description was changed from

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

Due to an unfortunate factoring oversight, the InputRouter was never informed of
the
mobile-optimized status from frame metadata.  This means that the touch event
timeout was globally applied for Android WebView.

This patch forwards the notification appropriately to the input router, so
that the timeout is limited to when viewing oversized ("desktop") pages in
WebView (just like in Chrome for Android).

BUG=645337
==========

to

==========
Add mobile optimized to TouchEvent Timeout For Android WebView

Due to an unfortunate factoring oversight, the InputRouter was never informed of
the
mobile-optimized status from frame metadata.  This means that the touch event
timeout was globally applied for Android WebView.

This patch forwards the notification appropriately to the input router, so
that the timeout is limited to when viewing oversized ("desktop") pages in
WebView (just like in Chrome for Android).

BUG=645337

Committed: https://crrev.com/e68beb5d5b4092c21ff77c0b9db69c4729bc99b9
Cr-Commit-Position: refs/heads/master@{#417869}
==========

[commit-bot: I haz the power, 2016-09-12 04:04:50]

Patchset 4 (id:??) landed as
https://crrev.com/e68beb5d5b4092c21ff77c0b9db69c4729bc99b9
Cr-Commit-Position: refs/heads/master@{#417869}