Dismiss the multi window resizer when the user clicks outside of the resizer

Dismiss the multi window resizer when the user clicks outside of the multi
window resizer

BUG=437125
TEST=MultiWindowResizeController.ClickOutside

Committed: https://crrev.com/5159609e9574b27da51ef18631d6c05a0824fbf3
Cr-Commit-Position: refs/heads/master@{#308402}

[pkotwicz, 2014-12-04 22:46:44]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-12-04 22:59:46]

pkotwicz@chromium.org changed reviewers:
+ flackr@chromium.org

[pkotwicz, 2014-12-04 22:59:47]

Rob, can you please take a look?
This CL depends on https://codereview.chromium.org/747343004/

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.cc
File ui/views/mouse_watcher.cc (right):

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:42: case ui::ET_MOUSE_PRESSED:
Yes, we should check ET_GESTURE_TAP_DOWN. The conversion from the
"stacked/touch" tabstrip to the normal tabstrip is buggy. Checking
ET_GESTURE_TAP_DOWN here may make that behavior more buggy

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:68: : mouse_watcher_->notify_on_exit_time_);
We could probably always use the same time. kNotifyOnExitTimeMS in
download_shelf_view.cc is not used the way the comment says it is. I have not
found a useful scenario in which kNotifyOnExitTimeMS is used.

[pkotwicz, 2014-12-08 15:15:23]

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

[pkotwicz, 2014-12-08 15:16:22]

flackr@ seems OOO. Oshima can you please take a look?

[flackr, 2014-12-09 14:29:40]

I was just OOO for a couple days, back now.

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.cc
File ui/views/mouse_watcher.cc (right):

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:68: : mouse_watcher_->notify_on_exit_time_);
On 2014/12/04 22:59:47, pkotwicz wrote:
> We could probably always use the same time. kNotifyOnExitTimeMS in
> download_shelf_view.cc is not used the way the comment says it is. I have not
> found a useful scenario in which kNotifyOnExitTimeMS is used.

Yeah, I can't seem to trigger the kNotifyListenerTimeMs for the download shelf.
This could be a regression though, possibly dating as far back as the transition
to aura. The shelf is in its own window right? So the first time the mouse would
leave it would be on an exit event which would trigger the long timer.

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h
File ui/views/mouse_watcher.h (right):

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h...
ui/views/mouse_watcher.h:32: // The type of mouse event.
If we're just going to specify the type of mouse event then why not just pass
the ui::EventType? Alternately, I could see adding a MOUSE_ACTION value to
represent an action which isn't strictly movement.

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h...
ui/views/mouse_watcher.h:37: // Click or gesture tap
This doesn't actually handle gesture tap though does it? If not, remove tap from
comment until it does. If it does, this should really be more of a generic
EventType or EventHint if it still represents a translation of the event type.

[oshima, 2014-12-09 19:25:14]

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

[oshima, 2014-12-09 19:25:23]

removing myself

[pkotwicz, 2014-12-09 21:32:47]

Rob, can you please take another look?

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.cc
File ui/views/mouse_watcher.cc (right):

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:68: : mouse_watcher_->notify_on_exit_time_);
It seems that clicking on the download item to open a download triggers the long
hide delay for the download shelf but that opening a download via the download
item's menu triggers the short hide delay.

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h
File ui/views/mouse_watcher.h (right):

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h...
ui/views/mouse_watcher.h:32: // The type of mouse event.
If one day we want to take into account gesture taps (there is suboptimal
behavior because we do not do so), it will be nice to have a separate enum and
not use ui::EventType

https://codereview.chromium.org/753933003/diff/20001/ui/views/mouse_watcher.h...
ui/views/mouse_watcher.h:37: // Click or gesture tap
Removed

[flackr, 2014-12-09 21:37:22]

LGTM

[pkotwicz, 2014-12-10 06:16:27]

pkotwicz@chromium.org changed reviewers:
+ sky@chromium.org

[pkotwicz, 2014-12-10 06:16:28]

sky@ for OWNERS

[sky, 2014-12-10 15:51:36]

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.cc
File ui/views/mouse_watcher.cc (right):

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:58: // Mouse moved outside the host's zone, start a
timer to notify the
Update comment.

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:61: NotifyListener();
Did you look at all listeners and make sure they really want to be notified on
mouse press immediately?

[pkotwicz, 2014-12-10 18:34:51]

Patchset #3 (id:60001) has been deleted

[pkotwicz, 2014-12-10 18:38:28]

Scott, can you please take another look?

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.cc
File ui/views/mouse_watcher.cc (right):

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:58: // Mouse moved outside the host's zone, start a
timer to notify the
I have moved the comment into the else statement

https://codereview.chromium.org/753933003/diff/40001/ui/views/mouse_watcher.c...
ui/views/mouse_watcher.cc:61: NotifyListener();
I have not found any cases where notifying the listener on mouse press
immediately would cause harm.
It is slightly useful in the context of
chrome/browser/ui/views/autofill/tooltip_icon.cc

[sky, 2014-12-10 22:00:58]

LGTM

[pkotwicz, 2014-12-15 18:12:31]

Patchset #4 (id:100001) has been deleted

[pkotwicz, 2014-12-15 18:13:17]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-12-15 18:14:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/753933003/120001

[commit-bot: I haz the power, 2014-12-15 20:05:53]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2014-12-15 20:06:45]

Patchset 4 (id:??) landed as
https://crrev.com/5159609e9574b27da51ef18631d6c05a0824fbf3
Cr-Commit-Position: refs/heads/master@{#308402}