Fix initial hiding and centering cursor on Ozone

Fix initial hiding and centering cursor on ChromeOS

DisplayController::UpdateMouseLocationAfterDisplayChange() is now called during
startup:
- In Ozone once a channel to the GPU has been established
- In X11 as a result of
  DisplayChangeObserver::OnTouchscreenDeviceConfigurationChanged()

Moving the mouse on startup:
- Causes side effects such as mouse hover
- Causes a mouse event to be generated which causes
  CompoundEventFilter::SetCursorVisibilityOnEvent() to show the
  mouse cursor.

This CL makes DisplayController::UpdateMouseLocationAfterDisplayChange() move the
mouse only if necessary.

BUG=450860
TEST=Manual, see bug

Committed: https://crrev.com/04bbbd5582af60920656cc9768d2ac4e03709b69
Cr-Commit-Position: refs/heads/master@{#318708}

[pkotwicz, 2015-02-06 16:07:33]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2015-02-06 16:12:44]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org

[pkotwicz, 2015-02-06 16:12:44]

Oshima PTAL

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
ash/display/display_controller.cc:541: if
(!aura::Env::GetInstance()->mouse_moved() &&
I am unsure whether checking whether the mouse was moved is the best thing.

I considered checking whether the cursor has ever been visible (e.g.
aura::client::WasCursorVisible() I couldn't think of a way of correctly
initializing the cursor visibility in CursorManager though. There are issues
with passing the initial visibility to the CursorManager constructor)

[pkotwicz, 2015-02-10 16:57:18]

Ping!

[pkotwicz, 2015-02-10 16:57:19]

Ping!

[oshima, 2015-02-10 22:01:57]

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
ash/display/display_controller.cc:592:
dst_root_window->MoveCursorTo(target_location_in_native);
This was added to make sure that X cursor stays inside RootWindow that
ash manages, so this is probably not necessary on ozone.

Maybe you can initialize the mouse location during init process to the center of
display, and then skip MoveCursorTo if the native location didn't change?

[pkotwicz, 2015-02-12 01:52:56]

Hi Oshima, can you please take another look? Sorry for the delay

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
ash/display/display_controller.cc:592:
dst_root_window->MoveCursorTo(target_location_in_native);
This bug occurs on both X and Ozone so we need to find a solution which works on
both

When DriCursor moves the cursor (e.g. DriCursor::OnWindowAdded()) a
ui::MouseEvent is not generated. Hence:
- aura::Env::last_mouse_location() is not updated
- if a user has "Large Cursors" turned on, the cursor position is not updated

More specifically, if we skipped MoveCursorTo() if the native location didn't
change, and the user had large cursors turned on, when the user swaps primary
displays:
- The large cursor will appear to jump when the user moves the mouse immediately
after having swapped the primary display 

We could make DriCursor generate mouse events when it moves the cursor, however
when DriCursor::OnWindowAdded() initially moves the cursor to the center of the
screen this would generate a ui::MouseEvent which in turn will cause the cursor
to get shown

Do you have suggestions as to where we can initialize the mouse location during
the init process? I am not sure if we want to do this. However, if we did I am
not sure where we would do it.

[oshima, 2015-02-12 02:08:57]

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
ash/display/display_controller.cc:592:
dst_root_window->MoveCursorTo(target_location_in_native);
On 2015/02/12 01:52:55, pkotwicz wrote:
> This bug occurs on both X and Ozone so we need to find a solution which works
on
> both

My point is that this method was added because cursor may leak (that is move
outside of display) on X impl. I believe it won't happen on ozone?

> 
> When DriCursor moves the cursor (e.g. DriCursor::OnWindowAdded()) a
> ui::MouseEvent is not generated. Hence:
> - aura::Env::last_mouse_location() is not updated
> - if a user has "Large Cursors" turned on, the cursor position is not updated
> 
> More specifically, if we skipped MoveCursorTo() if the native location didn't
> change, and the user had large cursors turned on, when the user swaps primary
> displays:
> - The large cursor will appear to jump when the user moves the mouse
immediately
> after having swapped the primary display 
> 
> We could make DriCursor generate mouse events when it moves the cursor,
however
> when DriCursor::OnWindowAdded() initially moves the cursor to the center of
the
> screen this would generate a ui::MouseEvent which in turn will cause the
cursor
> to get shown
> 
> Do you have suggestions as to where we can initialize the mouse location
during
> the init process? I am not sure if we want to do this. However, if we did I am
> not sure where we would do it.

[pkotwicz, 2015-02-12 02:37:41]

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
ash/display/display_controller.cc:592:
dst_root_window->MoveCursorTo(target_location_in_native);
How do you suggest fixing this bug on CrOS X11 (or should we ignore CrOS X11)

Yes, I believe that DriCursor is supposed to keep the cursor within the display
bounds
Are you suggesting:
- Making DriCursor generate mouse events when it moves the mouse so that it
stays on the display (e.g. DriCursor::OnWindowRemoved() )
- We add special logic in DriCursor so that it does not generate a mouse event
when it moves the mouse cursor to its initial location

[oshima, 2015-02-12 19:11:13]

On 2015/02/12 02:37:41, pkotwicz wrote:
>
https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
> File ash/display/display_controller.cc (right):
> 
>
https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
> ash/display/display_controller.cc:592:
> dst_root_window->MoveCursorTo(target_location_in_native);
> How do you suggest fixing this bug on CrOS X11 (or should we ignore CrOS X11)
> 
> Yes, I believe that DriCursor is supposed to keep the cursor within the
display
> bounds
> Are you suggesting:
> - Making DriCursor generate mouse events when it moves the mouse so that it
> stays on the display (e.g. DriCursor::OnWindowRemoved() )

yes. I think it's ok to show mouse after display configuration change that
affected the
actual location of the mouse.

> - We add special logic in DriCursor so that it does not generate a mouse event
> when it moves the mouse cursor to its initial location

Is it possible to initialize Env::last_mouse_location to the center of the
primary display at startup,
and then skip mouse motion event if the event's location is same as last mouse
location?

[spang, 2015-02-13 15:33:44]

On 2015/02/12 19:11:13, oshima wrote:
> On 2015/02/12 02:37:41, pkotwicz wrote:
> >
>
https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
> > File ash/display/display_controller.cc (right):
> > 
> >
>
https://codereview.chromium.org/902123002/diff/20001/ash/display/display_cont...
> > ash/display/display_controller.cc:592:
> > dst_root_window->MoveCursorTo(target_location_in_native);
> > How do you suggest fixing this bug on CrOS X11 (or should we ignore CrOS
X11)
> > 
> > Yes, I believe that DriCursor is supposed to keep the cursor within the
> display
> > bounds
> > Are you suggesting:
> > - Making DriCursor generate mouse events when it moves the mouse so that it
> > stays on the display (e.g. DriCursor::OnWindowRemoved() )
> 
> yes. I think it's ok to show mouse after display configuration change that
> affected the
> actual location of the mouse.

Yeah, I think it's fair to say it is a bug that we move the cursor without
creating an event.

> 
> > - We add special logic in DriCursor so that it does not generate a mouse
event
> > when it moves the mouse cursor to its initial location
> 
> Is it possible to initialize Env::last_mouse_location to the center of the
> primary display at startup,
> and then skip mouse motion event if the event's location is same as last mouse
> location?

[pkotwicz, 2015-02-13 17:13:27]

I got stuck. I tried making DisplayController::EnsurePointerInDisplays() only
called in X11 and making ozone handle moving the mouse if necessary.

I found a problem with this approach and do not know how to fix it. Repro steps:
1) Do not have an external monitor connected
2) In chrome://settings change the internal display's rotation
3) With my changes a ui::ET_MOUSE_MOVED event is NOT generated because the
native mouse position does not change.
Expected:
- aura::Env::last_mouse_location() has the new screen position (Note that
although the native mouse location did not change, the screen mouse location did
change)
- any hover effects are cleared (I left the mouse over the refresh button and
the mouse is no longer over the refresh button after rotation)
Actual:
- aura::Env::last_mouse_location() is not updated
- There are stray hover effects

