Enable browser process hit testing on Android

Enable browser process hit testing on Android

Enable RenderWidgetHostViewAndroid to route mouse/touch events directly
to out-of-process iframe renderers, instead of sending them to the
top-level renderer and relying on renderer-to-renderer forwarding.

This patch also enables the relevant tests on Android.

BUG=491334
Review-Url: https://codereview.chromium.org/2509103002
Cr-Commit-Position: refs/heads/master@{#455559}
Committed: https://chromium.googlesource.com/chromium/src/+/2b5d08b7569cbefecc4bb0eba8f0da15b282672d

[kenrb, 2016-11-16 21:51:33]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 21:52:30]

Dry run: 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-11-17 00:25:53]

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

[commit-bot: I haz the power, 2016-11-17 00:25:54]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[kenrb, 2016-11-22 16:39:28]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 16:39:39]

Dry run: 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-11-22 18:05:14]

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

[commit-bot: I haz the power, 2016-11-22 18:05:15]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[kenrb, 2016-11-28 21:09:13]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 21:10:47]

Dry run: 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-11-28 21:15:04]

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

[commit-bot: I haz the power, 2016-11-28 21:15:05]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kenrb, 2016-11-29 19:44:49]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-29 19:45:14]

Dry run: 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-11-29 20:10:09]

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

[commit-bot: I haz the power, 2016-11-29 20:10:10]

Dry run: 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...)

[kenrb, 2017-02-28 20:39:05]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 20:39:56]

Dry run: 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, 2017-02-28 20:43:47]

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

[commit-bot: I haz the power, 2017-02-28 20:43:48]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kenrb, 2017-02-28 21:43:13]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 21:44:11]

Dry run: 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, 2017-02-28 22:12:33]

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

[commit-bot: I haz the power, 2017-02-28 22:12:34]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[kenrb, 2017-03-01 16:35:28]

Description was changed from

==========
[WIP] Enable browser process hit testing on Android

BUG=491334
==========

to

==========
Enable browser process hit testing on Android

Enable RenderWidgetHostViewAndroid to route mouse/touch events directly
to out-of-process iframe renderers, instead of sending them to the
top-level renderer and relying on renderer-to-renderer forwarding.

This patch also enables the relevant tests on Android.

BUG=491334
==========

[kenrb, 2017-03-01 16:38:06]

kenrb@chromium.org changed reviewers:
+ alexmos@chromium.org, tedchoc@chromium.org

[kenrb, 2017-03-01 16:38:07]

