CrOS - Make it possible to Alt+Tab while dragging a tab

CrOS - Make it possible to Alt+Tab while dragging a tab
without interfering with the drag and drop operation.

The Alt+Tab UI no longer does a mouse capture, and instead relies on event filters.

BUG=660945
Review-Url: https://codereview.chromium.org/2642273002
Cr-Commit-Position: refs/heads/master@{#445851}
Committed: https://chromium.googlesource.com/chromium/src/+/2080dad23aec7f5225154092b527acc4737f261c

[Evan Stade, 2017-01-20 01:07:47]

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

[Evan Stade, 2017-01-20 01:08:30]

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

[commit-bot: I haz the power, 2017-01-20 01:09:10]

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

[Evan Stade, 2017-01-20 01:11:31]

estade@chromium.org changed reviewers:
+ oshima@chromium.org

[Evan Stade, 2017-01-20 01:11:32]

Side note: crrev.com/3be101d020da2ce8eb7c0ab is no longer necessary for the
Alt+Tab case but is still needed for the screenshot-while-dragging-a-tab case.

[Evan Stade, 2017-01-20 01:13:01]

Description was changed from

==========
CrOS - Make it possible to Alt+Tab while dragging a tab
without interfering with the drag and drop operation.

BUG=660945
==========

to

==========
CrOS - Make it possible to Alt+Tab while dragging a tab
without interfering with the drag and drop operation.

The Alt+Tab UI no longer does a mouse capture, and instead relies on event
filters.

BUG=660945
==========

[commit-bot: I haz the power, 2017-01-20 02:08:38]

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

[commit-bot: I haz the power, 2017-01-20 02:08:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2017-01-20 19:27:44]

can you look into test failures?

https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
File ash/wm/window_cycle_event_filter_aura.cc (right):

https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
this may prevent a mouse from moving to another display. can you test 
on linux+x11 desktop by using

--ash-host-window-bounds=1000x500,0+500-1000x500 --ash-constrain-pointer-to-root

?

[Evan Stade, 2017-01-21 01:04:45]

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

[commit-bot: I haz the power, 2017-01-21 01:05:14]

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

[Evan Stade, 2017-01-21 01:05:31]

tests should be fixed

https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
File ash/wm/window_cycle_event_filter_aura.cc (right):

https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
On 2017/01/20 19:27:44, oshima wrote:
> this may prevent a mouse from moving to another display. can you test 
> on linux+x11 desktop by using
> 
> --ash-host-window-bounds=1000x500,0+500-1000x500
--ash-constrain-pointer-to-root
> 
> ?

What should I be looking for? The pointer gets hard to move from one display to
another when I use that command line, regardless of whether I'm using Alt+Tab,
but the behavior doesn't seem to be different with or without alt+tab.

I think it's reasonable to prevent the mouse from moving between displays while
Alt+Tabbing. You can't do anything with the mouse anyway except continue a drag
you've already started, and there'd be no reason to Alt+Tab while dragging from
one display to another.

[commit-bot: I haz the power, 2017-01-21 02:09:08]

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

[commit-bot: I haz the power, 2017-01-21 02:09:09]

Dry run: This issue passed the CQ dry run.

[oshima, 2017-01-23 22:44:27]

On 2017/01/21 01:05:31, Evan Stade wrote:
> tests should be fixed
> 
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> File ash/wm/window_cycle_event_filter_aura.cc (right):
> 
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
> On 2017/01/20 19:27:44, oshima wrote:
> > this may prevent a mouse from moving to another display. can you test 
> > on linux+x11 desktop by using
> > 
> > --ash-host-window-bounds=1000x500,0+500-1000x500
> --ash-constrain-pointer-to-root
> > 
> > ?
> 
> What should I be looking for? The pointer gets hard to move from one display
to
> another when I use that command line, regardless of whether I'm using Alt+Tab,
> but the behavior doesn't seem to be different with or without alt+tab.
> 
> I think it's reasonable to prevent the mouse from moving between displays
while
> Alt+Tabbing. You can't do anything with the mouse anyway except continue a
drag
> you've already started, and there'd be no reason to Alt+Tab while dragging
from
> one display to another.

All we need is to add ET_MOUSE_MOVE (in addition to DRAGGED, DRAG_RELEASE). I
believe it's harmless?

By the way, I tried the repro step on linux desktop, but pressing ALT key
dropped the window.
Is it just on a linux desktop?

[Evan Stade, 2017-01-23 23:49:22]

On 2017/01/23 22:44:27, oshima wrote:
> On 2017/01/21 01:05:31, Evan Stade wrote:
> > tests should be fixed
> > 
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > File ash/wm/window_cycle_event_filter_aura.cc (right):
> > 
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
> > On 2017/01/20 19:27:44, oshima wrote:
> > > this may prevent a mouse from moving to another display. can you test 
> > > on linux+x11 desktop by using
> > > 
> > > --ash-host-window-bounds=1000x500,0+500-1000x500
> > --ash-constrain-pointer-to-root
> > > 
> > > ?
> > 
> > What should I be looking for? The pointer gets hard to move from one display
> to
> > another when I use that command line, regardless of whether I'm using
Alt+Tab,
> > but the behavior doesn't seem to be different with or without alt+tab.
> > 
> > I think it's reasonable to prevent the mouse from moving between displays
> while
> > Alt+Tabbing. You can't do anything with the mouse anyway except continue a
> drag
> > you've already started, and there'd be no reason to Alt+Tab while dragging
> from
> > one display to another.
> 
> All we need is to add ET_MOUSE_MOVE (in addition to DRAGGED, DRAG_RELEASE). I
> believe it's harmless?

I still don't understand what the bug is here. I don't think it's best to let
ET_MOUSE_MOVE through because then hover effects on buttons, tabs, etc. show,
even though clicking does nothing (as intended).

> 
> By the way, I tried the repro step on linux desktop, but pressing ALT key
> dropped the window.
> Is it just on a linux desktop?

Yes, your DWM is probably to blame. For example it might eat Alt+Tab or
Alt+click. When working on this I frequently have to change my WM's hotkeys
(e.g. switching the mod key from alt to win).

[oshima, 2017-01-24 21:09:35]

lgtm


On 2017/01/23 23:49:22, Evan Stade wrote:
> On 2017/01/23 22:44:27, oshima wrote:
> > On 2017/01/21 01:05:31, Evan Stade wrote:
> > > tests should be fixed
> > > 
> > >
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > > File ash/wm/window_cycle_event_filter_aura.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > > ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
> > > On 2017/01/20 19:27:44, oshima wrote:
> > > > this may prevent a mouse from moving to another display. can you test 
> > > > on linux+x11 desktop by using
> > > > 
> > > > --ash-host-window-bounds=1000x500,0+500-1000x500
> > > --ash-constrain-pointer-to-root
> > > > 
> > > > ?
> > > 
> > > What should I be looking for? The pointer gets hard to move from one
display
> > to
> > > another when I use that command line, regardless of whether I'm using
> Alt+Tab,
> > > but the behavior doesn't seem to be different with or without alt+tab.
> > > 
> > > I think it's reasonable to prevent the mouse from moving between displays
> > while
> > > Alt+Tabbing. You can't do anything with the mouse anyway except continue a
> > drag
> > > you've already started, and there'd be no reason to Alt+Tab while dragging
> > from
> > > one display to another.
> > 
> > All we need is to add ET_MOUSE_MOVE (in addition to DRAGGED, DRAG_RELEASE).
I
> > believe it's harmless?

Actually, looking into the new ozone path, this seems to be handled differently
now so
mouse warp works correctly. Sorry about false claim.

> I still don't understand what the bug is here. I don't think it's best to let
> ET_MOUSE_MOVE through because then hover effects on buttons, tabs, etc. show,
> even though clicking does nothing (as intended).

Just FYI, passing through DRAG events can cause these effect as well.
I don't think that's a big issue though.

> 
> > 
> > By the way, I tried the repro step on linux desktop, but pressing ALT key
> > dropped the window.
> > Is it just on a linux desktop?
> 
> Yes, your DWM is probably to blame. For example it might eat Alt+Tab or
> Alt+click. When working on this I frequently have to change my WM's hotkeys
> (e.g. switching the mod key from alt to win).

One thing I noticed while testing was that attaching tab does remove the
dragging window from the cycle list,
but detaching again does not restore it. I don't think that's a big issue either
though.

[Evan Stade, 2017-01-24 22:11:27]

On 2017/01/24 21:09:35, oshima wrote:
> lgtm
> 
> 
> On 2017/01/23 23:49:22, Evan Stade wrote:
> > On 2017/01/23 22:44:27, oshima wrote:
> > > On 2017/01/21 01:05:31, Evan Stade wrote:
> > > > tests should be fixed
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > > > File ash/wm/window_cycle_event_filter_aura.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2642273002/diff/20001/ash/wm/window_cycle_eve...
> > > > ash/wm/window_cycle_event_filter_aura.cc:61: event->StopPropagation();
> > > > On 2017/01/20 19:27:44, oshima wrote:
> > > > > this may prevent a mouse from moving to another display. can you test 
> > > > > on linux+x11 desktop by using
> > > > > 
> > > > > --ash-host-window-bounds=1000x500,0+500-1000x500
> > > > --ash-constrain-pointer-to-root
> > > > > 
> > > > > ?
> > > > 
> > > > What should I be looking for? The pointer gets hard to move from one
> display
> > > to
> > > > another when I use that command line, regardless of whether I'm using
> > Alt+Tab,
> > > > but the behavior doesn't seem to be different with or without alt+tab.
> > > > 
> > > > I think it's reasonable to prevent the mouse from moving between
displays
> > > while
> > > > Alt+Tabbing. You can't do anything with the mouse anyway except continue
a
> > > drag
> > > > you've already started, and there'd be no reason to Alt+Tab while
dragging
> > > from
> > > > one display to another.
> > > 
> > > All we need is to add ET_MOUSE_MOVE (in addition to DRAGGED,
DRAG_RELEASE).
> I
> > > believe it's harmless?
> 
> Actually, looking into the new ozone path, this seems to be handled
differently
> now so
> mouse warp works correctly. Sorry about false claim.
> 
> > I still don't understand what the bug is here. I don't think it's best to
let
> > ET_MOUSE_MOVE through because then hover effects on buttons, tabs, etc.
show,
> > even though clicking does nothing (as intended).
> 
> Just FYI, passing through DRAG events can cause these effect as well.
> I don't think that's a big issue though.
> 
> > 
> > > 
> > > By the way, I tried the repro step on linux desktop, but pressing ALT key
> > > dropped the window.
> > > Is it just on a linux desktop?
> > 
> > Yes, your DWM is probably to blame. For example it might eat Alt+Tab or
> > Alt+click. When working on this I frequently have to change my WM's hotkeys
> > (e.g. switching the mod key from alt to win).
> 
> One thing I noticed while testing was that attaching tab does remove the
> dragging window from the cycle list,
> but detaching again does not restore it. I don't think that's a big issue
either
> though.

Yea I think that's true in general; we handle destroyed windows but not ones
that are added during the Alt+Tab session. This may be the easiest way to
demonstrate that oddity. Perhaps a good follow up would be to not show dragging
windows in Alt+Tab in the first place.

[Evan Stade, 2017-01-24 22:11:33]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-01-24 22:12: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, 2017-01-24 23:01:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485295893724160,
"parent_rev": "1bd1cf4ed15972f6510819f0d9cf62da968ef1b1", "commit_rev":
"2080dad23aec7f5225154092b527acc4737f261c"}

[commit-bot: I haz the power, 2017-01-24 23:01:46]

Description was changed from

==========
CrOS - Make it possible to Alt+Tab while dragging a tab
without interfering with the drag and drop operation.

The Alt+Tab UI no longer does a mouse capture, and instead relies on event
filters.

BUG=660945
==========

to

==========
CrOS - Make it possible to Alt+Tab while dragging a tab
without interfering with the drag and drop operation.

The Alt+Tab UI no longer does a mouse capture, and instead relies on event
filters.

BUG=660945

Review-Url: https://codereview.chromium.org/2642273002
Cr-Commit-Position: refs/heads/master@{#445851}
Committed:
https://chromium.googlesource.com/chromium/src/+/2080dad23aec7f5225154092b527...
==========

[commit-bot: I haz the power, 2017-01-24 23:01:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2080dad23aec7f5225154092b527...