Fix page navigation being incorrectly fired on Mac OSX.

Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org

Committed: https://crrev.com/b2c5c9d8120abd6dd8605ab430ad557857f75bda
Cr-Commit-Position: refs/heads/master@{#384770}

[dtapuska, 2016-03-31 22:03:50]

Description was changed from

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

BUG=599153
==========

to

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
==========

[dtapuska, 2016-03-31 22:03:50]

dtapuska@chromium.org changed reviewers:
+ asvitkine@chromium.org, creis@chromium.org, erikchen@chromium.org

[erikchen, 2016-04-01 03:44:56]

Looks like the test failure is real?

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
// renderer. This variables defaults to NO for new gestures.
This comment applies to the previous ivar, which I understood. I don't
understand the new ivars, nor why you need two rather than just one. 

Is this the right doc?
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...

My understanding:
1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser, which are
forwarded to blink.
2. Blink converts these to GestureScrollBegin/Update/End events.
3. Depending on whether the relevant layer has passive, blocking, or no
listener, a different message is sent to the main thread. [which thread is this
done on? impl thread?]
4. The main thread then forwards the results to the browser.

If this is accurate, why does the browser need to know about
GesturescrollEvents? That seems like an implementation detail of Blink. The
embedder passes in a wheel event, and just needs to know whether Blink consumed
it.

[dtapuska, 2016-04-01 04:15:02]

On 2016/04/01 03:44:56, erikchen wrote:
> Looks like the test failure is real?
> 
>
https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
> File
>
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
> (right):
> 
>
https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
>
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
> // renderer. This variables defaults to NO for new gestures.
> This comment applies to the previous ivar, which I understood. I don't
> understand the new ivars, nor why you need two rather than just one. 
> 
> Is this the right doc?
>
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...
> 
> My understanding:
> 1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser, which are
> forwarded to blink.
> 2. Blink converts these to GestureScrollBegin/Update/End events.
> 3. Depending on whether the relevant layer has passive, blocking, or no
> listener, a different message is sent to the main thread. [which thread is
this
> done on? impl thread?]
> 4. The main thread then forwards the results to the browser.
> 
> If this is accurate, why does the browser need to know about
> GesturescrollEvents? That seems like an implementation detail of Blink. The
> embedder passes in a wheel event, and just needs to know whether Blink
consumed
> it.

Yes that is the right doc.

1) The OS Sends the NSEvents to the browser.
2) The browser turns this into a MouseWheelEvent which is still sent to blink.
3) Blink doesn't normally handle the event (unless javascript consumes it)
4) If unconsumed the browser process generates gestures for GSB/GSU/GSE