PTAL?
- alexmos@ for content/*
- tedchoc@ for ui/android/*

[Ted C, 2017-03-02 05:14:29]

tedchoc@chromium.org changed reviewers:
+ aelias@chromium.org

[Ted C, 2017-03-02 05:14:30]

+aelias for the content android code since it deals with input

ui/android - lgtm

[aelias_OOO_until_Jul13, 2017-03-03 03:27:37]

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:823: // |point|
from DIPs to pixels before hittesting.
Input events originate in device pixels, so this implies a round-trip from
device -> DIP -> physical, which results in off-by-one for some values.  Is
there a way we could plumb the device pixel values directly with no
transformation?  Other platforms are moving away from DIP in the browser process
over time anyway.

[kenrb, 2017-03-03 18:38:44]

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:823: // |point|
from DIPs to pixels before hittesting.
On 2017/03/03 03:27:37, aelias wrote:
> Input events originate in device pixels, so this implies a round-trip from
> device -> DIP -> physical, which results in off-by-one for some values.  Is
> there a way we could plumb the device pixel values directly with no
> transformation?  Other platforms are moving away from DIP in the browser
process
> over time anyway.

This sounds like a good suggestion, but would really warrant a follow up bug
because it would have to be changed in all the RWHVs, and the callers would have
to be modified, as well as several years.

As it is, we try to avoid the off-by-one in the basic case by not modifying the
point if there is only one RenderWidgetHostView for the page (I.e. no OOPIFs
present).

[aelias_OOO_until_Jul13, 2017-03-03 22:33:09]

OK, lgtm.  I guess this cruft will naturally get cleared out when all platforms
move away from DIP and the issue will be easier to deal with at that stage than
it is today.

[alexmos, 2017-03-06 07:22:04]

Sorry for the delay!  This looks good, and I'm glad we're enabling all those
tests for Android.  Just  a couple of nits and questions - probably naive, since
I'm not too familiar with this code, and otherwise I'll trust aelias@'s review
for all the transforms, etc.

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:469:
SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
Is the AreCrossProcessFramesPossible necessary here?  The equivalent in
RenderWidgetHostViewAura doesn't use it, and wouldn't ShouldRouteEvent check it
as necessary based on individual events?  Or would we not hit ShouldRouteEvent
on the Android paths?

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:812: return
delegated_frame_host_->SurfaceId();
Does this need to worry about delegated_frame_host_ being null at all for any of
the new code (like some other functions in RenderWidgetHostViewAndroid)?  Looks
like its existence is gated on using_browser_compositor_, and there's a comment
in RenderWidgetHostViewAndroid::SupportsAnimation about this possibly being
false for WebView?

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:842: return
GetFrameSinkId();
nit: you could move this if and return up before the if (surface_id.is_valid())
check, which would allow you to just remove the "if (surface_id.is_valid())"

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:901:
gfx::Point* transformed_point) {
In the equivalent RenderWidgetHostViewAura function, there is another check to
return true and set transformed_point to point if target_view == this.  Do we
need the same here? (I admit I don't fully understand why that check is there in
RWHVA)

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:949:
SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
There's a comment on RenderWidgetHostViewEventHandler::ShouldRouteEvent that
touch events should always be routed.  So is the AreCrossProcessFramesPossible
needed here? (And could this use ShouldRouteEvent?)

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:287: float scale_factor =
GetFrameDeviceScaleFactor(shell->web_contents());
Just curious, what values would this actually return for Android?

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1045: EXPECT_TRUE(diff_x <= 2 &&
diff_x >= -2);
nit: in places like these, you could include something like 
  << " actual diff_x: " << diff_x 
at the end to simplify debugging if it ever fails

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1311: // High DPI browser tests
are not needed on Android, and confuse some of the
nit: mention why the High DPI tests are not needed, like in the other tests
below.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1627: #endif
nit: This block is repeated several times; it might make the tests a bit more
readable if you extract it out into a helper.

[kenrb, 2017-03-07 17:43:56]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 17:44:42]

Dry run: 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, 2017-03-07 18:28:47]

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

[commit-bot: I haz the power, 2017-03-07 18:28:48]

Dry run: This issue passed the CQ dry run.

[kenrb, 2017-03-07 19:20:01]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[kenrb, 2017-03-07 19:20:20]

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:469:
SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> Is the AreCrossProcessFramesPossible necessary here?  The equivalent in
> RenderWidgetHostViewAura doesn't use it, and wouldn't ShouldRouteEvent check
it
> as necessary based on individual events?  Or would we not hit ShouldRouteEvent
> on the Android paths?

Done.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:812: return
delegated_frame_host_->SurfaceId();
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> Does this need to worry about delegated_frame_host_ being null at all for any
of
> the new code (like some other functions in RenderWidgetHostViewAndroid)? 
Looks
> like its existence is gated on using_browser_compositor_, and there's a
comment
> in RenderWidgetHostViewAndroid::SupportsAnimation about this possibly being
> false for WebView?

Good point. I have added guards to account for that.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:842: return
GetFrameSinkId();
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> nit: you could move this if and return up before the if
(surface_id.is_valid())
> check, which would allow you to just remove the "if (surface_id.is_valid())"

Having it here guards against the case of it becoming invalid during the hit
test on line 830 (which would mean the hit test failed for some reason).

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:901:
gfx::Point* transformed_point) {
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> In the equivalent RenderWidgetHostViewAura function, there is another check to
> return true and set transformed_point to point if target_view == this.  Do we
> need the same here? (I admit I don't fully understand why that check is there
in
> RWHVA)

Oh yeah, that should be here. I think I added that check to RWHVA after I had
created this CL, and so neglected to propagate it. The reason is that we want to
avoid off-by-one errors from rounding if we are routing the event to its
original target.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:949:
SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> There's a comment on RenderWidgetHostViewEventHandler::ShouldRouteEvent that
> touch events should always be routed.  So is the AreCrossProcessFramesPossible
> needed here? (And could this use ShouldRouteEvent?)

I don't think that is strictly necessary because there is no BrowserPlugin on
Android, and ShouldRouteEvent() is specific to each RenderWidgetHostView
(RenderWidgetHostViewEventHandler, despite its generic name, is Aura-only).

That said, it is probably reasonable to remove the AreCrossProcessFramePossible
check and see how that goes.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
File content/browser/site_per_process_browsertest.cc (right):

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:287: float scale_factor =
GetFrameDeviceScaleFactor(shell->web_contents());
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> Just curious, what values would this actually return for Android?

It is always 3, in my testing.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1045: EXPECT_TRUE(diff_x <= 2 &&
diff_x >= -2);
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> nit: in places like these, you could include something like 
>   << " actual diff_x: " << diff_x 
> at the end to simplify debugging if it ever fails

Done.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1311: // High DPI browser tests
are not needed on Android, and confuse some of the
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> nit: mention why the High DPI tests are not needed, like in the other tests
> below.

Done.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/site_pe...
content/browser/site_per_process_browsertest.cc:1627: #endif
On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> nit: This block is repeated several times; it might make the tests a bit more
> readable if you extract it out into a helper.

Done.

[commit-bot: I haz the power, 2017-03-07 19:21:40]

Dry run: 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, 2017-03-07 19:26:44]

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

[commit-bot: I haz the power, 2017-03-07 19:26:45]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kenrb, 2017-03-07 19:36:02]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 19:36:37]

Dry run: 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, 2017-03-07 20:23:05]

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

[commit-bot: I haz the power, 2017-03-07 20:23:06]

Dry run: This issue passed the CQ dry run.

[alexmos, 2017-03-08 19:50:04]

Thanks, LGTM with one remaining comment below.

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:842: return
GetFrameSinkId();
On 2017/03/07 19:20:19, kenrb wrote:
> On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> > nit: you could move this if and return up before the if
> (surface_id.is_valid())
> > check, which would allow you to just remove the "if (surface_id.is_valid())"
> 
> Having it here guards against the case of it becoming invalid during the hit
> test on line 830 (which would mean the hit test failed for some reason).

Acknowledged.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:901:
gfx::Point* transformed_point) {
On 2017/03/07 19:20:19, kenrb wrote:
> On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> > In the equivalent RenderWidgetHostViewAura function, there is another check
to
> > return true and set transformed_point to point if target_view == this.  Do
we
> > need the same here? (I admit I don't fully understand why that check is
there
> in
> > RWHVA)
> 
> Oh yeah, that should be here. I think I added that check to RWHVA after I had
> created this CL, and so neglected to propagate it. The reason is that we want
to
> avoid off-by-one errors from rounding if we are routing the event to its
> original target.

Do you still need to add it?  I don't see it in the latest PS.

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:949:
SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
On 2017/03/07 19:20:19, kenrb wrote:
> On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> > There's a comment on RenderWidgetHostViewEventHandler::ShouldRouteEvent that
> > touch events should always be routed.  So is the
AreCrossProcessFramesPossible
> > needed here? (And could this use ShouldRouteEvent?)
> 
> I don't think that is strictly necessary because there is no BrowserPlugin on
> Android, and ShouldRouteEvent() is specific to each RenderWidgetHostView
> (RenderWidgetHostViewEventHandler, despite its generic name, is Aura-only).
> 
> That said, it is probably reasonable to remove the
AreCrossProcessFramePossible
> check and see how that goes.

Ah, I see.  I think it's ok to keep this for now then, and try removing it in a
separate CL to make it easier to revert if something goes wrong.

[kenrb, 2017-03-08 21:09:59]

The CQ bit was checked by kenrb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 21:10:39]

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

[kenrb, 2017-03-08 21:15:29]

Thanks for reviewing.

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

https://codereview.chromium.org/2509103002/diff/80001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:901:
gfx::Point* transformed_point) {
On 2017/03/08 19:50:03, alexmos (OOO on 3-7) wrote:
> On 2017/03/07 19:20:19, kenrb wrote:
> > On 2017/03/06 07:22:03, alexmos (OOO on 3-7) wrote:
> > > In the equivalent RenderWidgetHostViewAura function, there is another
check
> to
> > > return true and set transformed_point to point if target_view == this.  Do
> we
> > > need the same here? (I admit I don't fully understand why that check is
> there
> > in
> > > RWHVA)
> > 
> > Oh yeah, that should be here. I think I added that check to RWHVA after I
had
> > created this CL, and so neglected to propagate it. The reason is that we
want
> to
> > avoid off-by-one errors from rounding if we are routing the event to its
> > original target.
> 
> Do you still need to add it?  I don't see it in the latest PS.

That's weird, but thanks for catching it. I am sure I added that,but maybe
forgot to save the file. Fixed now.

[commit-bot: I haz the power, 2017-03-08 21:53:58]

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

[commit-bot: I haz the power, 2017-03-08 21:53:59]

Dry run: This issue passed the CQ dry run.

[kenrb, 2017-03-08 22:15:55]

The CQ bit was checked by kenrb@chromium.org

[kenrb, 2017-03-08 22:15:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, aelias@chromium.org, alexmos@chromium.org
Link to the patchset: https://codereview.chromium.org/2509103002/#ps140001
(title: "Added missing check")

[commit-bot: I haz the power, 2017-03-08 22:16:26]

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, 2017-03-08 22:31:33]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1489011355274770,
"parent_rev": "8bd7796c6ec121c6885f16218ce3c3585a24db75", "commit_rev":
"2b5d08b7569cbefecc4bb0eba8f0da15b282672d"}

[commit-bot: I haz the power, 2017-03-08 22:33:42]

Description was changed from

==========
Enable browser process hit testing on Android

Enable RenderWidgetHostViewAndroid to route mouse/touch events directly
to out-of-process iframe renderers, instead of sending them to the
top-level renderer and relying on renderer-to-renderer forwarding.

This patch also enables the relevant tests on Android.

BUG=491334
==========

to

==========
Enable browser process hit testing on Android

Enable RenderWidgetHostViewAndroid to route mouse/touch events directly
to out-of-process iframe renderers, instead of sending them to the
top-level renderer and relying on renderer-to-renderer forwarding.

This patch also enables the relevant tests on Android.

BUG=491334

Review-Url: https://codereview.chromium.org/2509103002
Cr-Commit-Position: refs/heads/master@{#455559}
Committed:
https://chromium.googlesource.com/chromium/src/+/2b5d08b7569cbefecc4bb0eba8f0...
==========

[commit-bot: I haz the power, 2017-03-08 22:33:43]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/2b5d08b7569cbefecc4bb0eba8f0...

[jbudorick, 2017-03-10 01:24:31]

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

[jbudorick, 2017-03-10 01:24:32]

Some of these appear to be broken on the 64-bit debug bot:
https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit...