Do not interchange MOUSE_MOVED and MOUSE_DRAGGED events in Widget

Do not interchange MOUSE_MOVED and MOUSE_DRAGGED events in Widget

In Widget::OnMouseEvent(), do not handle MOUSE_MOVED
events as MOUSE_DRAGGED events, and do not handle
MOUSE_DRAGGED events as MOUSE_MOVED events; instead
trust that the mouse event type has been set correctly
before arriving at Widget. This also allows us to
remove the member |is_mouse_button_pressed_|
from Widget.

BUG=412929, 412931
TEST=WidgetTest.DragIntoView

[tdanderson, 2014-09-11 15:19:47]

tdanderson@chromium.org changed reviewers:
+ sadrul@chromium.org

[tdanderson, 2014-09-11 15:19:47]

Sadrul, some tests are in progress, but how does this look to you so far? Do you
foresee any problems based on your pre-Win-Aura attempts to do this?

[tdanderson, 2014-09-24 21:19:34]

Sadrul, can you PTAL?

[sadrul, 2014-09-25 16:19:38]

For testing: Do we have enough tests that cover dragging views (e.g. in/out of
bookmark menu, the extension buttons in the toolbar, etc.)?

https://codereview.chromium.org/560053002/diff/100001/ui/views/widget/widget.cc
File ui/views/widget/widget.cc (right):

https://codereview.chromium.org/560053002/diff/100001/ui/views/widget/widget....
ui/views/widget/widget.cc:1189: // Terry - move this out of each case to before
switch?
Sounds like a good plan, yeah.

https://codereview.chromium.org/560053002/diff/100001/ui/views/widget/widget.h
File ui/views/widget/widget.h (right):

https://codereview.chromium.org/560053002/diff/100001/ui/views/widget/widget....
ui/views/widget/widget.h:923: // TODO(beng): Remove NativeWidgetGtk's dependence
on this:
You can just remove this comment. NativeWidgetGtk hasn't existed since forever.

https://codereview.chromium.org/560053002/diff/100001/ui/views/widget/widget....
ui/views/widget/widget.h:932: gfx::Point last_mouse_event_position_;
We should also attempt removing these and see what breaks.

[tdanderson, 2014-09-26 18:49:40]

Sadrul, PTAL.

Regarding testing, it looks as though we do have existing tests for dragging in
and out of views using mouse events. See for example:

bookmark_bar_view_test.cc
tab_drag_controller_interactive_uitest.cc
menu_view_drag_and_drop_test.cc
toolbar_button_test.cc
toolbar_view_interactive_uitest.cc

With that said, do you think my new test WidgetTest.DragOutOfView will be
sufficient for this CL? IIUC, nested message loops are only invoked for specific
types of views that handle drag-and-drop operations.

https://chromiumcodereview.appspot.com/560053002/diff/100001/ui/views/widget/...
File ui/views/widget/widget.cc (right):

https://chromiumcodereview.appspot.com/560053002/diff/100001/ui/views/widget/...
ui/views/widget/widget.cc:1189: // Terry - move this out of each case to before
switch?
On 2014/09/25 16:19:38, sadrul wrote:
> Sounds like a good plan, yeah.

Done.

https://chromiumcodereview.appspot.com/560053002/diff/100001/ui/views/widget/...
File ui/views/widget/widget.h (right):

https://chromiumcodereview.appspot.com/560053002/diff/100001/ui/views/widget/...
ui/views/widget/widget.h:923: // TODO(beng): Remove NativeWidgetGtk's dependence
on this:
On 2014/09/25 16:19:38, sadrul wrote:
> You can just remove this comment. NativeWidgetGtk hasn't existed since
forever.

Done.

https://chromiumcodereview.appspot.com/560053002/diff/100001/ui/views/widget/...
ui/views/widget/widget.h:932: gfx::Point last_mouse_event_position_;
On 2014/09/25 16:19:38, sadrul wrote:
> We should also attempt removing these and see what breaks.

Agreed, I will try that in a follow-up CL. If I were a betting man, I'd put my
money on something breaking on Windows here.

[sadrul, 2014-09-26 20:59:13]

LGTM

https://codereview.chromium.org/560053002/diff/140001/ui/views/view_unittest.cc
File ui/views/view_unittest.cc (right):

https://codereview.chromium.org/560053002/diff/140001/ui/views/view_unittest....
ui/views/view_unittest.cc:371: root->OnMouseReleased(released);
whoa

https://codereview.chromium.org/560053002/diff/140001/ui/views/widget/widget_...
File ui/views/widget/widget_unittest.cc (right):

https://codereview.chromium.org/560053002/diff/140001/ui/views/widget/widget_...
ui/views/widget/widget_unittest.cc:3268: EXPECT_EQ(0,
container->GetEventCount(ui::ET_MOUSE_RELEASED));
Should we be sending an EXIT event here?

[tdanderson, 2014-09-29 18:38:13]

Just did some manual testing with this CL on Windows and saw no obvious
problems, so I'm going to commit.

https://chromiumcodereview.appspot.com/560053002/diff/140001/ui/views/widget/...
File ui/views/widget/widget_unittest.cc (right):

https://chromiumcodereview.appspot.com/560053002/diff/140001/ui/views/widget/...
ui/views/widget/widget_unittest.cc:3268: EXPECT_EQ(0,
container->GetEventCount(ui::ET_MOUSE_RELEASED));
On 2014/09/26 20:59:13, sadrul wrote:
> Should we be sending an EXIT event here?

From what I can see, we don't send an EXIT on ToT so I didn't expect to be doing
so with this CL. Do you think this is incorrect?

[tdanderson, 2014-09-29 18:38:19]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-09-29 18:38:41]

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

[commit-bot: I haz the power, 2014-09-29 18:43:45]

Committed patchset #8 (id:140001) as bb96ba92657cad0140a30983f2bfe72ed47f4dc5

[commit-bot: I haz the power, 2014-09-29 18:44:36]

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

[tdanderson, 2014-11-07 18:28:26]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/703643004/ by tdanderson@chromium.org.

The reason for reverting is: Responsible for the crash reported in
crbug.com/430427. Relanding will require addressing crbug.com/431350 first..