Oshima, do you have any suggestions?

[oshima, 2015-02-13 23:34:04]

On 2015/02/13 17:13:27, pkotwicz wrote:
> I got stuck. I tried making DisplayController::EnsurePointerInDisplays() only
> called in X11 and making ozone handle moving the mouse if necessary.
> 
> I found a problem with this approach and do not know how to fix it. Repro
steps:
> 1) Do not have an external monitor connected
> 2) In chrome://settings change the internal display's rotation
> 3) With my changes a ui::ET_MOUSE_MOVED event is NOT generated because the
> native mouse position does not change.
> Expected:
> - aura::Env::last_mouse_location() has the new screen position (Note that
> although the native mouse location did not change, the screen mouse location
did
> change)
>
> - any hover effects are cleared (I left the mouse over the refresh button and
> the mouse is no longer over the refresh button after rotation)
> Actual:
> - aura::Env::last_mouse_location() is not updated
> - There are stray hover effects
> 
> Oshima, do you have any suggestions?

You can have ozone version of EnsurePointerInDisplay
(UpdateMouseLocationAfterDisplayChange?),
where you convert the native location of cursor to logical location and move it
if necessary.
If you initialize the mouse location in Shel::Init in the same way, it won't
generate new mouse move
event during startup.

[pkotwicz, 2015-02-18 21:25:45]

