[M-37]: Fix dragging tab via mouse when touch is down on ChromeOS

[M-37]: Fix dragging tab via mouse when touch is down on ChromeOS

Do not change value of |ToplevelWindowEventHandler::drag_reverted_| if
ET_MOUSE_CAPTURE_CHANGED is received after ET_MOUSE_RELEASED but before
the move loop has ended. This is currently the case with views::Widget.

This CL does not fix all of the possible places where CompleteDrag() may be
called after the drag was completed for the sake of being easier to merge
back to M-37

BUG=380481
TEST=ToplevelWindowEventHandlerTest.CaptureLossAfterMouseRelease

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

[pkotwicz, 2014-06-28 18:37:36]

Oshima, can you please take a look?

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

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:497: if (window_resizer_) {
We should also avoid calling CompleteDrag() here if the drag has already been
completed. I will fix this up as part of another CL.

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:508: !target->IsVisible()) {
This code never gets run. (That's as a result of a bad suggestion on a CL on my
part.) I am going to fix it up in a different CL.

[pkotwicz, 2014-06-28 18:41:27]

Oshima, can you please take a look?

[pkotwicz, 2014-06-29 16:00:40]

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

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:486: } else {
We should also avoid calling CompleteDrag() here if the drag has already been
completed. I will fix this up as part of another CL.

[oshima, 2014-07-01 17:52:01]

lgtm with one q

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

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:486: } else {
On 2014/06/29 16:00:40, pkotwicz wrote:
> We should also avoid calling CompleteDrag() here if the drag has already been
> completed. I will fix this up as part of another CL.

Why another CL? Don't we want to fix them all and merge it?

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
File ash/wm/toplevel_window_event_handler_unittest.cc (right):

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler_unittest.cc:709: namespace {
I personally don't mind to have this here, but this file already has the
anonymous namespace, in case you didn't notice.

[pkotwicz, 2014-07-04 14:28:41]

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

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:486: } else {
I wanted to keep the CL to be merged as small as possible

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
File ash/wm/toplevel_window_event_handler_unittest.cc (right):

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler_unittest.cc:709: namespace {
I prefer having functions which are tightly coupled with the test right above
the test. tab_drag_controller_interactive_uitest.cc follows this pattern. It
also has an anonymous namespace at the top of the file.

[pkotwicz, 2014-07-04 14:28:43]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-07-04 14:29:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/354343002/40001

[commit-bot: I haz the power, 2014-07-04 16:18:49]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[pkotwicz, 2014-07-04 20:41:16]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-07-04 20:41:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pkotwicz@chromium.org/354343002/60001

[commit-bot: I haz the power, 2014-07-04 22:21:32]

Change committed as 281448

[varkha, 2014-07-04 23:09:01]

Sorry, just noticed that this nit comment was drafted but not sent. Please see
if it makes sense.

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

https://codereview.chromium.org/354343002/diff/40001/ash/wm/toplevel_window_e...
ash/wm/toplevel_window_event_handler.cc:497: if (window_resizer_) {
Should this method just return early when !window_resizer_? This would simplify
the logic here and below.

[pkotwicz, 2014-07-05 16:04:10]

Valery, please see https://codereview.chromium.org/345133005/ which is part #2