[Mac] Don't ignore mouse events outside of the view while dragging.

[Mac] Don't ignore mouse events outside of the view while dragging.

Previously, we ignored mouse events atop of another RenderWidgetHostView
while dragging, which lead to weird behavior.

BUG=319879
TEST=Open DevTools docked to right, scroll page by dragging the thumb,
move mouse atop the DevTools. Page should still be scrolled. Observe
the same while moving mouse out of the window.

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

[dgozman, 2013-11-28 15:44:04]

Nico, may you have a look please?

[Nico, 2013-11-29 23:01:59]

+asvitkine who authored https://codereview.chromium.org/6676094 . The code in
question touched in this CL is from
https://chromiumcodereview.appspot.com/15315005

If we have overlapping rwhvs (e.g. instant – but come to think of it, I'm not
sure if the instant popup is still an rwhv these days) and one is clickdragged,
will we send mouse move events to two rwhvs now?

[dgozman, 2013-11-29 23:25:41]

On 2013/11/29 23:01:59, Nico wrote:
> +asvitkine who authored https://codereview.chromium.org/6676094 . The code in
> question touched in this CL is from
> https://chromiumcodereview.appspot.com/15315005
> 
> If we have overlapping rwhvs (e.g. instant – but come to think of it, I'm not
> sure if the instant popup is still an rwhv these days) and one is
clickdragged,
> will we send mouse move events to two rwhvs now?
My testing on DevTools shown that we receive mouse events only for one rwhv
while dragging, and we just ignore them in this check.

[Alexei Svitkine (slow), 2013-11-29 23:51:19]

On 2013/11/29 23:25:41, dgozman wrote:
> On 2013/11/29 23:01:59, Nico wrote:
> > +asvitkine who authored https://codereview.chromium.org/6676094 . The code
in
> > question touched in this CL is from
> > https://chromiumcodereview.appspot.com/15315005
> > 
> > If we have overlapping rwhvs (e.g. instant – but come to think of it, I'm
not
> > sure if the instant popup is still an rwhv these days) and one is
> clickdragged,
> > will we send mouse move events to two rwhvs now?
> My testing on DevTools shown that we receive mouse events only for one rwhv
> while dragging, and we just ignore them in this check.

The original reason for that code is so that moving the mouse over the find bar
doesn't send events to the web view. What happens with your CL if you bring up
the find bar (on a page that has e.g. links underneath it like codereview) and
drag something over that?

[Alexei Svitkine (slow), 2013-11-30 17:44:30]

Looking at the linked bug, the problem seems like it may be more general than
this specific case (overlapping views). Wouldn't the same thing happen if your
mouse moves out of the window completely? If it does happen in that case, then a
different solution is needed. If it doesn't happen in that case, then what
happens differently (and how can we make the overlapping view case behave the
same way as far as Blink is concerned.)

[dgozman, 2013-12-02 11:51:28]

On 2013/11/30 17:44:30, Alexei Svitkine wrote:
> Looking at the linked bug, the problem seems like it may be more general than
> this specific case (overlapping views). Wouldn't the same thing happen if your
> mouse moves out of the window completely? If it does happen in that case, then
a
> different solution is needed. If it doesn't happen in that case, then what
> happens differently (and how can we make the overlapping view case behave the
> same way as far as Blink is concerned.)

While dragging, we only get events for the render view where drag started. Even
if you move mouse outside the window or atop of another view, overlapping or
not, events still come to the original view. Probably, something in NSView does
this for us. But we artificially ignore such events in some cases, which is what
I'm fixing in this CL.

Played with a find bar, it takes events instead of the page underneath. You can
drag some text into find bar, and clicking on it does not trigger links on the
page. This looks like a desired behavior, doesn't it?
You can also drag text to another render view (i.e. DevTools). This works with
and without my patch.

Overall, this change seems to only improve the situation, not regressing
anything.

[Alexei Svitkine (slow), 2013-12-02 14:35:48]

On 2013/12/02 11:51:28, dgozman wrote:
> On 2013/11/30 17:44:30, Alexei Svitkine wrote:
> > Looking at the linked bug, the problem seems like it may be more general
than
> > this specific case (overlapping views). Wouldn't the same thing happen if
your
> > mouse moves out of the window completely? If it does happen in that case,
then
> a
> > different solution is needed. If it doesn't happen in that case, then what
> > happens differently (and how can we make the overlapping view case behave
the
> > same way as far as Blink is concerned.)
> 
> While dragging, we only get events for the render view where drag started.
Even
> if you move mouse outside the window or atop of another view, overlapping or
> not, events still come to the original view. Probably, something in NSView
does
> this for us. But we artificially ignore such events in some cases, which is
what
> I'm fixing in this CL.
> 
> Played with a find bar, it takes events instead of the page underneath. You
can
> drag some text into find bar, and clicking on it does not trigger links on the
> page. This looks like a desired behavior, doesn't it?
> You can also drag text to another render view (i.e. DevTools). This works with
> and without my patch.
> 
> Overall, this change seems to only improve the situation, not regressing
> anything.

Cool! Can you expand the comment you're adding to mention some of this?

Also, please add a TEST= line mentioning the manual steps to test the bug this
is fixing.

LGTM modulo above

[dgozman, 2013-12-02 15:07:06]

> Cool! Can you expand the comment you're adding to mention some of this?
> 
> Also, please add a TEST= line mentioning the manual steps to test the bug this
> is fixing.
> 
> LGTM modulo above

Thank you for the review. I've added TEST= and changed the comment.

[commit-bot: I haz the power, 2013-12-02 15:08:44]

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

[commit-bot: I haz the power, 2013-12-02 15:28:24]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Avi (use Gerrit), 2013-12-02 15:31:34]

Alexei isn't an owner so his LGTM isn't enough. Mine is :)

[commit-bot: I haz the power, 2013-12-02 15:31:49]

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

[commit-bot: I haz the power, 2013-12-02 20:51:35]

Change committed as 238177