Synthesize mouse move event after window creation/deletion or size/visibiilty changed

Synthesize mouse move event after window creation/deletion or size/visibiilty changed
I also updated method names to match other OnXXX methods.

BUG=111073
TEST=to be added

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

[oshima, 2012-01-25 23:46:58]

I'll add tests if this LGTY.

[sky, 2012-01-26 00:07:55]

This approach seems fine by me.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:311: PostMouseMoveEventAfterWindowChange();
Same comment as in 623 applies to all these places.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:623: if (old_bounds.Contains(last_mouse_location_))
Only do this if detached was visible. In fact do you only need  to do this if
the mouse is over detached or one of its descendants? Is there some way to
quickly check that?

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

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode99
ui/aura/window.cc:99: root_window->OnWindowDestroyed(this, old_bounds);
Do we really need do this here? Could we instead do it from OnWindowDestroying?
That would mean you don't need to pass the bounds up.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode174
ui/aura/window.cc:174: ConvertPointToWindow(parent(), root_window, &origin);
Doesn't this miss the transform set on the layer?

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode296
ui/aura/window.cc:296: root_window->OnWindowDetachedFromRootWindow(child,
orig_bounds);
Could we do the processing before setting the parent? That way RootWindow would
only need ask for the bounds if necessary. If this works, rename it to
OnWillDetachFromRootWindow.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h#newcode128
ui/aura/window.h:128: gfx::Rect GetRootTargetBounds();
How about GetTargetBoundsInRoot.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h#newcode286
ui/aura/window.h:286: // from its parent.
Document the coordinates of orig_bounds.

[oshima, 2012-01-26 22:24:00]

uploaded new patch that supports translation. PTAL

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:311: PostMouseMoveEventAfterWindowChange();
On 2012/01/26 00:07:56, sky wrote:
> Same comment as in 623 applies to all these places.

Done.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:623: if (old_bounds.Contains(last_mouse_location_))
On 2012/01/26 00:07:56, sky wrote:
> Only do this if detached was visible.
done.

> In fact do you only need  to do this if
> the mouse is over detached or one of its descendants? Is there some way to
> quickly check that?

Descendants should be covered by parent, shouldn't it? I know layer doesn't
enforce this (or maybe it does now with Webkit compositor?) and HitTest can
(technically) extend the window size, but do you think we should consider such
case?

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

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode99
ui/aura/window.cc:99: root_window->OnWindowDestroyed(this, old_bounds);
On 2012/01/26 00:07:56, sky wrote:
> Do we really need do this here? Could we instead do it from
OnWindowDestroying?
> That would mean you don't need to pass the bounds up.

thank you for the catch. This is no longer needed as I switched to "posting
event".

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode174
ui/aura/window.cc:174: ConvertPointToWindow(parent(), root_window, &origin);
On 2012/01/26 00:07:56, sky wrote:
> Doesn't this miss the transform set on the layer?

Good point. Actually we need to generate mouse event when transform changed on a
window.  I updated CL to support it.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.cc#newcode296
ui/aura/window.cc:296: root_window->OnWindowDetachedFromRootWindow(child,
orig_bounds);
On 2012/01/26 00:07:56, sky wrote:
> Could we do the processing before setting the parent? That way RootWindow
would
> only need ask for the bounds if necessary. If this works, rename it to
> OnWillDetachFromRootWindow.

Renamed to OnWindowDetachingFromRootWindow. I think that's more consistent with
other methods.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h#newcode128
ui/aura/window.h:128: gfx::Rect GetRootTargetBounds();
On 2012/01/26 00:07:56, sky wrote:
> How about GetTargetBoundsInRoot.

This method is removed. ContainsRootPoint is added instead.

http://codereview.chromium.org/9235025/diff/2006/ui/aura/window.h#newcode286
ui/aura/window.h:286: // from its parent.
On 2012/01/26 00:07:56, sky wrote:
> Document the coordinates of orig_bounds.

argument is removed.

[sky, 2012-01-26 22:57:33]

