CrOS: Fix appearance of notification toasts when sliding out via gesture

CrOS: Fix appearance of notification toasts when sliding out via gesture

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG=716429
Review-Url: https://codereview.chromium.org/2849523005
Cr-Commit-Position: refs/heads/master@{#468339}
Committed: https://chromium.googlesource.com/chromium/src/+/3d7851ff607c370234e4f93ecf66727a8f8498d9

[Evan Stade, 2017-04-28 21:18:32]

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

[commit-bot: I haz the power, 2017-04-28 21:19:06]

Dry run: CQ is trying da patch.

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

[Evan Stade, 2017-04-28 21:21:05]

estade@chromium.org changed reviewers:
+ sky@chromium.org, yoshiki@chromium.org

[Evan Stade, 2017-04-28 21:21:05]

estade@chromium.org changed required reviewers:
+ sky@chromium.org

[Evan Stade, 2017-04-28 21:21:06]

sky@, ptal.
yoshiki@, FYI/optional review.

[commit-bot: I haz the power, 2017-04-28 21:39:34]

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

[commit-bot: I haz the power, 2017-04-28 21:39:35]

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_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2017-04-28 22:35:36]

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.cc (right):

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:214: return is_nested_ ? layer() :
GetWidget()->GetLayer();
Is it possible for this to be called when is_nested_ is false, and no longer in
a widget?

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.h (right):

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.h:118: bool is_nested_ = false;
Is this member necessary? Could you instead key off whether there is a layer?

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
File ui/views/controls/slide_out_controller.h (right):

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:16: // A View that can be closed by a
slide-out touch gesture.
Please update description.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:17: class VIEWS_EXPORT
SlideOutController : public ui::EventHandler,
I would prefer this code move to where it is used, which is in messagecenter.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:22: // Returns the layer to for slide
operations.
'to for' -> 'for' ?

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:42: enum SlideDirection {
enum class?

[Evan Stade, 2017-04-28 23:18:38]

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.cc (right):

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:214: return is_nested_ ? layer() :
GetWidget()->GetLayer();
On 2017/04/28 22:35:36, sky wrote:
> Is it possible for this to be called when is_nested_ is false, and no longer
in
> a widget?

I don't think so. It is only called from inside
SlideOutController::OnGestureEvent.

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
File ui/message_center/views/message_view.h (right):

https://codereview.chromium.org/2849523005/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.h:118: bool is_nested_ = false;
On 2017/04/28 22:35:36, sky wrote:
> Is this member necessary? Could you instead key off whether there is a layer?

I didn't want to do that because then someone adds a layer for some unrelated
reason (to show an ink drop, or something) and suddenly this breaks.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
File ui/views/controls/slide_out_controller.h (right):

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:16: // A View that can be closed by a
slide-out touch gesture.
On 2017/04/28 22:35:36, sky wrote:
> Please update description.

Done.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:17: class VIEWS_EXPORT
SlideOutController : public ui::EventHandler,
On 2017/04/28 22:35:36, sky wrote:
> I would prefer this code move to where it is used, which is in messagecenter.

Done.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:22: // Returns the layer to for slide
operations.
On 2017/04/28 22:35:36, sky wrote:
> 'to for' -> 'for' ?

Done.

https://codereview.chromium.org/2849523005/diff/1/ui/views/controls/slide_out...
ui/views/controls/slide_out_controller.h:42: enum SlideDirection {
On 2017/04/28 22:35:36, sky wrote:
> enum class?

removed this.

[Evan Stade, 2017-04-29 00:23:32]

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

[commit-bot: I haz the power, 2017-04-29 00:23:59]

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-04-29 00:32:54]

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

[commit-bot: I haz the power, 2017-04-29 00:32:55]

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_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Evan Stade, 2017-05-01 16:04:33]

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

[commit-bot: I haz the power, 2017-05-01 16:05:06]

Dry run: CQ is trying da patch.

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

[sky, 2017-05-01 17:05:23]

LGTM

[Evan Stade, 2017-05-01 17:09:57]

The CQ bit was unchecked by estade@chromium.org

[Evan Stade, 2017-05-01 17:10:02]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-05-01 17:10:27]

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-05-01 17:15:52]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493658602518260,
"parent_rev": "1f02534fb0fb3a45f76c16d06da2b46c0625a668", "commit_rev":
"3d7851ff607c370234e4f93ecf66727a8f8498d9"}

[commit-bot: I haz the power, 2017-05-01 17:16:02]

Description was changed from

==========
CrOS: Fix appearance of notification toasts when sliding out via gesture

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG=716429
==========

to

==========
CrOS: Fix appearance of notification toasts when sliding out via gesture

This converts SlideOutView to SlideOutController, which provides all the
same functionality but can be added to any view rather than just those
that extend it.

It allows the target view to control the layer that's sliding out. For
toasts, that is the widget's layer. For notifications inside the message
center, that will continue to be the NotificationView itself.

BUG=716429

Review-Url: https://codereview.chromium.org/2849523005
Cr-Commit-Position: refs/heads/master@{#468339}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d7851ff607c370234e4f93ecf66...
==========

[commit-bot: I haz the power, 2017-05-01 17:16:03]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/3d7851ff607c370234e4f93ecf66...

[sky, 2017-05-01 20:14:35]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2850153003/ by sky@chromium.org.

The reason for reverting is: This appears to have broken
message_center_unittests on chromium.win/Win10 Tests x64. See
https://uberchromegw.corp.google.com/i/chromium.win/builders/Win10%20Tests%20...
for one example..