Implement hit tests/client area.

views_aura_mua: Implement hit tests/client area.

BUG=663525
Committed: https://crrev.com/63a80c37206d7e07759164e4f86e3b545bab9b95
Cr-Commit-Position: refs/heads/master@{#433996}

[Elliot Glaysher, 2016-11-16 21:23:43]

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

[commit-bot: I haz the power, 2016-11-16 21:24: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, 2016-11-16 22:03:59]

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

[commit-bot: I haz the power, 2016-11-16 22:04:00]

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

[Elliot Glaysher, 2016-11-16 23:00:55]

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

[commit-bot: I haz the power, 2016-11-16 23:01:35]

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-11-17 00:15:18]

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

[commit-bot: I haz the power, 2016-11-17 00:17:04]

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-11-17 01:38:21]

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

[commit-bot: I haz the power, 2016-11-17 01:38:22]

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, 2016-11-17 22:04:34]

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

[commit-bot: I haz the power, 2016-11-17 22:06:13]

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-11-17 22:47:30]

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

[commit-bot: I haz the power, 2016-11-17 22:47:31]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  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-11-17 23:48:30]

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

[commit-bot: I haz the power, 2016-11-17 23:50:25]

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-11-18 00:27:22]

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

[commit-bot: I haz the power, 2016-11-18 00:27:23]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Elliot Glaysher, 2016-11-18 01:20:47]

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

[commit-bot: I haz the power, 2016-11-18 01:22:51]

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-11-18 03:38:35]

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

[commit-bot: I haz the power, 2016-11-18 03:38:36]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2016-11-18 20:28:16]

Description was changed from

==========
Implement hit tests/client area.

BUG=
==========

to

==========
Implement hit tests/client area.

BUG=663525
==========

[Elliot Glaysher, 2016-11-18 21:12:17]

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

[Elliot Glaysher, 2016-11-18 21:12:34]

Description was changed from

==========
Implement hit tests/client area.

BUG=663525
==========

to

==========
views_aura_mua: Implement hit tests/client area.

BUG=663525
==========

[Elliot Glaysher, 2016-11-18 21:12:34]

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

[commit-bot: I haz the power, 2016-11-18 21:13: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, 2016-11-18 21:14:54]

This implements basic hit tests/client area support. These fields also need to
be updated when we have more complete set of host events, but this should cover
the basics. 

The testing here is done purely at the aura to mus layer because I had some
problems mocking out some of the higher views code.

https://codereview.chromium.org/2507963002/diff/100001/ui/aura/mus/DEPS
File ui/aura/mus/DEPS (right):

https://codereview.chromium.org/2507963002/diff/100001/ui/aura/mus/DEPS#newco...
ui/aura/mus/DEPS:15: "+ui/wm/core/wm_state.h",
We already include this in //ui/aura/tests.

[commit-bot: I haz the power, 2016-11-18 23:23:16]

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

[commit-bot: I haz the power, 2016-11-18 23:23:17]

Dry run: This issue passed the CQ dry run.

[sky, 2016-11-19 15:29:32]

