mus: Use TooltipManagerAura in NativeWidgetMus.

[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of NativeWidgetMus::
{Get,Set}NativeWindowProperty. corewm code assumes that properties that
get set on a Widget get set on that Widget's NativeWidget's aura content
window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to
resolve if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003

Committed: https://crrev.com/c60b24dc5fe1bd688177d5a6cd53bafd36a5c636
Cr-Commit-Position: refs/heads/master@{#423970}

[Elliot Glaysher, 2016-10-04 20:41:04]

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

[commit-bot: I haz the power, 2016-10-04 20:41:45]

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, 2016-10-04 22:13:31]

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

[commit-bot: I haz the power, 2016-10-04 22:13:32]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2016-10-04 22:16:51]

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

[commit-bot: I haz the power, 2016-10-04 22:17:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2016-10-04 22:26:46]

Description was changed from

==========
mus: Use TooltipManagerAura in NativeWidgetMus.

mash now shows tooltips on hover over items.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003
==========

to

==========
[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of
NativeWidgetMus::{Get,Set}NativeWindowProperty. corewm code assumes that
properties that get set on a Widget get set on that Widget's NativeWidget's aura
content window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to resolve
if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003
==========

[Elliot Glaysher, 2016-10-04 22:26:46]

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

[Elliot Glaysher, 2016-10-04 22:27:04]

Description was changed from

==========
[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of
NativeWidgetMus::{Get,Set}NativeWindowProperty. corewm code assumes that
properties that get set on a Widget get set on that Widget's NativeWidget's aura
content window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to resolve
if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003
==========

to

==========
[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of NativeWidgetMus::
{Get,Set}NativeWindowProperty. corewm code assumes that properties that
get set on a Widget get set on that Widget's NativeWidget's aura content
window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to
resolve if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003
==========

[Elliot Glaysher, 2016-10-04 22:27:19]

Changeset 1 is the previous patch, changeset 2 is the fixes.

https://codereview.chromium.org/2392063003/diff/20001/ui/views/mus/screen_mus.cc
File ui/views/mus/screen_mus.cc (right):

https://codereview.chromium.org/2392063003/diff/20001/ui/views/mus/screen_mus...
ui/views/mus/screen_mus.cc:92:
static_cast<internal::NativeWidgetPrivate*>(nw_mus)->GetNativeView();
AFAICT based on the windows implementation of GetWindowAtScreenPoint, we
*aren't* supposed to recurse into |aura_window|. This method is supposed to
return the toplevel content window associated with the native widget.

[commit-bot: I haz the power, 2016-10-04 23:21:26]

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

[commit-bot: I haz the power, 2016-10-04 23:21:27]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-05 16:10:52]

https://codereview.chromium.org/2392063003/diff/1/ui/views/mus/native_widget_...
File ui/views/mus/native_widget_mus.h (right):

https://codereview.chromium.org/2392063003/diff/1/ui/views/mus/native_widget_...
ui/views/mus/native_widget_mus.h:10: #include <map>
If you can, remove this now.

https://codereview.chromium.org/2392063003/diff/40001/ui/views/corewm/tooltip...
File ui/views/corewm/tooltip_controller_unittest.cc (right):

https://codereview.chromium.org/2392063003/diff/40001/ui/views/corewm/tooltip...
ui/views/corewm/tooltip_controller_unittest.cc:499: widget_->SetCapture(view_);
Currently all views_mus_unittests use the same mus-ws. This means capture in one
test effects all. In other words tests doing capture should be moved to
interactive ui tests. Really we shouldn't have capture in any unit tests. If you
want to early out for now if run in mus that's fine. I suspect this is the
reason CloseOnCaptureLost is marked flaky above.

https://codereview.chromium.org/2392063003/diff/40001/ui/views/mus/window_man...
File ui/views/mus/window_manager_connection.cc (right):

https://codereview.chromium.org/2392063003/diff/40001/ui/views/mus/window_man...
ui/views/mus/window_manager_connection.cc:173: if
(root->bounds().Contains(point))
Don't you want to recurse? Also, you should offset by the bounds of the display
the window is on.

[Elliot Glaysher, 2016-10-05 20:50:46]

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

[commit-bot: I haz the power, 2016-10-05 20:51:32]

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, 2016-10-05 21:51:27]

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

[commit-bot: I haz the power, 2016-10-05 21:51:27]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2016-10-06 17:58:57]

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

[commit-bot: I haz the power, 2016-10-06 17:59:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2016-10-06 18:00:09]

https://codereview.chromium.org/2392063003/diff/1/ui/views/mus/native_widget_...
File ui/views/mus/native_widget_mus.h (right):

https://codereview.chromium.org/2392063003/diff/1/ui/views/mus/native_widget_...
ui/views/mus/native_widget_mus.h:10: #include <map>
On 2016/10/05 16:10:52, sky wrote:
> If you can, remove this now.

There are additional map members that lint warns on.

https://codereview.chromium.org/2392063003/diff/40001/ui/views/corewm/tooltip...
File ui/views/corewm/tooltip_controller_unittest.cc (right):

