Implement cursor changing on Mushrome

Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798, 734807, 734809
Review-Url: https://codereview.chromium.org/2932563002
Cr-Commit-Position: refs/heads/master@{#481278}
Committed: https://chromium.googlesource.com/chromium/src/+/36cfb953ea04dac15159b516cd9ce741c6b7384b

[Elliot Glaysher, 2017-06-07 22:15:52]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 22:16:17]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-07 23:06:43]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-07 23:06:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 00:24:24]

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

[commit-bot: I haz the power, 2017-06-08 00:24:25]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-06-08 20:25:33]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-08 20:26:12]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-08 23:37:31]

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

[commit-bot: I haz the power, 2017-06-08 23:37:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-06-09 19:54:50]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 19:55:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-09 21:42:02]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 21:42:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-09 22:53:09]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-09 22:53:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-09 23:27:04]

Description was changed from

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This does not implement most of the interface, such as ash-side bitmap
cursors, or cursor sets.

BUG=729798
==========

to

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This does not implement most of the interface, such as cursor sets.

BUG=729798
==========

[commit-bot: I haz the power, 2017-06-10 00:20:52]

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

[commit-bot: I haz the power, 2017-06-10 00:20:52]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-06-12 18:08:16]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 18:08:49]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-12 19:24:12]

Description was changed from

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This does not implement most of the interface, such as cursor sets.

BUG=729798
==========

to

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798
==========

[Elliot Glaysher, 2017-06-12 19:24:40]

erg@chromium.org changed reviewers:
+ jamescook@chromium.org

[commit-bot: I haz the power, 2017-06-12 20:07:11]

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

[commit-bot: I haz the power, 2017-06-12 20:07:14]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-06-12 20:17:24]

https://codereview.chromium.org/2932563002/diff/120001/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/BUILD.gn#newcode632
ash/BUILD.gn:632: "wm/classic_ash_native_cursor_manager.cc",
I would go with native_cursor_manager_ash_classic and
native_cursor_manager_ash_mus[hrome], or native_cursor_manager_classic and
native_cursor_manager_mus[hrome] to match shell_port_classic.h /
shell_port_mash.h

tools/git/move_source_file.py + tools/git/mffr.py for the win!

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode442
ash/shell.cc:442: // TODO: needs to work in mus. http://crbug.com/705592.
Comment still relevant? Should "mus" be "mash"?

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode861
ash/shell.cc:861: // TODO: needs to work in mus. http://crbug.com/705592.
ditto

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode863
ash/shell.cc:863: cursor_manager_.reset(
nit: base::MakeUnique<>

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/ash_native_curs...
File ash/wm/ash_native_cursor_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/ash_native_curs...
ash/wm/ash_native_cursor_manager.cc:9:
AshNativeCursorManager::AshNativeCursorManager() {}
Do we still need this .cc file? Maybe just have a protected
~AshNativeCursorManager() {} in the header?

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
File ash/wm/mushrome_ash_native_cursor_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
ash/wm/mushrome_ash_native_cursor_manager.cc:95:
MushromeAshNativeCursorManager::~MushromeAshNativeCursorManager() {}
nit: = default

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
ash/wm/mushrome_ash_native_cursor_manager.cc:202: }  // namespace ash
Is it possible to write a simple test for this code? I don't really care how
much it does, I just want to make sure that we can either:

* Instantiate one of these in a test harness, or
* Grab one out of ash::Shell in an AshTestBase and make some assertions about it

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
File ash/wm/mushrome_ash_native_cursor_manager.h (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
ash/wm/mushrome_ash_native_cursor_manager.h:54: bool native_cursor_enabled_;
Is this initialized?

[Elliot Glaysher, 2017-06-12 21:19:25]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 21:19:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-12 21:54:35]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 21:55:09]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-12 23:18:46]

ptal