Is using WindowManagerFrameValues planned for a future cl? See
ClientSideNonClientFrameView in NativeWidgetMus.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/BUILD.gn
File ui/aura/BUILD.gn (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/BUILD.gn#newco...
ui/aura/BUILD.gn:354: "//ui/wm:wm",
no need for the ":wm"

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/DEPS
File ui/aura/mus/DEPS (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/DEPS#newco...
ui/aura/mus/DEPS:16: "+ui/wm/core/wm_state.h",
The only file we want aura to use from ui/wm is window_types. Everything else
should be outside of aura. That said, it's fine for tests to use ui/wm. I
believe you only need this include for tests. Can you add a
specific_include_rules for ui/wm? You can do it for all tests files.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_client.cc:1509:
SetClientArea(window_tree_host->window(), insets, std::vector<gfx::Rect>());
optional: I don't think anyone else is calling SetClientArea/ClearHitTestMask
and SetHitTestMask. How about moving the implementations to these functions?

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_host_mus_delegate.h (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_host_mus_delegate.h:34: virtual void
OnWindowTreeHostClearHitTestMask(
How about a single function for this that takes a const gfx::Rect* null means
clear?

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_host_mus_unittest.cc (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_host_mus_unittest.cc:28: // WindowTreeHostMus path at
all. So instead, we create a minimal environment
There is a TODO about making the default display and root do something different
for mus, but you can certainly subclass AuraMusClientTestBase and then create
your own WindowTreeHostMus. See WindowTreeClientClientTest.InputEventBasic as an
example of this. I think that should work for your case here and would mean you
don't need to copy all this boilerplate. By this I mean:
using WindowTreeHostMusTest = AuraMusClientTestBase;
and create a WindowTreeHostMus() in your test. window_tree() returns the
TestWindowTree.

https://codereview.chromium.org/2507963002/diff/120001/ui/views/mus/desktop_w...
File ui/views/mus/desktop_window_tree_host_mus.h (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/views/mus/desktop_w...
ui/views/mus/desktop_window_tree_host_mus.h:36: void UpdateClientArea();
optional: It isn't clear what these do. Maybe SendClientAreaToServer or at least
better descriptions.

[Elliot Glaysher, 2016-11-21 20:39:50]

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

[commit-bot: I haz the power, 2016-11-21 20:40:44]

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-11-21 21:25:50]

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

[commit-bot: I haz the power, 2016-11-21 21:26: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, 2016-11-21 22:00:46]

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

[commit-bot: I haz the power, 2016-11-21 22:00:47]

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_...)
  linux_chromium_clobber_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_...)

[Elliot Glaysher, 2016-11-21 23:11:45]

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

[commit-bot: I haz the power, 2016-11-21 23:12:31]

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-11-22 00:11:16]

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

[commit-bot: I haz the power, 2016-11-22 00:11:18]

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, 2016-11-22 00:48:07]

Also moved the ClientSideNonClientFrameView to the DesktopWindowTreeHostMus and
moved the current code in DesktopNativeWidgetAura to the other DWTHs.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/BUILD.gn
File ui/aura/BUILD.gn (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/BUILD.gn#newco...
ui/aura/BUILD.gn:354: "//ui/wm:wm",
On 2016/11/19 15:29:31, sky wrote:
> no need for the ":wm"

Deleted line entirely.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/DEPS
File ui/aura/mus/DEPS (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/DEPS#newco...
ui/aura/mus/DEPS:16: "+ui/wm/core/wm_state.h",
On 2016/11/19 15:29:31, sky wrote:
> The only file we want aura to use from ui/wm is window_types. Everything else
> should be outside of aura. That said, it's fine for tests to use ui/wm. I
> believe you only need this include for tests. Can you add a
> specific_include_rules for ui/wm? You can do it for all tests files.

Reverted due to changes in how the tests work.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_client.cc:1509:
SetClientArea(window_tree_host->window(), insets, std::vector<gfx::Rect>());
On 2016/11/19 15:29:31, sky wrote:
> optional: I don't think anyone else is calling SetClientArea/ClearHitTestMask
> and SetHitTestMask. How about moving the implementations to these functions?

Done.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_host_mus_delegate.h (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_host_mus_delegate.h:34: virtual void
OnWindowTreeHostClearHitTestMask(
On 2016/11/19 15:29:31, sky wrote:
> How about a single function for this that takes a const gfx::Rect* null means
> clear?

Done.

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_host_mus_unittest.cc (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_host_mus_unittest.cc:28: // WindowTreeHostMus path at
all. So instead, we create a minimal environment
On 2016/11/19 15:29:31, sky wrote:
> There is a TODO about making the default display and root do something
different
> for mus, but you can certainly subclass AuraMusClientTestBase and then create
> your own WindowTreeHostMus. See WindowTreeClientClientTest.InputEventBasic as
an
> example of this. I think that should work for your case here and would mean
you
> don't need to copy all this boilerplate. By this I mean:
> using WindowTreeHostMusTest = AuraMusClientTestBase;
> and create a WindowTreeHostMus() in your test. window_tree() returns the
> TestWindowTree.

Done.

The confusion on my end was having aura::WindowTreeClient and
ui::mojom::WindowTreeClient. Not a fan of having the same class name in
different namespaces because we don't always fully qualify some of these names.

https://codereview.chromium.org/2507963002/diff/120001/ui/views/mus/desktop_w...
File ui/views/mus/desktop_window_tree_host_mus.h (right):

https://codereview.chromium.org/2507963002/diff/120001/ui/views/mus/desktop_w...
ui/views/mus/desktop_window_tree_host_mus.h:36: void UpdateClientArea();
On 2016/11/19 15:29:31, sky wrote:
> optional: It isn't clear what these do. Maybe SendClientAreaToServer or at
least
> better descriptions.

Done.

[sky, 2016-11-22 04:19:09]

https://codereview.chromium.org/2507963002/diff/180001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2507963002/diff/180001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_client.cc:1467: // TODO(erg): Currently, nobody uses
|additional_client_area_in_pixels|.
It's used in two places. See NonClientFrameController in ash/mus, and see
event_dispatcher in services/ui/ws. So, we definitely still need it.

[Elliot Glaysher, 2016-11-22 18:30:52]

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

[commit-bot: I haz the power, 2016-11-22 18:31: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, 2016-11-22 18:57:12]

https://codereview.chromium.org/2507963002/diff/180001/ui/aura/mus/window_tre...
File ui/aura/mus/window_tree_client.cc (right):

https://codereview.chromium.org/2507963002/diff/180001/ui/aura/mus/window_tre...
ui/aura/mus/window_tree_client.cc:1467: // TODO(erg): Currently, nobody uses
|additional_client_area_in_pixels|.
On 2016/11/22 04:19:09, sky wrote:
> It's used in two places. See NonClientFrameController in ash/mus, and see
> event_dispatcher in services/ui/ws. So, we definitely still need it.

Done.

[commit-bot: I haz the power, 2016-11-22 19:02:38]

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

[commit-bot: I haz the power, 2016-11-22 19:02:39]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[sky, 2016-11-22 20:53:52]

LGTM.

The places I pointed you are don't actually set the additional insets. Setting
is done here:
https://chromium.googlesource.com/chromium/src/+/master/chrome/browser/ui/vie...
. Which will need to convert to DesktopNativeWidgetAura soon.

[Elliot Glaysher, 2016-11-22 21:04:15]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2016-11-22 21:05:02]

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-11-22 21:52:58]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1479848655001570,
"parent_rev": "66982989478dc6d20d14c100e17137a8e32699bb", "commit_rev":
"57d70c301cad2d32cf9fea11b5587b61f3861c44"}

[commit-bot: I haz the power, 2016-11-22 21:53:33]

Description was changed from

==========
views_aura_mua: Implement hit tests/client area.

BUG=663525
==========

to

==========
views_aura_mua: Implement hit tests/client area.

BUG=663525
==========

[commit-bot: I haz the power, 2016-11-22 21:53:35]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-11-22 21:56:27]

Description was changed from

==========
views_aura_mua: Implement hit tests/client area.

BUG=663525
==========

to

==========
views_aura_mua: Implement hit tests/client area.

BUG=663525

Committed: https://crrev.com/63a80c37206d7e07759164e4f86e3b545bab9b95
Cr-Commit-Position: refs/heads/master@{#433996}
==========

[commit-bot: I haz the power, 2016-11-22 21:56:28]

Patchset 11 (id:??) landed as
https://crrev.com/63a80c37206d7e07759164e4f86e3b545bab9b95
Cr-Commit-Position: refs/heads/master@{#433996}