Patchset #2 (id:40001) has been deleted

[pkotwicz, 2015-02-18 21:28:33]

Patchset #2 (id:60001) has been deleted

[pkotwicz, 2015-02-18 21:56:41]

pkotwicz@chromium.org changed reviewers:
+ mukai@chromium.org

[pkotwicz, 2015-02-18 21:56:41]

mukai@, can you please take a preliminary look? (oshima@ seems to be OOO)
This seems like the cleanest solution

- I know I need to change the CL description
- I know I need add tests (There seems to be a few tests in
AshNativeCursorManagerTest but there could be more)

I looked into doing what oshima@ suggested. (I believe that Oshima's suggestion
was for us to trust ozone fully to move the mouse cursor when a display is
removed) The problems are:
- We still need to dispatch a mouse event in ozone in
DisplayController::EnsurePointerInDisplays() to clear mouse hover effects in the
case that the user changes the display rotation (We would presumably do this if
cursor_location_in_screen_coords_for_restore_ changed in the interval between
PreDisplayConfigurationChange() and PostDisplayConfigurationChange())
- If Ozone generates mouse moves as a result of a display being removed
(DriCursor::OnWindowRemoved() in particular), it would generate the event
asynchronously. This makes it hard to detect whether
|cursor_location_in_screen_coords_for_restore_| changed
- There is a lot of work required to test ozone's cursor repositioning logic in
a ash_unittest (Have an ash_unittest which checks whether the cursor scale
factor changed as a result of swapping the primary display seems desirable, but
would be hard to do)

[Jun Mukai, 2015-02-18 23:14:06]

