Only drag or maximize / restore when event has not yet been handled.

Only drag or maximize / restore when event has not yet been handled.

BUG=332295
TEST=ToplevelWindowEventHandlerTest.DontDragIfModalChild, WorkspaceEventHandlerTest.DoubleClickOrTapWithModalChildDoesntMaximize
TEST=Press Ctrl+O and then try to double click or drag the browser window. It should not move or maximize.

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

[flackr, 2014-01-21 18:22:31]

PTAL, thanks. This CL prevents dragging / resizing / maximizing / restoring
parents of modal transient windows by checking the event's handled attribute
before beginning these interactions.

[oshima, 2014-01-21 22:14:53]

On 2014/01/21 18:22:31, flackr wrote:
> PTAL, thanks. This CL prevents dragging / resizing / maximizing / restoring
> parents of modal transient windows by checking the event's handled attribute
> before beginning these interactions.

Hmm, this looks fragile. How I can tell if a given method should check
event->handled() when I changing / adding event handler?
(ToplevelWindowEventHnalder::OnMouseEvent only check for pressed , while
WorkspaceEventHandler::OnMouseEvent checks for all events, for example)

Looking at the original change, I wonder if there is a better way to handle the
issue,
say by letting modality controller knows about overview mode?

[pkotwicz, 2014-01-21 22:22:39]

https://codereview.chromium.org/144193002/diff/60001/ash/wm/toplevel_window_e...
File ash/wm/toplevel_window_event_handler.cc (right):

https://codereview.chromium.org/144193002/diff/60001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:391: event->StopPropagation();
Can you do me a favor and only call StopPropagation() here if we have a
|window_resizer_| ?

[flackr, 2014-01-29 15:27:31]

It's perfectly safe to always ignore handled events, which should be less
fragile. This will behave exactly the same as it was before when StopPropagation
was called and ToplevelWindowEventHandler wouldn't even see the events. WDYT?

We can change this again if/when we redesign the event dispatching.

https://codereview.chromium.org/144193002/diff/60001/ash/wm/toplevel_window_e...
File ash/wm/toplevel_window_event_handler.cc (right):

https://codereview.chromium.org/144193002/diff/60001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:391: event->StopPropagation();
On 2014/01/21 22:22:39, pkotwicz wrote:
> Can you do me a favor and only call StopPropagation() here if we have a
> |window_resizer_| ?

Done.

[oshima, 2014-01-29 20:02:22]

thanks, this looks better, lgtm.

https://codereview.chromium.org/144193002/diff/120001/ash/wm/workspace/worksp...
File ash/wm/workspace/workspace_event_handler_unittest.cc (right):

https://codereview.chromium.org/144193002/diff/120001/ash/wm/workspace/worksp...
ash/wm/workspace/workspace_event_handler_unittest.cc:253:
DoubleClickOrTapWithModalChildDoesntMaximize) {
nit: indent

[flackr, 2014-01-30 15:16:26]

https://codereview.chromium.org/144193002/diff/120001/ash/wm/workspace/worksp...
File ash/wm/workspace/workspace_event_handler_unittest.cc (right):

https://codereview.chromium.org/144193002/diff/120001/ash/wm/workspace/worksp...
ash/wm/workspace/workspace_event_handler_unittest.cc:253:
DoubleClickOrTapWithModalChildDoesntMaximize) {
On 2014/01/29 20:02:22, oshima wrote:
> nit: indent

Done, and the other instances in this file.

[commit-bot: I haz the power, 2014-01-30 15:18:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/144193002/160001

[commit-bot: I haz the power, 2014-01-30 16:38:59]

Retried try job too often on win_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-30 16:52:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/144193002/160001

[commit-bot: I haz the power, 2014-01-30 20:06:37]

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

[commit-bot: I haz the power, 2014-01-30 20:06:39]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-30 20:16:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/flackr@chromium.org/144193002/160001

[commit-bot: I haz the power, 2014-01-30 20:46:57]

Change committed as 248022

[commit-bot: I haz the power, 2014-01-30 20:47:03]

CQ bit was unchecked on CL. Ignoring.