https://codereview.chromium.org/2932563002/diff/120001/ash/BUILD.gn
File ash/BUILD.gn (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/BUILD.gn#newcode632
ash/BUILD.gn:632: "wm/classic_ash_native_cursor_manager.cc",
On 2017/06/12 20:17:24, James Cook wrote:
> I would go with native_cursor_manager_ash_classic and
> native_cursor_manager_ash_mus[hrome], or native_cursor_manager_classic and
> native_cursor_manager_mus[hrome] to match shell_port_classic.h /
> shell_port_mash.h
> 
> tools/git/move_source_file.py + tools/git/mffr.py for the win!

Done.

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode442
ash/shell.cc:442: // TODO: needs to work in mus. http://crbug.com/705592.
On 2017/06/12 20:17:24, James Cook wrote:
> Comment still relevant? Should "mus" be "mash"?

Done.

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode861
ash/shell.cc:861: // TODO: needs to work in mus. http://crbug.com/705592.
On 2017/06/12 20:17:24, James Cook wrote:
> ditto

Done.

https://codereview.chromium.org/2932563002/diff/120001/ash/shell.cc#newcode863
ash/shell.cc:863: cursor_manager_.reset(
On 2017/06/12 20:17:24, James Cook wrote:
> nit: base::MakeUnique<>

Done.

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/ash_native_curs...
File ash/wm/ash_native_cursor_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/ash_native_curs...
ash/wm/ash_native_cursor_manager.cc:9:
AshNativeCursorManager::AshNativeCursorManager() {}
On 2017/06/12 20:17:24, James Cook wrote:
> Do we still need this .cc file? Maybe just have a protected
> ~AshNativeCursorManager() {} in the header?

Removed file, put dtor public (because of smart pointers), but protected ctor
which is what I assume you meant.

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
File ash/wm/mushrome_ash_native_cursor_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
ash/wm/mushrome_ash_native_cursor_manager.cc:202: }  // namespace ash
On 2017/06/12 20:17:24, James Cook wrote:
> Is it possible to write a simple test for this code? I don't really care how
> much it does, I just want to make sure that we can either:
> 
> * Instantiate one of these in a test harness, or
> * Grab one out of ash::Shell in an AshTestBase and make some assertions about
it

I've reenabled more than half of the NativeCursorManagerAsh tests, which work on
this implementation instead of the classic one when run in --mus mode.

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
File ash/wm/mushrome_ash_native_cursor_manager.h (right):

https://codereview.chromium.org/2932563002/diff/120001/ash/wm/mushrome_ash_na...
ash/wm/mushrome_ash_native_cursor_manager.h:54: bool native_cursor_enabled_;
On 2017/06/12 20:17:24, James Cook wrote:
> Is this initialized?

Done.

[commit-bot: I haz the power, 2017-06-12 23:55:58]

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

[commit-bot: I haz the power, 2017-06-12 23:55:59]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_TIMED_OUT, build has not
started yet; builder either lacks capacity or does not exist (misspelled?))

[James Cook, 2017-06-13 00:49:27]

LGTM with nits. Nice patch. I didn't realize we already had test coverage via
NativeCursorManagerAsh tests.

