The branch point has passed, so it's good time to refactor the code.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb commit 25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb Author: yoshiki <yoshiki@chromium.org> Date: Mon May 08 07:44:39 2017 Retrieve pinned flag directly from Notification class This CL removes the IsPinned getter method from CustomNotificationContentViewDelegate. The flag "pinned" can be retrieved directly from Notification class, and doesn't need to use delegate. This is refactoring CL and shouldn't change any behavior. BUG=717455 TEST=manual Review-Url: https://codereview.chromium.org/2853783004 Cr-Commit-Position: refs/heads/master@{#469915} [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/arc/notification/arc_custom_notification_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/custom_notification_content_view_delegate.h [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/custom_notification_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/custom_notification_view.h [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/custom_notification_view_unittest.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/message_center_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/message_list_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/message_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/message_view.h [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/notification_view.cc [modify] https://crrev.com/25d817a0ebe1e8c830e56cc20adedb9fd7b7deeb/ui/message_center/views/notification_view.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/021f1b2f0d59d8db33f90e1d1ca0fb3b2c23ba23 commit 021f1b2f0d59d8db33f90e1d1ca0fb3b2c23ba23 Author: yoshiki <yoshiki@chromium.org> Date: Wed Jun 14 05:46:57 2017 Detach notification surface A attached surface may be changed to another, so we need to detach the old one when it gets unnecessary. BUG=717455 Review-Url: https://codereview.chromium.org/2941703002 Cr-Commit-Position: refs/heads/master@{#479299} [modify] https://crrev.com/021f1b2f0d59d8db33f90e1d1ca0fb3b2c23ba23/ui/arc/notification/arc_notification_content_view.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0962c12a78af3ea8488f9a0525df87651eb48c52 commit 0962c12a78af3ea8488f9a0525df87651eb48c52 Author: yoshiki <yoshiki@chromium.org> Date: Thu Jun 29 13:15:48 2017 Extract the view of control buttons on notification into a separated class This patch adds NotificationControlButtonsView, which is the view for the control buttons. It was implemented in ArcNotificationContentView and this patch extracts it into a separated class. In near feature, NotificationControlButtonsView is shared among non-ARC notification as well. BUG=717455 TEST=manual test, tests passes Review-Url: https://codereview.chromium.org/2960333003 Cr-Commit-Position: refs/heads/master@{#483346} [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/arc/notification/arc_notification_content_view.h [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/arc/notification/arc_notification_content_view_unittest.cc [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/arc/notification/arc_notification_delegate.cc [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/arc/notification/arc_notification_delegate.h [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/BUILD.gn [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/views/message_view.cc [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/views/message_view.h [add] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/views/notification_control_buttons_view.cc [add] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/views/notification_control_buttons_view.h [modify] https://crrev.com/0962c12a78af3ea8488f9a0525df87651eb48c52/ui/message_center/views/notification_view.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/12cad72cd90108551c79ebde78ed564832478707 commit 12cad72cd90108551c79ebde78ed564832478707 Author: yoshiki <yoshiki@chromium.org> Date: Wed Jul 05 08:50:04 2017 Use shared NotificationControlButtonsView for non-arc notification buttons We recently introduced NotificationControlButtonsView for displaying control buttons. ARC notification already uses it. This CL makes non-ARC notifications use it. BUG=717455 TEST=unittest passes Review-Url: https://codereview.chromium.org/2964973002 Cr-Commit-Position: refs/heads/master@{#484219} [modify] https://crrev.com/12cad72cd90108551c79ebde78ed564832478707/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/12cad72cd90108551c79ebde78ed564832478707/ui/message_center/views/notification_view.cc [modify] https://crrev.com/12cad72cd90108551c79ebde78ed564832478707/ui/message_center/views/notification_view.h [modify] https://crrev.com/12cad72cd90108551c79ebde78ed564832478707/ui/message_center/views/notification_view_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f296638b446e73d14fb90901448cf117d03669c commit 5f296638b446e73d14fb90901448cf117d03669c Author: glider <glider@chromium.org> Date: Wed Jul 05 13:09:19 2017 Revert of Use shared NotificationControlButtonsView for non-arc notification buttons (patchset #3 id:60001 of https://codereview.chromium.org/2964973002/ ) Reason for revert: Reverting, as this patch caused numerous leaks (see https://bugs.chromium.org/p/chromium/issues/detail?id=739356) Original issue's description: > Use shared NotificationControlButtonsView for non-arc notification buttons > > We recently introduced NotificationControlButtonsView for displaying control buttons. ARC notification already uses it. This CL makes non-ARC notifications use it. > > BUG=717455 > TEST=unittest passes > > Review-Url: https://codereview.chromium.org/2964973002 > Cr-Commit-Position: refs/heads/master@{#484219} > Committed: https://chromium.googlesource.com/chromium/src/+/12cad72cd90108551c79ebde78ed564832478707 TBR=yhanada@chromium.org,yoshiki@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=717455 Review-Url: https://codereview.chromium.org/2970953002 Cr-Commit-Position: refs/heads/master@{#484255} [modify] https://crrev.com/5f296638b446e73d14fb90901448cf117d03669c/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/5f296638b446e73d14fb90901448cf117d03669c/ui/message_center/views/notification_view.cc [modify] https://crrev.com/5f296638b446e73d14fb90901448cf117d03669c/ui/message_center/views/notification_view.h [modify] https://crrev.com/5f296638b446e73d14fb90901448cf117d03669c/ui/message_center/views/notification_view_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/37f1fcbb6e58b5414e2fa310e3a26dea0865656e commit 37f1fcbb6e58b5414e2fa310e3a26dea0865656e Author: yoshiki <yoshiki@chromium.org> Date: Wed Jul 05 13:46:16 2017 Reland of Use shared NotificationControlButtonsView for non-arc notification buttons (patchset #1 id:1 of https://codereview.chromium.org/2970953002/ ) Reason for revert: Let me reland this CL. This should break the ASAN but the fix is ready and I'll commit it soon after this. Fix CL: https://codereview.chromium.org/2967133002/ Original issue's description: > Revert of Use shared NotificationControlButtonsView for non-arc notification buttons (patchset #3 id:60001 of https://codereview.chromium.org/2964973002/ ) > > Reason for revert: > Reverting, as this patch caused numerous leaks (see https://bugs.chromium.org/p/chromium/issues/detail?id=739356) > > Original issue's description: > > Use shared NotificationControlButtonsView for non-arc notification buttons > > > > We recently introduced NotificationControlButtonsView for displaying control buttons. ARC notification already uses it. This CL makes non-ARC notifications use it. > > > > BUG=717455 > > TEST=unittest passes > > > > Review-Url: https://codereview.chromium.org/2964973002 > > Cr-Commit-Position: refs/heads/master@{#484219} > > Committed: https://chromium.googlesource.com/chromium/src/+/12cad72cd90108551c79ebde78ed564832478707 > > TBR=yhanada@chromium.org,yoshiki@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=717455 > > Review-Url: https://codereview.chromium.org/2970953002 > Cr-Commit-Position: refs/heads/master@{#484255} > Committed: https://chromium.googlesource.com/chromium/src/+/5f296638b446e73d14fb90901448cf117d03669c TBR=yhanada@chromium.org,glider@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=717455 Review-Url: https://codereview.chromium.org/2966063003 Cr-Commit-Position: refs/heads/master@{#484261} [modify] https://crrev.com/37f1fcbb6e58b5414e2fa310e3a26dea0865656e/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/37f1fcbb6e58b5414e2fa310e3a26dea0865656e/ui/message_center/views/notification_view.cc [modify] https://crrev.com/37f1fcbb6e58b5414e2fa310e3a26dea0865656e/ui/message_center/views/notification_view.h [modify] https://crrev.com/37f1fcbb6e58b5414e2fa310e3a26dea0865656e/ui/message_center/views/notification_view_unittest.cc
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/084388c254c2cde590a4e3df8601ac1e1ed84c88 commit 084388c254c2cde590a4e3df8601ac1e1ed84c88 Author: yoshiki iguchi <yoshiki@chromium.org> Date: Fri Jul 21 06:12:09 2017 Move the control buttons from dedicate widget to view Previously, the buttons are on the dedicate widget to show them above the ARC notification (NativeViewHost). This CL moves them to a view. In addition, this CL adds a hacks for ARC notification which manipulates the hierarycy of layers to show the buttons at top most. Bug: b/62771497 Bug: 717455 Bug: 747217 Change-Id: I876c653e01d4a4237055bd4f763aa55017ba38ef Reviewed-on: https://chromium-review.googlesource.com/579107 Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#488599} [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_content_view.h [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_content_view_delegate.h [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_content_view_unittest.cc [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_view.cc [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_view.h [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/arc/notification/arc_notification_view_unittest.cc [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/084388c254c2cde590a4e3df8601ac1e1ed84c88/ui/message_center/views/notification_control_buttons_view.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b14f20f949abe14f4317b525f279e95004512b4f commit b14f20f949abe14f4317b525f279e95004512b4f Author: Yoshiki Iguchi <yoshiki@chromium.org> Date: Tue Jul 25 10:28:24 2017 Revert "Move the control buttons from dedicate widget to view" This reverts commit 084388c254c2cde590a4e3df8601ac1e1ed84c88. Reason for revert: This change breaks some layout in notification. Original change's description: > Move the control buttons from dedicate widget to view > > Previously, the buttons are on the dedicate widget to show them above > the ARC notification (NativeViewHost). This CL moves them to a view. > > In addition, this CL adds a hacks for ARC notification which manipulates > the hierarycy of layers to show the buttons at top most. > > Bug: b/62771497 > Bug: 717455 > Bug: 747217 > Change-Id: I876c653e01d4a4237055bd4f763aa55017ba38ef > Reviewed-on: https://chromium-review.googlesource.com/579107 > Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> > Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> > Cr-Commit-Position: refs/heads/master@{#488599} TBR=yoshiki@chromium.org,yhanada@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: b/62771497, 717455, 747217 Change-Id: I2a4cbf639c1a13c910a1424589074d739fbc259b Reviewed-on: https://chromium-review.googlesource.com/584215 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Commit-Queue: Yoshiki Iguchi <yoshiki@chromium.org> Cr-Commit-Position: refs/heads/master@{#489271} [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view_delegate.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_content_view_unittest.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view.h [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/arc/notification/arc_notification_view_unittest.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/message_center/views/notification_control_buttons_view.cc [modify] https://crrev.com/b14f20f949abe14f4317b525f279e95004512b4f/ui/message_center/views/notification_control_buttons_view.h
Comment 1 by bugdroid1@chromium.org
, May 8 2017