Do not dispatch gesture events to disabled views

Do not dispatch gesture events to disabled views

The current logic in RootView::DispatchGestureEvent() will dispatch
gesture events to disabled views in some circumstances but not others.
The correct behaviour here is:

* Disabled views may be the target of a gesture event.
* Gesture events should never be dispatched to a disabled view,
  and instead the event should simply be marked as handled.

This prevents context menus being shown when long-pressing
disabled textfields (issue 337568).

BUG=403893, 337568
TEST=WidgetTest.DisabledGestureEventTarget,
     RootViewTest.ContextMenuFromLongPress,
     RootViewTest.ContextMenuFromLongPressOnDisabledView

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

[tdanderson, 2014-08-15 18:22:00]

Sadrul, can you please take a look? Let's wait until early M39 to land this,
though.

[sadrul, 2014-08-15 18:58:52]

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc
File ui/views/widget/root_view.cc (right):

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc...
ui/views/widget/root_view.cc:645: event->SetHandled();
the indent here seems off

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc...
ui/views/widget/root_view.cc:655: }
Can you combine this block with the code below in lines 671:679? something like:

  if (gesture_handler_->enabled()) {
    <dispatch lines 660:669>
    <update |event| from |handler_event| lines 681:686 without returns>
  } else {
    <mark event handled>
  }

  if (event->type() == END) {
  }

[tdanderson, 2014-08-15 20:21:51]

Code cleaned up. Can you PTAL?

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc
File ui/views/widget/root_view.cc (right):

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc...
ui/views/widget/root_view.cc:645: event->SetHandled();
On 2014/08/15 18:58:52, sadrul wrote:
> the indent here seems off

Done.

https://codereview.chromium.org/472303004/diff/1/ui/views/widget/root_view.cc...
ui/views/widget/root_view.cc:655: }
On 2014/08/15 18:58:52, sadrul wrote:
> Can you combine this block with the code below in lines 671:679? something
like:
> 
>   if (gesture_handler_->enabled()) {
>     <dispatch lines 660:669>
>     <update |event| from |handler_event| lines 681:686 without returns>
>   } else {
>     <mark event handled>
>   }
> 
>   if (event->type() == END) {
>   }

Done. (I didn't do this originally because the plan is to just rip all of this
code out soon anyway.)

[sadrul, 2014-08-15 21:11:43]

lgtm

[tdanderson, 2014-08-16 17:31:48]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-08-16 17:32:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdanderson@chromium.org/472303004/20001

[tdanderson, 2014-08-16 17:33:55]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-08-16 17:46:18]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-08-16 17:46:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdanderson@chromium.org/472303004/20001

[tdanderson, 2014-08-16 19:52:16]

The CQ bit was unchecked by tdanderson@chromium.org

[tdanderson, 2014-08-16 19:52:17]

The CQ bit was checked by tdanderson@chromium.org

[commit-bot: I haz the power, 2014-08-16 19:52:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tdanderson@chromium.org/472303004/20001

[commit-bot: I haz the power, 2014-08-16 20:47:58]

Committed patchset #2 (20001) as 290159