https://codereview.chromium.org/2932563002/diff/160001/ash/test/shell_test_api.h
File ash/test/shell_test_api.h (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/test/shell_test_ap...
ash/test/shell_test_api.h:33: NativeCursorManagerAsh*
ash_native_cursor_manager();
nit: rename method too

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash.h (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash.h:8: #include <memory>
not needed

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash.h:11: #include "base/compiler_specific.h"
Is this needed?

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash.h:13: #include "base/strings/string16.h"
many of these are not needed

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash_interactive_uitest.cc (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash_interactive_uitest.cc:4: #include
"ash/wm/native_cursor_manager_ash.h"
nit: blank line above

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash_unittest.cc (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash_unittest.cc:56: typedef test::AshTestBase
NativeCursorManagerAshTest;
nit: use using

https://codereview.chromium.org/2932563002/diff/160001/testing/buildbot/filte...
File testing/buildbot/filters/ash_unittests_mus.filter (right):

https://codereview.chromium.org/2932563002/diff/160001/testing/buildbot/filte...
testing/buildbot/filters/ash_unittests_mus.filter:2:
-NativeCursorManagerAshTest.SetCursorSet
Woo hoo, more stuff works!

[Elliot Glaysher, 2017-06-13 18:05:45]

https://codereview.chromium.org/2932563002/diff/160001/ash/test/shell_test_api.h
File ash/test/shell_test_api.h (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/test/shell_test_ap...
ash/test/shell_test_api.h:33: NativeCursorManagerAsh*
ash_native_cursor_manager();
On 2017/06/13 00:49:26, James Cook wrote:
> nit: rename method too

Done.

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash.h (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash.h:8: #include <memory>
On 2017/06/13 00:49:26, James Cook wrote:
> not needed

Removed half the includes.

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash_interactive_uitest.cc (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash_interactive_uitest.cc:4: #include
"ash/wm/native_cursor_manager_ash.h"
On 2017/06/13 00:49:27, James Cook wrote:
> nit: blank line above

Done.

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
File ash/wm/native_cursor_manager_ash_unittest.cc (right):

https://codereview.chromium.org/2932563002/diff/160001/ash/wm/native_cursor_m...
ash/wm/native_cursor_manager_ash_unittest.cc:56: typedef test::AshTestBase
NativeCursorManagerAshTest;
On 2017/06/13 00:49:27, James Cook wrote:
> nit: use using

Done.

https://codereview.chromium.org/2932563002/diff/160001/testing/buildbot/filte...
File testing/buildbot/filters/ash_unittests_mus.filter (right):

https://codereview.chromium.org/2932563002/diff/160001/testing/buildbot/filte...
testing/buildbot/filters/ash_unittests_mus.filter:2:
-NativeCursorManagerAshTest.SetCursorSet
On 2017/06/13 00:49:27, James Cook wrote:
> Woo hoo, more stuff works!

Acknowledged.

[Elliot Glaysher, 2017-06-13 18:05:50]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-06-13 18:05:50]

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

[commit-bot: I haz the power, 2017-06-13 18:06:20]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-13 18:06:39]

The CQ bit was unchecked by erg@chromium.org

[Elliot Glaysher, 2017-06-13 18:09:13]

erg@chromium.org changed reviewers:
+ reillyg@chromium.org, sadrul@chromium.org

[Elliot Glaysher, 2017-06-13 18:09:14]

+reillyg: //extensions comment change.
+sadrul: //ui

[Reilly Grant (use Gerrit), 2017-06-13 18:11:04]

lgtm

[sadrul, 2017-06-14 03:03:55]

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

[sadrul, 2017-06-14 03:03:57]

+oshima@ for ui/display owner.

stamp lgtm for views

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:705:
CreateSoftwareMirroringDisplayInfo(&new_display_info_list);
This seems somewhat subtle. One of the ui/display owners should review this.

[Elliot Glaysher, 2017-06-15 17:35:07]

ping oshima

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:705:
CreateSoftwareMirroringDisplayInfo(&new_display_info_list);
On 2017/06/14 03:03:56, sadrul wrote:
> This seems somewhat subtle. One of the ui/display owners should review this.

Yes, the bug here is that CloseMirroringDisplayIfNotNecessary() can access
display information. If you first call CreateSoftwareMirroringDisplayInfo(), you
get into a state where there is stale references to the current mirror display.

[oshima, 2017-06-15 18:16:18]

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:705:
CreateSoftwareMirroringDisplayInfo(&new_display_info_list);
On 2017/06/15 17:35:07, Elliot Glaysher wrote:
> On 2017/06/14 03:03:56, sadrul wrote:
> > This seems somewhat subtle. One of the ui/display owners should review this.
> 
> Yes, the bug here is that CloseMirroringDisplayIfNotNecessary() can access
> display information. If you first call CreateSoftwareMirroringDisplayInfo(),
you
> get into a state where there is stale references to the current mirror
display.

This was moved in this CL (https://codereview.chromium.org/1133323002), but it's
before ozone era.
I need to refresh my memory and dig code/bugs. Please give me some time.

[oshima, 2017-06-19 19:17:55]

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
File ui/display/manager/display_manager.cc (right):

https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
ui/display/manager/display_manager.cc:705:
CreateSoftwareMirroringDisplayInfo(&new_display_info_list);
On 2017/06/15 17:35:07, Elliot Glaysher wrote:
> On 2017/06/14 03:03:56, sadrul wrote:
> > This seems somewhat subtle. One of the ui/display owners should review this.
> 
> Yes, the bug here is that CloseMirroringDisplayIfNotNecessary() can access
> display information. If you first call CreateSoftwareMirroringDisplayInfo(),
you
> get into a state where there is stale references to the current mirror
display.

Which reference becomes stale?

[Elliot Glaysher, 2017-06-19 20:10:10]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-19 20:10:23]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-19 20:35:59]

On 2017/06/19 19:17:55, oshima wrote:
>
https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
> File ui/display/manager/display_manager.cc (right):
> 
>
https://codereview.chromium.org/2932563002/diff/180001/ui/display/manager/dis...
> ui/display/manager/display_manager.cc:705:
> CreateSoftwareMirroringDisplayInfo(&new_display_info_list);
> On 2017/06/15 17:35:07, Elliot Glaysher wrote:
> > On 2017/06/14 03:03:56, sadrul wrote:
> > > This seems somewhat subtle. One of the ui/display owners should review
this.
> > 
> > Yes, the bug here is that CloseMirroringDisplayIfNotNecessary() can access
> > display information. If you first call CreateSoftwareMirroringDisplayInfo(),
> you
> > get into a state where there is stale references to the current mirror
> display.
> 
> Which reference becomes stale?

The one in RootWindowSettings.

I don't entirely understand why things are like they are, but each RootWindow
has a RootWindowSettings (ash/root_window_settings.h). This struct contains a
reference to the |display_id|. This |display_id| is changed in
ash/display/mirror_window_controller.cc during
MirrorWindowController::UpdateWindow().

Here are how the two sequences play out:

First:
1) CreateSoftwareMirroringDisplayInfo()
  - Destroys the old mirroring display id and creates a new mirroring display
with a new id.
2) CloseMirroringDisplayIfNotNecessary()
  - Causes a cascade which causes a lookup of the root window's display_id to
translate screen coordinates. Attempts to access this cause a crash because we
just destroyed the old mirroring display id.
3) (if it hadn't crashed) CreateMirrorWindowAsyncIfAny()
  - Creates the new mirror window and updates the |display_id| in root window
settings.

Second:

1) CloseMirroringDisplayIfNotNecessary()
  - Deep down the stack, the old mirroring display still exists for coordinate
translation so we don't crash.
2) CreateSoftwareMirroringDisplayInfo()
  - Destroys the old mirroring display id, which isn't a problem anymore.