On 2015/02/18 21:56:41, pkotwicz wrote:
> mukai@, can you please take a preliminary look? (oshima@ seems to be OOO)
> This seems like the cleanest solution
> 
> - I know I need to change the CL description
> - I know I need add tests (There seems to be a few tests in
> AshNativeCursorManagerTest but there could be more)
> 
> I looked into doing what oshima@ suggested. (I believe that Oshima's
suggestion
> was for us to trust ozone fully to move the mouse cursor when a display is
> removed) The problems are:
> - We still need to dispatch a mouse event in ozone in
> DisplayController::EnsurePointerInDisplays() to clear mouse hover effects in
the
> case that the user changes the display rotation (We would presumably do this
if
> cursor_location_in_screen_coords_for_restore_ changed in the interval between
> PreDisplayConfigurationChange() and PostDisplayConfigurationChange())
> - If Ozone generates mouse moves as a result of a display being removed
> (DriCursor::OnWindowRemoved() in particular), it would generate the event
> asynchronously. This makes it hard to detect whether
> |cursor_location_in_screen_coords_for_restore_| changed
> - There is a lot of work required to test ozone's cursor repositioning logic
in
> a ash_unittest (Have an ash_unittest which checks whether the cursor scale
> factor changed as a result of swapping the primary display seems desirable,
but
> would be hard to do)

Sorry, I have little context here.

EnsurePointerInDisplays() is originally for display configuration changes as
oshima
mentioned above, I think it's better to keep it as is. Suppressing unnecessary
MoveCursorTo() in initialization would be a good idea, but it might not be ozone
specific.

I am also not so sure how this solves single display issue. Is EnsurePointer...
called
on single display initialization?

[Jun Mukai, 2015-02-18 23:14:21]

