Set slide_out_enabled property for CustomNotification

Set slide_out_enabled property for CustomNotification

The "slide_out_enabled" property of SlideOutView should be set according to the "pinned" property of notification. It was set for both CustomNotificationView and NotificationView correctly, but now the code has been moved to NotificationView after recent refactoring.

This patch moves the code NotificationView to MessageView and fixes the regression.

BUG=680353
TEST=manually

Review-Url: https://codereview.chromium.org/2620133003
Cr-Commit-Position: refs/heads/master@{#444275}
Committed: https://chromium.googlesource.com/chromium/src/+/0993bc30936c72dfdd60ec7c15aacc1579756704

[yoshiki, 2017-01-11 11:32:24]

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

[commit-bot: I haz the power, 2017-01-11 11:32: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, 2017-01-11 12:30:51]

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

[commit-bot: I haz the power, 2017-01-11 12:30:51]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-01-12 02:19:54]

Description was changed from

==========
wip

.

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

to

==========
Set slide_out_enabled property for CustomNotification

The "slide_out_enabled" property of SlideOutView should be set according to the
"pinned" property of notification. It was set for both CustomNotificationView
and NotificationView correctly, but now the code has been moved to
NotificationView after recent refactoring.

This patch moves the code NotificationView to MessageView and fixes the
regression.

BUG=680353
TEST=manually
==========

[yoshiki, 2017-01-12 02:19:54]

yoshiki@chromium.org changed reviewers:
+ yhanada@chromium.org

[yoshiki, 2017-01-12 02:20:35]

Hanada-san, PTAL since you touched the code around recently?

[yhanada, 2017-01-12 05:51:58]

lgtm

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

https://codereview.chromium.org/2620133003/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:89:
set_slide_out_enabled(!notification.pinned());
optional: How about calling UpdateWithNotification() here?

[yoshiki, 2017-01-16 05:56:16]

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

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

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

[yoshiki, 2017-01-16 06:08:50]

yoshiki@chromium.org changed reviewers:
+ dewittj@chromium.org

[yoshiki, 2017-01-16 06:08:50]

Justin, PTAL. Thanks

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

https://codereview.chromium.org/2620133003/diff/1/ui/message_center/views/mes...
ui/message_center/views/message_view.cc:89:
set_slide_out_enabled(!notification.pinned());
On 2017/01/12 05:51:58, yhanada wrote:
> optional: How about calling UpdateWithNotification() here?

Good idea! done.

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

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

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

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

[yoshiki, 2017-01-16 07:13:43]

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

[commit-bot: I haz the power, 2017-01-16 07:13:51]

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

[yoshiki, 2017-01-16 07:56:03]

Patchset #2 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-01-16 07:58:46]

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

[commit-bot: I haz the power, 2017-01-16 07:58:47]

Dry run: This issue passed the CQ dry run.

[dewittj, 2017-01-17 17:24:45]

lgtm

https://codereview.chromium.org/2620133003/diff/40001/ui/message_center/views...
File ui/message_center/views/custom_notification_view_unittest.cc (right):

https://codereview.chromium.org/2620133003/diff/40001/ui/message_center/views...
ui/message_center/views/custom_notification_view_unittest.cc:36:
std::unique_ptr<ui::GestureEvent> GenerateGestureVerticalScrollUpdateEvent(
Shouldn't this be Horizontal, not vertical?

[yoshiki, 2017-01-18 03:40:38]

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

[commit-bot: I haz the power, 2017-01-18 03:40:58]

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-01-18 04:25:38]

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

[commit-bot: I haz the power, 2017-01-18 04:25:39]

Dry run: This issue passed the CQ dry run.

[yoshiki, 2017-01-18 04:32:18]

Thanks!

https://codereview.chromium.org/2620133003/diff/40001/ui/message_center/views...
File ui/message_center/views/custom_notification_view_unittest.cc (right):

https://codereview.chromium.org/2620133003/diff/40001/ui/message_center/views...
ui/message_center/views/custom_notification_view_unittest.cc:36:
std::unique_ptr<ui::GestureEvent> GenerateGestureVerticalScrollUpdateEvent(
On 2017/01/17 17:24:44, dewittj wrote:
> Shouldn't this be Horizontal, not vertical?

You're right! Thanks.

[yoshiki, 2017-01-18 04:32:23]

The CQ bit was checked by yoshiki@chromium.org

[yoshiki, 2017-01-18 04:32:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
yhanada@chromium.org, dewittj@chromium.org
Link to the patchset: https://codereview.chromium.org/2620133003/#ps60001
(title: "Fix the wrong term: vertical -> horizontal")

[commit-bot: I haz the power, 2017-01-18 04:32:40]

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-01-18 04:39:11]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484713943163200,
"parent_rev": "9ed33cfc1ccc5146fbacba753218646cb3071658", "commit_rev":
"0993bc30936c72dfdd60ec7c15aacc1579756704"}

[commit-bot: I haz the power, 2017-01-18 04:39:39]

Description was changed from

==========
Set slide_out_enabled property for CustomNotification

The "slide_out_enabled" property of SlideOutView should be set according to the
"pinned" property of notification. It was set for both CustomNotificationView
and NotificationView correctly, but now the code has been moved to
NotificationView after recent refactoring.

This patch moves the code NotificationView to MessageView and fixes the
regression.

BUG=680353
TEST=manually
==========

to

==========
Set slide_out_enabled property for CustomNotification

The "slide_out_enabled" property of SlideOutView should be set according to the
"pinned" property of notification. It was set for both CustomNotificationView
and NotificationView correctly, but now the code has been moved to
NotificationView after recent refactoring.

This patch moves the code NotificationView to MessageView and fixes the
regression.

BUG=680353
TEST=manually

Review-Url: https://codereview.chromium.org/2620133003
Cr-Commit-Position: refs/heads/master@{#444275}
Committed:
https://chromium.googlesource.com/chromium/src/+/0993bc30936c72dfdd60ec7c15aa...
==========

[commit-bot: I haz the power, 2017-01-18 04:39:40]

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