https://codereview.chromium.org/2392063003/diff/40001/ui/views/corewm/tooltip...
ui/views/corewm/tooltip_controller_unittest.cc:499: widget_->SetCapture(view_);
On 2016/10/05 16:10:52, sky wrote:
> Currently all views_mus_unittests use the same mus-ws. This means capture in
one
> test effects all. In other words tests doing capture should be moved to
> interactive ui tests. Really we shouldn't have capture in any unit tests. If
you
> want to early out for now if run in mus that's fine. I suspect this is the
> reason CloseOnCaptureLost is marked flaky above.

Done.

https://codereview.chromium.org/2392063003/diff/40001/ui/views/mus/window_man...
File ui/views/mus/window_manager_connection.cc (right):

https://codereview.chromium.org/2392063003/diff/40001/ui/views/mus/window_man...
ui/views/mus/window_manager_connection.cc:173: if
(root->bounds().Contains(point))
On 2016/10/05 16:10:52, sky wrote:
> Don't you want to recurse? Also, you should offset by the bounds of the
display
> the window is on.

Done.

[commit-bot: I haz the power, 2016-10-06 18:58:46]

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

[commit-bot: I haz the power, 2016-10-06 18:58:46]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-06 21:23:42]

https://codereview.chromium.org/2392063003/diff/80001/ui/views/mus/window_man...
File ui/views/mus/window_manager_connection.cc (right):

https://codereview.chromium.org/2392063003/diff/80001/ui/views/mus/window_man...
ui/views/mus/window_manager_connection.cc:47: const int64_t window_display_id =
window->GetRoot()->display_id();
Each window has a display id, so you should be able to use window->display_id()
here.

https://codereview.chromium.org/2392063003/diff/80001/ui/views/mus/window_man...
ui/views/mus/window_manager_connection.cc:48: for (display::Display display :
displays) {
As you're going to be doing this in a loop every time I think it's worth
building a map display_id to display that you pass to this function.

https://codereview.chromium.org/2392063003/diff/80001/ui/views/mus/window_man...
ui/views/mus/window_manager_connection.cc:199: ui::Window*
WindowManagerConnection::GetWindowAtScreenPoint(
Please add test coverage of this.

[Elliot Glaysher, 2016-10-06 23:02:45]

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

[commit-bot: I haz the power, 2016-10-06 23:03:21]

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, 2016-10-06 23:08:33]

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

[commit-bot: I haz the power, 2016-10-06 23:08:33]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Elliot Glaysher, 2016-10-07 17:07:00]

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

[commit-bot: I haz the power, 2016-10-07 17:07:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2016-10-07 17:27:28]

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

[commit-bot: I haz the power, 2016-10-07 17:27:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Elliot Glaysher, 2016-10-07 18:06:50]

ptal

[commit-bot: I haz the power, 2016-10-07 18:22:30]

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

[commit-bot: I haz the power, 2016-10-07 18:22:31]

Dry run: This issue passed the CQ dry run.

[sky, 2016-10-07 20:00:08]

LGTM

https://codereview.chromium.org/2392063003/diff/140001/ui/views/mus/window_ma...
File ui/views/mus/window_manager_connection.cc (right):

https://codereview.chromium.org/2392063003/diff/140001/ui/views/mus/window_ma...
ui/views/mus/window_manager_connection.cc:43: ui::Window* GetWindowFrom(const
std::map<int64_t, gfx::Point>& display_origins,
Please add a description, especially as to what display_origins is.

[Elliot Glaysher, 2016-10-07 20:07:27]

The CQ bit was checked by erg@chromium.org

[Elliot Glaysher, 2016-10-07 20:07:28]

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

[commit-bot: I haz the power, 2016-10-07 20:07:52]

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, 2016-10-07 21:30:33]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-10-07 21:32:28]

Description was changed from

==========
[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of NativeWidgetMus::
{Get,Set}NativeWindowProperty. corewm code assumes that properties that
get set on a Widget get set on that Widget's NativeWidget's aura content
window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to
resolve if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003
==========

to

==========
[reland] mus: Use TooltipManagerAura in NativeWidgetMus.

This reland fixes the implementation of NativeWidgetMus::
{Get,Set}NativeWindowProperty. corewm code assumes that properties that
get set on a Widget get set on that Widget's NativeWidget's aura content
window.

It also implements ScreenMus::GetWindowAtScreenPoint(), which is used to
resolve if a window in the current application has capture.

BUG=599558
First Review Url: https://codereview.chromium.org/2379073003

Committed: https://crrev.com/c60b24dc5fe1bd688177d5a6cd53bafd36a5c636
Cr-Commit-Position: refs/heads/master@{#423970}
==========

[commit-bot: I haz the power, 2016-10-07 21:32:29]

Patchset 9 (id:??) landed as
https://crrev.com/c60b24dc5fe1bd688177d5a6cd53bafd36a5c636
Cr-Commit-Position: refs/heads/master@{#423970}