http://codereview.chromium.org/9235025/diff/7002/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9235025/diff/7002/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:717: if (synthesize_mouse_move_)
Rather than maintaining synthesize_mouse_move_, could you check event_factorY_
for weak pointers?

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

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.cc#newcode366
ui/aura/window.cc:366: if (!root_window)
Should this return if !visible?

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.cc#newcode596
ui/aura/window.cc:596: continue;
nit: you added a space here.

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.h#newcode208
ui/aura/window.h:208: // Returns true if the |ponit_in_root| in root window's
coordinate falls
ponit -> point

http://codereview.chromium.org/9235025/diff/7002/ui/views/controls/menu/menu_...
File ui/views/controls/menu/menu_controller.cc (right):

http://codereview.chromium.org/9235025/diff/7002/ui/views/controls/menu/menu_...
ui/views/controls/menu/menu_controller.cc:7: #include "base/debug/stack_trace.h"
Revert these changes.

[oshima, 2012-01-26 23:38:43]

http://codereview.chromium.org/9235025/diff/7002/ui/aura/root_window.cc
File ui/aura/root_window.cc (right):

http://codereview.chromium.org/9235025/diff/7002/ui/aura/root_window.cc#newco...
ui/aura/root_window.cc:717: if (synthesize_mouse_move_)
On 2012/01/26 22:57:34, sky wrote:
> Rather than maintaining synthesize_mouse_move_, could you check event_factorY_
> for weak pointers?

I still need to keep the state because I need to cancel when true mouse event
arrives first. I can switch to if (HasWeakPtrs()) if you prefer that way though.
Please let me know.

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

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.cc#newcode366
ui/aura/window.cc:366: if (!root_window)
On 2012/01/26 22:57:34, sky wrote:
> Should this return if !visible?

This needs to be outside because I need to generate event when it becomes
invisible. See RootWindow::OnWindowVisibilityChagned

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.cc#newcode596
ui/aura/window.cc:596: continue;
On 2012/01/26 22:57:34, sky wrote:
> nit: you added a space here.

Done.

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.h
File ui/aura/window.h (right):

http://codereview.chromium.org/9235025/diff/7002/ui/aura/window.h#newcode208
ui/aura/window.h:208: // Returns true if the |ponit_in_root| in root window's
coordinate falls
On 2012/01/26 22:57:34, sky wrote:
> ponit -> point

Done.

http://codereview.chromium.org/9235025/diff/7002/ui/views/controls/menu/menu_...
File ui/views/controls/menu/menu_controller.cc (right):

http://codereview.chromium.org/9235025/diff/7002/ui/views/controls/menu/menu_...
ui/views/controls/menu/menu_controller.cc:7: #include "base/debug/stack_trace.h"
On 2012/01/26 22:57:34, sky wrote:
> Revert these changes.

oops, done.

[sky, 2012-01-26 23:57:14]

LGTM

[commit-bot: I haz the power, 2012-01-27 00:05:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/9235025/6015

[commit-bot: I haz the power, 2012-01-27 00:05:55]

Can't apply patch for file ui/aura/root_window.cc.
While running patch -p1 --forward --force;
patching file ui/aura/root_window.cc
Hunk #4 succeeded at 276 (offset -2 lines).
Hunk #5 FAILED at 303.
Hunk #6 succeeded at 401 (offset 6 lines).
Hunk #7 succeeded at 411 (offset 6 lines).
Hunk #8 succeeded at 598 (offset 18 lines).
Hunk #9 succeeded at 619 (offset 18 lines).
Hunk #10 succeeded at 705 (offset 18 lines).
1 out of 10 hunks FAILED -- saving rejects to file ui/aura/root_window.cc.rej

[commit-bot: I haz the power, 2012-01-27 17:31:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/9235025/8001

[commit-bot: I haz the power, 2012-01-27 20:00:08]

Try job failure for 9235025-8001 (retry) on mac_rel for step "base_unittests".
It's a second try, previously, step "base_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...