The embedder render_view_host_impl is the one generating the gesture events
(because
that is where touch events are currently generated and we can't really handle
generating gesture sequences from two different points because they do have
to know about each other's states).

Since the wheel event goes unhandled; it was getting up into the history_swiper.

I guess we could put logic into the render_view_host_impl_mac to track the last
wheel event and try to associate it with a gesture event but that just sounds
ugly to me.

[Charlie Reis, 2016-04-01 16:16:49]

creis@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[Charlie Reis, 2016-04-01 16:16:50]

[+wjmaclean]

Adding James, who has been looking at content-level code for gesture events in
https://crbug.com/587023 (for OOPIFs).  James, any thoughts on the discussion
above?

[erikchen, 2016-04-01 16:18:58]

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
// renderer. This variables defaults to NO for new gestures.
On 2016/04/01 03:44:56, erikchen wrote:
> This comment applies to the previous ivar, which I understood. I don't
> understand the new ivars, nor why you need two rather than just one. 
> 
> Is this the right doc?
>
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...
> 
> My understanding:
> 1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser, which are
> forwarded to blink.
> 2. Blink converts these to GestureScrollBegin/Update/End events.
> 3. Depending on whether the relevant layer has passive, blocking, or no
> listener, a different message is sent to the main thread. [which thread is
this
> done on? impl thread?]
> 4. The main thread then forwards the results to the browser.
> 
> If this is accurate, why does the browser need to know about
> GesturescrollEvents? That seems like an implementation detail of Blink. The
> embedder passes in a wheel event, and just needs to know whether Blink
consumed
> it.

Please add your explanation here, as well as updated comments for these two
ivars.

Based on your explanation, would an ivar named gestureScrollUnconsumed_ make
more sense?

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm:134:
firstScrollUnconsumed_ = !consumed;
I don't understand this logic.

Does there exist a mechanism by which the renderer tells the browser: I will
never consume any more scroll events?

The logic on line 565 should reflect that condition.

[dtapuska, 2016-04-01 17:22:25]

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
// renderer. This variables defaults to NO for new gestures.
On 2016/04/01 16:18:58, erikchen wrote:
> On 2016/04/01 03:44:56, erikchen wrote:
> > This comment applies to the previous ivar, which I understood. I don't
> > understand the new ivars, nor why you need two rather than just one. 
> > 
> > Is this the right doc?
> >
>
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...
> > 
> > My understanding:
> > 1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser, which
are
> > forwarded to blink.
> > 2. Blink converts these to GestureScrollBegin/Update/End events.
> > 3. Depending on whether the relevant layer has passive, blocking, or no
> > listener, a different message is sent to the main thread. [which thread is
> this
> > done on? impl thread?]
> > 4. The main thread then forwards the results to the browser.
> > 
> > If this is accurate, why does the browser need to know about
> > GesturescrollEvents? That seems like an implementation detail of Blink. The
> > embedder passes in a wheel event, and just needs to know whether Blink
> consumed
> > it.
> 
> Please add your explanation here, as well as updated comments for these two
> ivars.
> 
> Based on your explanation, would an ivar named gestureScrollUnconsumed_ make
> more sense?

The logic is about the first gesture scroll after the phase began is unconsumed.
Not that the gesture scroll was unconsumed.

ie; in the example GSB, GSU(A), GSU(B), GSU(C), GSE

GSU(A), GSU(B) could be consumed, but GSU(C) won't trigger the arrow.

But in example: GSB, GSU(X), GSU(Y), GSE

GSU(X) goes unconsumed then it will trigger the arrow.

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm:134:
firstScrollUnconsumed_ = !consumed;
On 2016/04/01 16:18:58, erikchen wrote:
> I don't understand this logic.
> 
> Does there exist a mechanism by which the renderer tells the browser: I will
> never consume any more scroll events?
> 
> The logic on line 565 should reflect that condition.

The condition matches the condition logic on line 119 (which uses PhaseBegan).
In that if the first scroll isn't handled by the page then the history arrow is
shown. Being that ScrollBegin isn't a blocking event we don't know if it was
handled or not. But the first ScrollUpdate will tell us.

[wjmaclean, 2016-04-01 17:31:05]

On 2016/04/01 04:15:02, dtapuska wrote:
> 
> Yes that is the right doc.
> 
> 1) The OS Sends the NSEvents to the browser.
> 2) The browser turns this into a MouseWheelEvent which is still sent to blink.
> 3) Blink doesn't normally handle the event (unless javascript consumes it)
> 4) If unconsumed the browser process generates gestures for GSB/GSU/GSE
> 
> The embedder render_view_host_impl is the one generating the gesture events
> (because
> that is where touch events are currently generated and we can't really handle
> generating gesture sequences from two different points because they do have
> to know about each other's states).

I'm surprised that gesture events are generated in RenderViewHostImpl ...
generally these should come from the GestureRecognizer, no? If something in RVHI
*is* creating gesture events, they may not be targeted correctly (currently all
gesture events originating from the GestureRecognizer are routed through
RenderWidgetHostInputEventRouter::RouteGestureEvent, and this will ignore
gestures for which there is no corresponding touch sequence that has also
travelled through RWHIER.

This sounds vaguely like the generation of synthetic touch events for DevTools,
which is currently on my ToDo list to fix (and a major pain since it generates
the touches in the RenderWidgetHostImpl (using TouchEmulator).

+tdresser as he may have further insight into this.

[wjmaclean, 2016-04-01 17:31:23]

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

[erikchen, 2016-04-01 17:57:34]

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
// renderer. This variables defaults to NO for new gestures.
On 2016/04/01 17:22:25, dtapuska wrote:
> On 2016/04/01 16:18:58, erikchen wrote:
> > On 2016/04/01 03:44:56, erikchen wrote:
> > > This comment applies to the previous ivar, which I understood. I don't
> > > understand the new ivars, nor why you need two rather than just one. 
> > > 
> > > Is this the right doc?
> > >
> >
>
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...
> > > 
> > > My understanding:
> > > 1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser, which
> are
> > > forwarded to blink.
> > > 2. Blink converts these to GestureScrollBegin/Update/End events.
> > > 3. Depending on whether the relevant layer has passive, blocking, or no
> > > listener, a different message is sent to the main thread. [which thread is
> > this
> > > done on? impl thread?]
> > > 4. The main thread then forwards the results to the browser.
> > > 
> > > If this is accurate, why does the browser need to know about
> > > GesturescrollEvents? That seems like an implementation detail of Blink.
The
> > > embedder passes in a wheel event, and just needs to know whether Blink
> > consumed
> > > it.
> > 
> > Please add your explanation here, as well as updated comments for these two
> > ivars.
> > 
> > Based on your explanation, would an ivar named gestureScrollUnconsumed_ make
> > more sense?
> 
> The logic is about the first gesture scroll after the phase began is
unconsumed.
> Not that the gesture scroll was unconsumed.
> 
> ie; in the example GSB, GSU(A), GSU(B), GSU(C), GSE
> 
> GSU(A), GSU(B) could be consumed, but GSU(C) won't trigger the arrow.
> 
> But in example: GSB, GSU(X), GSU(Y), GSE
> 
> GSU(X) goes unconsumed then it will trigger the arrow.

I'm sorry, but I don't understand your explanations. You're using the terms
"won't trigger the arrow" and "trigger the arrow", but I don't know if you're
describing the previous behavior, or the behavior that you are trying to induce.

Can you update the comments for this ivars to describe what they mean?

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm:134:
firstScrollUnconsumed_ = !consumed;
On 2016/04/01 17:22:25, dtapuska wrote:
> On 2016/04/01 16:18:58, erikchen wrote:
> > I don't understand this logic.
> > 
> > Does there exist a mechanism by which the renderer tells the browser: I will
> > never consume any more scroll events?
> > 
> > The logic on line 565 should reflect that condition.
> 
> The condition matches the condition logic on line 119 (which uses PhaseBegan).
> In that if the first scroll isn't handled by the page then the history arrow
is
> shown. Being that ScrollBegin isn't a blocking event we don't know if it was
> handled or not. But the first ScrollUpdate will tell us.

Based on the description you just gave, firstScrollUnconsumed_ should only be
set on this line.

Perhaps you meant:
If rendererHandledWheelEvent does not consume the event, then the renderer
guarantees that it will not need/consume any future events.

If rendererHandledWheelEvent consumes the first event, but the first
non-synthetic/intertial GestureScrollUpdate event does not consume the event,
then the renderer also guarantees that it will not need/consume any future
events.

If my description is accurate, then renaming nextScrollUpdatesUnconsumed_ to
waitingForFirstGestureScrollUpdate_ would make a lot more sense.

[dtapuska, 2016-04-01 18:08:45]

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.mm:134:
firstScrollUnconsumed_ = !consumed;
On 2016/04/01 17:57:34, erikchen wrote:
> On 2016/04/01 17:22:25, dtapuska wrote:
> > On 2016/04/01 16:18:58, erikchen wrote:
> > > I don't understand this logic.
> > > 
> > > Does there exist a mechanism by which the renderer tells the browser: I
will
> > > never consume any more scroll events?
> > > 
> > > The logic on line 565 should reflect that condition.
> > 
> > The condition matches the condition logic on line 119 (which uses
PhaseBegan).
> > In that if the first scroll isn't handled by the page then the history arrow
> is
> > shown. Being that ScrollBegin isn't a blocking event we don't know if it was
> > handled or not. But the first ScrollUpdate will tell us.
> 
> Based on the description you just gave, firstScrollUnconsumed_ should only be
> set on this line.
> 
> Perhaps you meant:
> If rendererHandledWheelEvent does not consume the event, then the renderer
> guarantees that it will not need/consume any future events.
> 
> If rendererHandledWheelEvent consumes the first event, but the first
> non-synthetic/intertial GestureScrollUpdate event does not consume the event,
> then the renderer also guarantees that it will not need/consume any future
> events.
> 
> If my description is accurate, then renaming nextScrollUpdatesUnconsumed_ to
> waitingForFirstGestureScrollUpdate_ would make a lot more sense.

Ah I think I see the problem with your confusion.

There is a setting --enable-wheel-gestures (enabled by default); which will only
call rendererHandledGestureScrollEvent (and never rendererHandledWheelEvent).
And another setting --disable-wheel-gestures; which will cause it never to call
rendererHandledGestureScrollEvent and always rendererHandledWheelEvent..

See the relevant code and a TODO here:
https://codereview.chromium.org/1848063002/patch/1/10005

[dtapuska, 2016-04-01 19:00:25]

PTAL

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
File
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h
(right):

https://codereview.chromium.org/1848063002/diff/1/chrome/browser/renderer_hos...
chrome/browser/renderer_host/chrome_render_widget_host_view_mac_history_swiper.h:158:
// renderer. This variables defaults to NO for new gestures.
On 2016/04/01 17:57:34, erikchen wrote:
> On 2016/04/01 17:22:25, dtapuska wrote:
> > On 2016/04/01 16:18:58, erikchen wrote:
> > > On 2016/04/01 03:44:56, erikchen wrote:
> > > > This comment applies to the previous ivar, which I understood. I don't
> > > > understand the new ivars, nor why you need two rather than just one. 
> > > > 
> > > > Is this the right doc?
> > > >
> > >
> >
>
https://docs.google.com/document/d/19sFYkMUwJcgQTxM1GakrCryHOZpNeL2c41jxWB7rS...
> > > > 
> > > > My understanding:
> > > > 1. The OS sends NSEventPhaseBegin/Moved/Ended events to the browser,
which
> > are
> > > > forwarded to blink.
> > > > 2. Blink converts these to GestureScrollBegin/Update/End events.
> > > > 3. Depending on whether the relevant layer has passive, blocking, or no
> > > > listener, a different message is sent to the main thread. [which thread
is
> > > this
> > > > done on? impl thread?]
> > > > 4. The main thread then forwards the results to the browser.
> > > > 
> > > > If this is accurate, why does the browser need to know about
> > > > GesturescrollEvents? That seems like an implementation detail of Blink.
> The
> > > > embedder passes in a wheel event, and just needs to know whether Blink
> > > consumed
> > > > it.
> > > 
> > > Please add your explanation here, as well as updated comments for these
two
> > > ivars.
> > > 
> > > Based on your explanation, would an ivar named gestureScrollUnconsumed_
make
> > > more sense?
> > 
> > The logic is about the first gesture scroll after the phase began is
> unconsumed.
> > Not that the gesture scroll was unconsumed.
> > 
> > ie; in the example GSB, GSU(A), GSU(B), GSU(C), GSE
> > 
> > GSU(A), GSU(B) could be consumed, but GSU(C) won't trigger the arrow.
> > 
> > But in example: GSB, GSU(X), GSU(Y), GSE
> > 
> > GSU(X) goes unconsumed then it will trigger the arrow.
> 
> I'm sorry, but I don't understand your explanations. You're using the terms
> "won't trigger the arrow" and "trigger the arrow", but I don't know if you're
> describing the previous behavior, or the behavior that you are trying to
induce.
> 
> Can you update the comments for this ivars to describe what they mean?

Done.

[erikchen, 2016-04-01 19:03:46]

lgtm

[tdresser, 2016-04-01 19:08:02]

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

[Alexei Svitkine (slow), 2016-04-01 19:10:04]

rubberstamp owners lgtm based on Erik's review

[Charlie Reis, 2016-04-01 20:48:52]

[+tdresser for real, from James's comment.]

I'm not a good Mac reviewer, so I can just offer a rubber stamp LGTM on
content/public.  

James, are you ok with this landing, or do you want the RVH question resolved
first?

[dtapuska, 2016-04-01 20:53:01]

On 2016/04/01 20:48:52, Charlie Reis (slow til 4-8) wrote:
> [+tdresser for real, from James's comment.]
> 
> I'm not a good Mac reviewer, so I can just offer a rubber stamp LGTM on
> content/public.  
> 
> James, are you ok with this landing, or do you want the RVH question resolved
> first?

Tim was on the review; he talked to me and he had chatted with James offline how
the design works (for wheel gesture events that landed last month) and then he
removed himself from the review because he wasn't necessary.

[wjmaclean, 2016-04-01 20:56:24]

On 2016/04/01 20:53:01, dtapuska wrote:
> On 2016/04/01 20:48:52, Charlie Reis (slow til 4-8) wrote:
> > [+tdresser for real, from James's comment.]
> > 
> > I'm not a good Mac reviewer, so I can just offer a rubber stamp LGTM on
> > content/public.  
> > 
> > James, are you ok with this landing, or do you want the RVH question
resolved
> > first?
> 
> Tim was on the review; he talked to me and he had chatted with James offline
how
> the design works (for wheel gesture events that landed last month) and then he
> removed himself from the review because he wasn't necessary.

I'm ok with this CL landing as-is ... we will need to be sure at some point that
the gestures created from the wheel events are targeted the same as the original
wheel events (which go through RenderWidgetHostInputEventRouter), but this CL
isn't adding anything that is problematic so far as I can tell.

[dtapuska, 2016-04-01 21:51:54]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2016-04-01 21:52:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1848063002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1848063002/20001

[commit-bot: I haz the power, 2016-04-01 22:33:12]

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

[commit-bot: I haz the power, 2016-04-01 22:33:13]

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

[dtapuska, 2016-04-01 22:40:58]

dtapuska@chromium.org changed reviewers:
+ avi@chromium.org

[dtapuska, 2016-04-01 22:41:52]

On 2016/04/01 22:33:13, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

+avi; please stamp the header file. Apparently erik's review of the mm file is
good; but he doesn't have owner ship of the .h files.

[Avi (use Gerrit), 2016-04-01 22:50:16]

lgtm

stampity stamp

[dtapuska, 2016-04-01 23:59:32]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2016-04-01 23:59:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1848063002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1848063002/20001

[commit-bot: I haz the power, 2016-04-02 00:16:16]

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

[commit-bot: I haz the power, 2016-04-02 00:16:17]

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

[dtapuska, 2016-04-02 01:15:41]

Description was changed from

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
==========

to

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org
==========

[dtapuska, 2016-04-02 01:16:15]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2016-04-02 01:16:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1848063002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1848063002/20001

[commit-bot: I haz the power, 2016-04-02 01:23:01]

Description was changed from

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org
==========

to

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org
==========

[commit-bot: I haz the power, 2016-04-02 01:23:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-02 01:23:58]

Description was changed from

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org
==========

to

==========
Fix page navigation being incorrectly fired on Mac OSX.

With wheel gestures being enabled the wheel event goes
unhandled most of the time. There was specific code for Mac
to handle the not handled mouse wheel event that would
fire. Instead base it off gestures when we are using this
setting.

Update unit test to ensure this fires for gestures
correctly.

BUG=599153
TBR=thestig@chromium.org

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

[commit-bot: I haz the power, 2016-04-02 01:23:59]

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

[Lei Zhang, 2016-04-04 23:14:07]

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

[Lei Zhang, 2016-04-04 23:14:08]

I never got an email for this, and I have no idea what part of this CL I'm
suppose to review.

[dtapuska, 2016-04-04 23:26:19]

On 2016/04/04 23:14:08, Lei Zhang wrote:
> I never got an email for this, and I have no idea what part of this CL I'm
> suppose to review.

The owners files don't apply for the Mac_history_shipper.h because the regex for
the owners are *_Mac.h. so I needed a top level owner to stamp. I felt I had
enough owner stamps to tbr you and not block landing it on your review.

[Lei Zhang, 2016-04-05 00:10:04]

On 2016/04/04 23:26:19, dtapuska wrote:
> On 2016/04/04 23:14:08, Lei Zhang wrote:
> > I never got an email for this, and I have no idea what part of this CL I'm
> > suppose to review.
> 
> The owners files don't apply for the Mac_history_shipper.h because the regex
for
> the owners are *_Mac.h. so I needed a top level owner to stamp. I felt I had
> enough owner stamps to tbr you and not block landing it on your review.

I think you mean chrome_render_widget_host_view_mac_history_swiper.h, and sure,
feel free to TBR, because I'm just going to rubberstamp lgtm anyway.

In general, please remember to add the people you TBR as reviewers. Otherwise,
how would they know to review your CL?