[Android] Short-circuit touch forwarding when no touch handlers exist

[Android] Short-circuit touch forwarding when no touch handlers exist

Rather than forwarding the touch event to the input router and incurring the
attendant overhead, short-circuit touch forwarding from ContentViewCoreImpl
when no touch handler exists.  This aligns with existing Aura behavior, and
saves ~20us/event when no touch handler exists.

BUG=341613
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=260381

[jdduke (slow), 2014-03-28 17:26:58]

tdresser@: PTAL, thanks.

[tdresser, 2014-03-28 17:55:26]

LGTM with nits.

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.cc (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.cc:1136: // forwarding can be
short-circuited completely.
It feels like this comment is breaking encapsulation by referring to
implementation details of the input router.

Would it be reasonable to reword this more along the lines of "If we don't need
to forward the event to the RWHI, touch forwarding can be short-circuited
completely."

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.h (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.h:355: bool OnMotionEvent(const
ui::MotionEvent& event);
Would DispatchMotionEvent be a better name?

[jdduke (slow), 2014-03-28 18:33:55]

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.cc (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.cc:1136: // forwarding can be
short-circuited completely.
On 2014/03/28 17:55:27, tdresser wrote:
> It feels like this comment is breaking encapsulation by referring to
> implementation details of the input router.
> 
> Would it be reasonable to reword this more along the lines of "If we don't
need
> to forward the event to the RWHI, touch forwarding can be short-circuited
> completely."

Yes, though in my mind those details are vital to understanding why we can allow
this.  However, I suppose if a dev is interested enough in the way they can
refer to the implementation :) In that case, I'll just remove the comment.

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.h (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.h:355: bool OnMotionEvent(const
ui::MotionEvent& event);
On 2014/03/28 17:55:27, tdresser wrote:
> Would DispatchMotionEvent be a better name?

Probably, but in the Java boundary land |OnFoo| is essentially equivalent to
|DispatchFoo| and I'm not sure there's a need to clarify.

[jdduke (slow), 2014-03-28 18:34:05]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-28 18:37:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/217253002/20001

[tdresser, 2014-03-28 18:40:05]

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.cc (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.cc:1136: // forwarding can be
short-circuited completely.
On 2014/03/28 18:33:55, jdduke wrote:
> On 2014/03/28 17:55:27, tdresser wrote:
> > It feels like this comment is breaking encapsulation by referring to
> > implementation details of the input router.
> > 
> > Would it be reasonable to reword this more along the lines of "If we don't
> need
> > to forward the event to the RWHI, touch forwarding can be short-circuited
> > completely."
> 
> Yes, though in my mind those details are vital to understanding why we can
allow
> this.  However, I suppose if a dev is interested enough in the way they can
> refer to the implementation :) In that case, I'll just remove the comment.

SGTM.

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
File content/browser/android/content_view_core_impl.h (right):

https://codereview.chromium.org/217253002/diff/1/content/browser/android/cont...
content/browser/android/content_view_core_impl.h:355: bool OnMotionEvent(const
ui::MotionEvent& event);
On 2014/03/28 18:33:55, jdduke wrote:
> On 2014/03/28 17:55:27, tdresser wrote:
> > Would DispatchMotionEvent be a better name?
> 
> Probably, but in the Java boundary land |OnFoo| is essentially equivalent to
> |DispatchFoo| and I'm not sure there's a need to clarify.

Fine with me.

[commit-bot: I haz the power, 2014-03-28 19:37:31]

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

[commit-bot: I haz the power, 2014-03-28 19:37:32]

Retried try job too often on win_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[jdduke (slow), 2014-03-28 19:41:03]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-28 19:42:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/217253002/40001

[commit-bot: I haz the power, 2014-03-28 23:44:47]

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

[commit-bot: I haz the power, 2014-03-28 23:44:49]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[jdduke (slow), 2014-03-29 00:13:14]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:16:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/217253002/40001

[commit-bot: I haz the power, 2014-03-29 01:55:27]

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

[commit-bot: I haz the power, 2014-03-29 01:55:28]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[jdduke (slow), 2014-03-29 02:54:45]

The CQ bit was checked by jdduke@chromium.org

[commit-bot: I haz the power, 2014-03-29 02:59:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/217253002/40001

[commit-bot: I haz the power, 2014-03-29 03:09:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jdduke@chromium.org/217253002/40001

[commit-bot: I haz the power, 2014-03-29 17:38:38]

Change committed as 260381