oops, there's a comment as well.

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
ash/display/display_controller.cc:587:
::wm::ConvertPointToScreen(dst_root_window, &target_location_in_root);
&target_location_in_screen?

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
ash/display/display_controller.cc:597:
cursor_location_in_screen_coords_for_restore_) {
EnsurePointerInDisplay ensures mouse left outside of Ash's coordinates. Wouldn't
it be good to compare display id's instead of locations?  If the dst_root_window
is corresponded with the display where the mouse cursor resides at
PreDisplayConfigurationChange(), we can say we don't have to move mouse cursor
at all.

[pkotwicz, 2015-02-19 01:01:17]

mukai@ can you please take another look?

Yes, EnsurePointerInDisplays() is called on startup.

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
ash/display/display_controller.cc:587:
::wm::ConvertPointToScreen(dst_root_window, &target_location_in_root);
Thanks for noticing!

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
ash/display/display_controller.cc:597:
cursor_location_in_screen_coords_for_restore_) {
In some cases we still need to call WindowTreeHost::MoveCursorTo() even though
the cursor did not change displays and did not move
- If the display's rotation or scale factor changes, the cursor does not move
but it's screen location does. aura::Env::last_mouse_location() should be
updated
- WindowTreeHost::MoveCursorTo() has useful side effects such as updating the
cursor's rotation and scale factor. We currently rely on
DisplayController::EnsurePointerInDisplays() to do this updating.

[Jun Mukai, 2015-02-19 01:50:06]

Sorry if it's already discussed, but can we --

- initialize aura::Env::last_cursor_location to some extreme value (like
Point(int32max, int32max)) so that it can never be in a display, then
EnsurePointerInDisplays() can move it to the center of the primary screen?  (and
needs some trick to prevent showing the cursor if the position is unchanged)

- or, stop calling EnsurePointerInDisplays() in the initialization?  As I
quickly see, that would be ozone-specific (it's not called in X11), and probably
that's because ozone initialization starts with a dummy primary screen and then
switches to the actual screen size.

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/80001/ash/display/display_cont...
ash/display/display_controller.cc:597:
cursor_location_in_screen_coords_for_restore_) {
On 2015/02/19 01:01:17, pkotwicz wrote:
> In some cases we still need to call WindowTreeHost::MoveCursorTo() even though
> the cursor did not change displays and did not move
> - If the display's rotation or scale factor changes, the cursor does not move
> but it's screen location does. aura::Env::last_mouse_location() should be
> updated
> - WindowTreeHost::MoveCursorTo() has useful side effects such as updating the
> cursor's rotation and scale factor. We currently rely on
> DisplayController::EnsurePointerInDisplays() to do this updating.

Aah, right. thanks.

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
ash/display/display_controller.cc:585: #if defined(USE_OZONE)
This check itself doesn't look ozone specific -- although it can happen only on
ozone, it's safe to check on X11 too, right?

[pkotwicz, 2015-02-19 02:43:43]

We actually have the problem of calling EnsurePointerInDisplays() during
intiialization on both X11 and Ozone. Oshima was ok with a hack to fix X11

The problem with initializing the cursor to some extreme location and then
calling EnsurePointerInDisplays() is that:
- Moving the mouse generates a mouse event
- Mouse events cause the mouse cursor to show
CompoundEventFilter::OnMouseEvent() does this

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
ash/display/display_controller.cc:585: #if defined(USE_OZONE)
Ozone moves the mouse for us if the mouse's native position did not change but
the mouse is now on another display. X11 does not. When swapping the primary
display, the mouse moves to a different display even though it's native and
screen coordinates stay the same.

I think that Oshima mentioned that we move the cursor to prevent some X11
badness. I am unsure when this badness occurrs. Maybe it makes sense to wait for
him to come back

Notwithstanding X11 badness if we checked whether the display that the cursor is
on changed in addition to the other two checks, then we would be able to use the
same logic on X11 and Ozone

[Jun Mukai, 2015-02-19 02:57:24]

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
ash/display/display_controller.cc:585: #if defined(USE_OZONE)
On 2015/02/19 02:43:43, pkotwicz wrote:
> Ozone moves the mouse for us if the mouse's native position did not change but
> the mouse is now on another display. X11 does not. When swapping the primary
> display, the mouse moves to a different display even though it's native and
> screen coordinates stay the same.
> 
> I think that Oshima mentioned that we move the cursor to prevent some X11
> badness. I am unsure when this badness occurrs. Maybe it makes sense to wait
for
> him to come back
> 
> Notwithstanding X11 badness if we checked whether the display that the cursor
is
> on changed in addition to the other two checks, then we would be able to use
the
> same logic on X11 and Ozone 

Hm, okay. For display swapping, I guess the X11 badness means that the mouse
cursor location (from X11 point of view) gets inconsistent with the location
maintained by aura::Env or Ash.

Let me make sure what will happen for the display swapping in ozone with this
CL:
- EnsurePointerInDisplays() does not move the cursor position, stays on the same
location
- This however means the mouse cursor stays in the same root window, i.e. cursor
looks like jumping to the swapped display

correct?

[pkotwicz, 2015-02-19 06:13:50]

I am not sure what the X11 badness is. (I guess only Oshima does)

Yes, that's correct. DriWindow::SetBounds() has special logic to move the cursor
to a different display if the WTH moves to a different display

[Jun Mukai, 2015-02-19 18:07:55]

Okay, then lgtm.
Honestly I still hope this could be solved in another place,
but I can't think of it at all. Possibly I'm just wrong and
this is the cleanest approach as you said.

Also please write tests.

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
File ash/display/display_controller.cc (right):

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
ash/display/display_controller.cc:540: void
DisplayController::EnsurePointerInDisplays() {
This does not "ensure" pointers in displays anymore (actually it's not been a
while). As we discussed, it needs to be called for screen rotations and
primary-display-swap for X11.

Could you rename this like UpdateMouseLocationAfterDisplayChange (seen in
oshima's comment) or SyncMouseLocationAfter... or something.

https://codereview.chromium.org/902123002/diff/100001/ash/display/display_con...
ash/display/display_controller.cc:603: // display than when it was set.
Could you add a comment why we should move the cursor even if the cursor
location did not change at all?

[pkotwicz, 2015-02-24 20:33:58]

Patchset #4 (id:120001) has been deleted

[pkotwicz, 2015-02-24 20:37:33]

Patchset #4 (id:140001) has been deleted

[pkotwicz, 2015-02-24 20:46:18]

Oshima, for a second look.
Do you think we can use the Ozone behavior on X11? (If not, what would be an
appropriate comment?)

I now dispatch a mouse move when the displays are swapped. It is not necessary
Ozone but it makes things cleaner.

[pkotwicz, 2015-02-26 19:09:54]

Oshima, Ping!

[oshima, 2015-02-27 22:11:33]

lgtm for now. I think once we fully migrated to ozone, it'd be nice if ozone can
take care of this instead.

[pkotwicz, 2015-02-28 18:33:02]

We still need this logic even once we fully migrate to ozone.
- The position in screen coordinates may change even if the position in native
coordinates does not change (e.g. rotation)
- If ozone moved the mouse as a result of a bounds change, the move would be
asynchronous. This would make it difficult to detect if the screen position did
change
- If we relied on ozone to take care of this, the behavior would be hard to
test. My understanding is that ash_unittests has to use a stub implementation
and that a "real integration test" is impossible with ozone. For instance, I am
unsure how we would test that "the rotation of the composited cursor is
correctly updated when a screen is disconnected" if ozone was responsible for
repositioning the cursor after a screen is disconnected

[pkotwicz, 2015-02-28 18:50:52]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2015-02-28 18:50:53]

The patchset sent to the CQ was uploaded after l-g-t-m from mukai@chromium.org
Link to the patchset: https://codereview.chromium.org/902123002/#ps160001
(title: " ")

[commit-bot: I haz the power, 2015-02-28 18:51:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/160001

[commit-bot: I haz the power, 2015-02-28 18:56:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-28 18:56:04]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[pkotwicz, 2015-02-28 23:21:26]

The CQ bit was checked by pkotwicz@chromium.org

[pkotwicz, 2015-02-28 23:21:27]

The patchset sent to the CQ was uploaded after l-g-t-m from mukai@chromium.org,
oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/902123002/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-02-28 23:21:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/180001

[commit-bot: I haz the power, 2015-02-28 23:26:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-28 23:26:35]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[pkotwicz, 2015-03-01 00:41:56]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-01 00:42:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/180001

[commit-bot: I haz the power, 2015-03-01 00:47:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-01 00:47:41]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[pkotwicz, 2015-03-01 16:04:38]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-01 16:04:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/180001

[commit-bot: I haz the power, 2015-03-01 16:09:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-01 16:09:37]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[pkotwicz, 2015-03-01 22:17:33]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-01 22:18:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/180001

[commit-bot: I haz the power, 2015-03-01 22:23:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-01 22:23:17]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[pkotwicz, 2015-03-02 15:14:34]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2015-03-02 15:15:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/902123002/180001

[commit-bot: I haz the power, 2015-03-02 16:17:07]

Committed patchset #5 (id:180001)

[commit-bot: I haz the power, 2015-03-02 16:18:04]

Patchset 5 (id:??) landed as
https://crrev.com/04bbbd5582af60920656cc9768d2ac4e03709b69
Cr-Commit-Position: refs/heads/master@{#318708}

[oshima, 2015-03-02 20:53:03]

On 2015/02/28 18:33:02, pkotwicz wrote:
> We still need this logic even once we fully migrate to ozone.
> - The position in screen coordinates may change even if the position in native
> coordinates does not change (e.g. rotation)

We can simplify the logic by
1) including the native cursor location in the display change event
2) generate synthesized event if the native coord didn't change, but it resulted
in
   different screen location.

We can eliminate the logic to fit into the display at least.

> - If ozone moved the mouse as a result of a bounds change, the move would be
> asynchronous. This would make it difficult to detect if the screen position
did
> change

If the display change indeed caused the mouse move, it's ok to show the mouse.

> - If we relied on ozone to take care of this, the behavior would be hard to
> test. My understanding is that ash_unittests has to use a stub implementation
> and that a "real integration test" is impossible with ozone. For instance, I
am
> unsure how we would test that "the rotation of the composited cursor is
> correctly updated when a screen is disconnected" if ozone was responsible for
> repositioning the cursor after a screen is disconnected

It'd be nice if we can have mocked DriWindow / DriCursor, and run test on them.