3) CreateMirrorWindowAsyncIfAny()
  - Creates the new mirror window and updates the |display_id| in root window
settings.

[commit-bot: I haz the power, 2017-06-19 21:34:45]

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

[commit-bot: I haz the power, 2017-06-19 21:34:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-06-19 23:45:45]

Description was changed from

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798
==========

to

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798,734807
==========

[Elliot Glaysher, 2017-06-19 23:49:07]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-19 23:49:47]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-20 00:52:55]

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

[commit-bot: I haz the power, 2017-06-20 00:52:56]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-06-20 17:38:15]

Description was changed from

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798,734807
==========

to

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798,734807,734809
==========

[Elliot Glaysher, 2017-06-20 17:41:10]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 17:41:27]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-20 19:19:23]

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

[commit-bot: I haz the power, 2017-06-20 19:19:24]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-06-20 19:20:06]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 19:20:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-20 20:34:37]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 20:35:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-20 20:57:56]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 20:58:30]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2017-06-20 22:06:03]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 22:06:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-20 23:01:30]

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

[commit-bot: I haz the power, 2017-06-20 23:01:32]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-06-21 17:06:57]

The CQ bit was checked by erg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 17:07:18]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 17:34:51]

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

[commit-bot: I haz the power, 2017-06-21 17:34:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Elliot Glaysher, 2017-06-21 17:53:42]

oshima: i've applied your fix to mirror_window_controller.cc and it's passed the
linux_chromium_chromeos_ozone_rel_ng bot. ptal.

[oshima, 2017-06-21 18:06:11]

lgtm

[Elliot Glaysher, 2017-06-21 18:07:05]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2017-06-21 18:07:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamescook@chromium.org, sadrul@chromium.org, reillyg@chromium.org
Link to the patchset: https://codereview.chromium.org/2932563002/#ps340001
(title: "oshima patch take 2")

[commit-bot: I haz the power, 2017-06-21 18:07:21]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 19:41:47]

CQ is committing da patch.

Bot data: {"patchset_id": 340001, "attempt_start_ts": 1498068425458670,
"parent_rev": "9e737bfc5e4d818bc14f8282c4a0c241ef810c0a", "commit_rev":
"36cfb953ea04dac15159b516cd9ce741c6b7384b"}

[commit-bot: I haz the power, 2017-06-21 19:42:01]

Description was changed from

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798,734807,734809
==========

to

==========
Implement cursor changing on Mushrome.

This splits AshNativeCursorManager into an interface and creates two
implementations: one for classic ash (the original code), and one for
mushrome. The new mushrome implementation forwards to the mus window
manager interface instead of the individual windows due to how mus gets
cursors only from windows which have event handlers.

This leaves some features unimplemented, such as cursor sets, but
implements the core interface.

BUG=729798,734807,734809

Review-Url: https://codereview.chromium.org/2932563002
Cr-Commit-Position: refs/heads/master@{#481278}
Committed:
https://chromium.googlesource.com/chromium/src/+/36cfb953ea04dac15159b516cd9c...
==========

[commit-bot: I haz the power, 2017-06-21 19:42:05]

Committed patchset #18 (id:340001) as
https://chromium.googlesource.com/chromium/src/+/36cfb953ea04dac15159b516cd9c...