linux_aura: Fix crash on drags from the omnibox.

linux_aura: Fix crash on drags from the omnibox.

When we receive a mouse move event, there may be more events behind it
in the queue. Prevent those further mouse events from also triggering a
nested drag message loop.

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

[Elliot Glaysher, 2013-08-14 21:15:55]

https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc#newcode2329
ui/views/view.cc:2329: currently_dragging_ = true;
I vaguely remember us having some sort of scoped bool setter, but I can't seem
to find it. If you know what the class name is, I'd be happy to use it.

[sadrul, 2013-08-14 21:33:51]

+varunjain@

I looked for some other way for a View to decide that a drag is already in
progress. I seem to remember there was some such state maintained somewhere, but
I can't find it. Varun, do you know?

https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc#newcode2329
ui/views/view.cc:2329: currently_dragging_ = true;
On 2013/08/14 21:15:55, Elliot Glaysher wrote:
> I vaguely remember us having some sort of scoped bool setter, but I can't seem
> to find it. If you know what the class name is, I'd be happy to use it.

I think you mean base::AutoReset.

[varunjain, 2013-08-14 22:25:16]

On 2013/08/14 21:33:51, sadrul wrote:
> +varunjain@
> 
> I looked for some other way for a View to decide that a drag is already in
> progress. I seem to remember there was some such state maintained somewhere,
but
> I can't find it. Varun, do you know?

There is aura specific aura::client::DragDropClient::IsDragDropInProgress(). But
I dont think there is a generic views function. It might be worthwhile adding
something like that to corewm?

> 
> https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc
> File ui/views/view.cc (right):
> 
> https://codereview.chromium.org/22934007/diff/1/ui/views/view.cc#newcode2329
> ui/views/view.cc:2329: currently_dragging_ = true;
> On 2013/08/14 21:15:55, Elliot Glaysher wrote:
> > I vaguely remember us having some sort of scoped bool setter, but I can't
seem
> > to find it. If you know what the class name is, I'd be happy to use it.
> 
> I think you mean base::AutoReset.

[sadrul, 2013-08-14 22:49:28]

> There is aura specific aura::client::DragDropClient::IsDragDropInProgress().
> But I dont think there is a generic views function. It might be worthwhile
> adding something like that to corewm?

Ah yes, that's in aura land. Perhaps this will be simpler when views completely
switches to aura.

We should also improve our test coverage for drag-and-drop in views.

LGTM with the comment addressed:

https://codereview.chromium.org/22934007/diff/23001/ui/views/view.cc
File ui/views/view.cc (right):

https://codereview.chromium.org/22934007/diff/23001/ui/views/view.cc#newcode196
ui/views/view.cc:196: if (parent_)
Can you add a DCHECK(!currently_dragging_) here (with a comment explaining why)?
That would catch an invalid memory-write by AutoReset if RunShellDrag() can end
up destroying the View.

[varunjain, 2013-08-14 22:56:33]

On 2013/08/14 22:49:28, sadrul wrote:
> > There is aura specific aura::client::DragDropClient::IsDragDropInProgress().
> > But I dont think there is a generic views function. It might be worthwhile
> > adding something like that to corewm?
> 
> Ah yes, that's in aura land. Perhaps this will be simpler when views
completely
> switches to aura.
> 
> We should also improve our test coverage for drag-and-drop in views.

aura's DragDropClient is only tested for the chromeOS implementation.
Unfortunately no tests were added (either for linux or win) when desktop aura
implementation was created :(

> 
> LGTM with the comment addressed:
> 
> https://codereview.chromium.org/22934007/diff/23001/ui/views/view.cc
> File ui/views/view.cc (right):
> 
>
https://codereview.chromium.org/22934007/diff/23001/ui/views/view.cc#newcode196
> ui/views/view.cc:196: if (parent_)
> Can you add a DCHECK(!currently_dragging_) here (with a comment explaining
why)?
> That would catch an invalid memory-write by AutoReset if RunShellDrag() can
end
> up destroying the View.

[commit-bot: I haz the power, 2013-08-14 23:14:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/22934007/31001

[commit-bot: I haz the power, 2013-08-15 07:09:30]

Change committed as 217757