Release aura::Desktop event handler windows on detaching an ancestor window.

Release aura::Desktop event handler windows on detaching an ancestor window.

BUG=105443
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112236

[sadrul, 2011-11-26 20:28:53]

LGTM.

(you will need OWNERS)

[flackr, 2011-11-27 00:44:42]

+OWNERS: ben or sky, please take a look, thanks!

[flackr, 2011-11-28 15:50:49]

+sky, can you take a look?

[sky, 2011-11-28 17:20:22]

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc
File ui/aura/desktop.cc (right):

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc#newcode543
ui/aura/desktop.cc:543: if (IsAncestor(detached, capture_window_) &&
capture_window_ != this)
Is the capture_window_ condition necessary? Also, see the DCHECK @ 539.

[Ben Goodger (Google), 2011-11-28 17:50:19]

Don't forget to add a unittest for Contains() also.

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc
File ui/aura/desktop.cc (right):

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc#newcode107
ui/aura/desktop.cc:107: return descendant == ancestor;
Rather than defining this here, can you add a Contains() method to Window,
similar to Layer's?

[flackr, 2011-11-28 20:36:35]

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc
File ui/aura/desktop.cc (right):

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc#newcode107
ui/aura/desktop.cc:107: return descendant == ancestor;
On 2011/11/28 17:50:19, Ben Goodger (Google) wrote:
> Rather than defining this here, can you add a Contains() method to Window,
> similar to Layer's?

Done.

http://codereview.chromium.org/8700013/diff/1001/ui/aura/desktop.cc#newcode543
ui/aura/desktop.cc:543: if (IsAncestor(detached, capture_window_) &&
capture_window_ != this)
On 2011/11/28 17:20:22, sky wrote:
> Is the capture_window_ condition necessary? Also, see the DCHECK @ 539.

Oops, seems like I converted the old statement incorrectly. In the old code,
after the loop window is either NULL or equal to detached so I should check that
detached != this. Done.

[sky, 2011-11-28 21:55:25]

LGTM

http://codereview.chromium.org/8700013/diff/9003/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8700013/diff/9003/ui/aura/window.cc#newcode281
ui/aura/window.cc:281: for (const Window* parent = other; parent; parent =
parent->parent()) {
nit: use parent->parent_ here.

[flackr, 2011-11-29 14:54:13]

http://codereview.chromium.org/8700013/diff/9003/ui/aura/window.cc
File ui/aura/window.cc (right):

http://codereview.chromium.org/8700013/diff/9003/ui/aura/window.cc#newcode281
ui/aura/window.cc:281: for (const Window* parent = other; parent; parent =
parent->parent()) {
On 2011/11/28 21:55:25, sky wrote:
> nit: use parent->parent_ here.

Done.

[commit-bot: I haz the power, 2011-11-30 15:08:54]

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

[commit-bot: I haz the power, 2011-11-30 16:30:53]

Change committed as 112236