New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 769219 link

Starred by 1 user
Status: Fixed
Owner:
tetsui@chromium.org
Closed: Oct 25
Cc:
yamaguchi@chromium.org
█ yoshiki@chromium.org
yhanada@chromium.org
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 861909
issue 863213
issue 863590
issue 880167
issue 888175
issue 888484



Sign in to add a comment

Simplify MessageListView

Project Member Reported by edcourtney@chromium.org, Sep 27 2017 
There have been a number of crashes which were directly/indirectly caused by MessageListView. For example:

 crbug.com/692527 
crbug.com/712162
 crbug.com/713983 
crbug.com/737856
crbug.com/725299

In particular, the semantics of view animation (e.g. clear all), view lifetimes, and the |clearing_all_views_|, |deleting_views_|, and |deleted_when_done_| containers make this file very hard to reason about. This makes bugs easy to make and hard to fix.

I think we should try to simplify the logic here to prevent future crashes. At the same time, we should improve the testing in message_list_view_unittest.cc.

Comment 1 by yoshiki@chromium.org, Jun 22 2018

Cc: yamaguchi@chromium.org
Owner: tetsui@chromium.org

Comment 3 by tetsui@chromium.org, Aug 13

Labels: M-71

Comment 4 by tetsui@chromium.org, Aug 13

Blocking: 863213

Comment 5 by tetsui@chromium.org, Aug 13

Blocking: 863590

Comment 6 by tetsui@chromium.org, Aug 13

Blocking: 861909

Comment 7 by tetsui@chromium.org, Sep 26

Status: Started (was: Assigned)

Comment 8 by tetsui@chromium.org, Sep 27

Blocking: 888175

Comment 9 by tetsui@chromium.org, Sep 27

Blocking: 888484
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082

commit 2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Oct 03 03:43:07 2018

Initial implementation of New MessageListView.

This CL adds NewUnifiedMessageCenterView and UnifiedMessageListView
that replace UnifiedMessageCenterView, and MessageListView.

The new implementation is behind a flag
(--enable-features=NewMessageListView). Eventually old implementation
will be removed and NewUnifiedMessageCenterView will be renamed to
UnifiedMessageCenterView.

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest
BUG= 769219 

Change-Id: I1db9d73acfed918bc73b828ef3b406ef5da97a0c
Reviewed-on: https://chromium-review.googlesource.com/c/1244182
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596115}
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/BUILD.gn
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/public/cpp/ash_features.cc
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/public/cpp/ash_features.h
[add] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/new_unified_message_center_view.cc
[add] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/new_unified_message_center_view.h
[rename] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/unified_message_center_view.cc
[rename] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/unified_message_center_view.h
[add] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/unified_message_list_view.cc
[add] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/unified_message_list_view.h
[add] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/message_center/unified_message_list_view_unittest.cc
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/unified/unified_system_tray_view.cc
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/ash/system/unified/unified_system_tray_view.h
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/chrome/browser/about_flags.cc
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/2aa9d1e5e5c8b081d35bfebc2a058851c7fe2082/tools/metrics/histograms/enums.xml
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/617e24c9380a4eab3ab30b6b5a274c2d4901bbd6

commit 617e24c9380a4eab3ab30b6b5a274c2d4901bbd6
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Oct 03 08:39:13 2018

NewMessageListView: Auto collapse notifications

This CL implements auto collapsing / expanding of notifications in
UnifiedMessageListView.

UnifiedMessageListView will replace MessageListView. It's behind a flag:
--enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest.CollapseOlderNotifications
BUG= 769219 

Change-Id: Iff52b748c8a4faf9560586d049621b92baec68dc
Reviewed-on: https://chromium-review.googlesource.com/c/1258668
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596153}
[modify] https://crrev.com/617e24c9380a4eab3ab30b6b5a274c2d4901bbd6/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/617e24c9380a4eab3ab30b6b5a274c2d4901bbd6/ash/system/message_center/unified_message_list_view.h
[modify] https://crrev.com/617e24c9380a4eab3ab30b6b5a274c2d4901bbd6/ash/system/message_center/unified_message_list_view_unittest.cc
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 3 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f562bf689b0108eaff3f36266728ba9ed285afa

commit 4f562bf689b0108eaff3f36266728ba9ed285afa
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Oct 03 09:28:28 2018

NewMessageListView: Add borders and round corners

This CL adds rounded corners and separator lines to notifications in
UnifiedMessageListView.

UnifiedMessageListView will replace MessageListView. It's behind a flag:
--enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest
BUG= 769219 

Change-Id: I9616b8d3e6505046448742e3f8936188e23e0129
Reviewed-on: https://chromium-review.googlesource.com/c/1258783
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596160}
[modify] https://crrev.com/4f562bf689b0108eaff3f36266728ba9ed285afa/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/4f562bf689b0108eaff3f36266728ba9ed285afa/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/4f562bf689b0108eaff3f36266728ba9ed285afa/ash/system/message_center/unified_message_list_view.h
[modify] https://crrev.com/4f562bf689b0108eaff3f36266728ba9ed285afa/ash/system/message_center/unified_message_list_view_unittest.cc
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 5 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/483681a803fe978f559be5bc24792eb15f4203e0

commit 483681a803fe978f559be5bc24792eb15f4203e0
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Fri Oct 05 05:41:04 2018

NewMessageListView: Add Clear All button

This CL adds Clear All button to NewUnifiedMessageCenterView.

NewUnifiedMessageCenterView will replace UnifiedMessageCenterView.
It's behind a flag: --enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=NewUnifiedMessageCenterViewTest
BUG= 769219 

Change-Id: Ibadc6e5a3d799fcbeb6c94199d68656d169295d4
Reviewed-on: https://chromium-review.googlesource.com/c/1260530
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597005}
[modify] https://crrev.com/483681a803fe978f559be5bc24792eb15f4203e0/ash/BUILD.gn
[modify] https://crrev.com/483681a803fe978f559be5bc24792eb15f4203e0/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/483681a803fe978f559be5bc24792eb15f4203e0/ash/system/message_center/new_unified_message_center_view.h
[add] https://crrev.com/483681a803fe978f559be5bc24792eb15f4203e0/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/483681a803fe978f559be5bc24792eb15f4203e0/ash/system/unified/unified_system_tray_view.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 9 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7

commit 0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Oct 09 08:48:37 2018

Refactor NotificationSwipeControlView

This CL removes NotificationSwipeControlView::Observer and moves some
logic from SlidableMessageView to NotificationSwipeControlView.

This is a preparation CL to add slide controls to NewMessageListView.
WIP CL that depends on this cleanup: https://crrev.com/c/1264378

TEST=manual
BUG= 769219 

Change-Id: I1125b7e9c6226797f0cbb2d6b08bdda548140596
Reviewed-on: https://chromium-review.googlesource.com/c/1264100
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597843}
[modify] https://crrev.com/0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7/ash/message_center/notification_swipe_control_view.cc
[modify] https://crrev.com/0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7/ash/message_center/notification_swipe_control_view.h
[modify] https://crrev.com/0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7/ash/message_center/slidable_message_view.cc
[modify] https://crrev.com/0b50f001f3fcd76f43bb26248cbffa2b6dbe85d7/ash/message_center/slidable_message_view.h

Comment 15 by tetsui@chromium.org, Oct 10

Blocking: 880167
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9d6759aef73d42deb4c0570e629c28e83925949

commit e9d6759aef73d42deb4c0570e629c28e83925949
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 11 01:34:09 2018

NewMessageListView: Propagate scroll content size

By https://crrev.com/c/1260530 , UnifiedMessageListView is no longer
a direct child of scroll contents view, and preferred size change
from MessageListView was not propagated.

NewUnifiedMessageCenterView will replace UnifiedMessageCenterView.
It's behind a flag: --enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=NewUnifiedMessageCenterViewTest.ContentsRelayout
BUG= 769219 

Change-Id: Ie0d055d4eb78cf7e7969a117761f167272b0c222
Reviewed-on: https://chromium-review.googlesource.com/c/1270696
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598621}
[modify] https://crrev.com/e9d6759aef73d42deb4c0570e629c28e83925949/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/e9d6759aef73d42deb4c0570e629c28e83925949/ash/system/message_center/new_unified_message_center_view.h
[modify] https://crrev.com/e9d6759aef73d42deb4c0570e629c28e83925949/ash/system/message_center/new_unified_message_center_view_unittest.cc
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/879a5598297719727970bcdd5d1d269d3f5c487d

commit 879a5598297719727970bcdd5d1d269d3f5c487d
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 11 02:37:18 2018

NewMessageListView: Add slide control buttons

This CL adds swipe control support to UnifiedMessageListView.

UnifiedMessageListView will replace MessageListView. It's behind a flag:
--enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

I tried to add test for this, but it turned out to be hard, because
SlideOutController is not really testable. Given this feature did not
have test coverage in old MessageListView, I'll leave that to future
CLs.

TEST=manual
BUG= 769219 

Change-Id: Ic4a65a3e27c25aae11af931a0d901512921cd34e
Reviewed-on: https://chromium-review.googlesource.com/c/1264378
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598640}
[modify] https://crrev.com/879a5598297719727970bcdd5d1d269d3f5c487d/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/879a5598297719727970bcdd5d1d269d3f5c487d/ash/system/message_center/unified_message_list_view.h
[modify] https://crrev.com/879a5598297719727970bcdd5d1d269d3f5c487d/ash/system/message_center/unified_message_list_view_unittest.cc
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f70db209d63373f459f98a52d85111473cbacadd

commit f70db209d63373f459f98a52d85111473cbacadd
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 11 05:10:06 2018

NewMessageListView: Scroll to the bottom on open.

This CL implements custom scrolling/resizing behavior to
NewUnifiedMessageListView. In Unified, the latest notification is placed
at the bottom, so notification list should be scrolled to the bottom
when it's opened.

NewUnifiedMessageCenterView will replace UnifiedMessageCenterView.
It's behind a flag: --enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=NewUnifiedMessageCenterViewTest
BUG= 769219 

Change-Id: Ia817e7f07a2bdf161195d298771dbc641e627a23
Reviewed-on: https://chromium-review.googlesource.com/c/1270420
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598677}
[modify] https://crrev.com/f70db209d63373f459f98a52d85111473cbacadd/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/f70db209d63373f459f98a52d85111473cbacadd/ash/system/message_center/new_unified_message_center_view.h
[modify] https://crrev.com/f70db209d63373f459f98a52d85111473cbacadd/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/f70db209d63373f459f98a52d85111473cbacadd/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/f70db209d63373f459f98a52d85111473cbacadd/ash/system/message_center/unified_message_list_view.h
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 15 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6028817d5a506219d1e7ca8002a8bf44b0d79cb

commit d6028817d5a506219d1e7ca8002a8bf44b0d79cb
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Mon Oct 15 07:47:22 2018

NewMessageListView: Implement stacking counter.

This CL implements stacking notification counter at the top that shows
number of hidden notifications above the visible area.

NewUnifiedMessageCenterView will replace UnifiedMessageCenterView.
It's behind a flag: --enable-features=NewMessageListView

TEST=NewUnifiedMessageCenterViewTest
BUG= 769219 

Change-Id: I829c75ae4642c7cf0d7408defe3d59c1a68977a2
Reviewed-on: https://chromium-review.googlesource.com/c/1278690
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599583}
[modify] https://crrev.com/d6028817d5a506219d1e7ca8002a8bf44b0d79cb/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/d6028817d5a506219d1e7ca8002a8bf44b0d79cb/ash/system/message_center/new_unified_message_center_view.h
[modify] https://crrev.com/d6028817d5a506219d1e7ca8002a8bf44b0d79cb/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/d6028817d5a506219d1e7ca8002a8bf44b0d79cb/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/d6028817d5a506219d1e7ca8002a8bf44b0d79cb/ash/system/message_center/unified_message_list_view.h
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a27980800275613d3a2fc5cf2f5256aeeeb3077

commit 4a27980800275613d3a2fc5cf2f5256aeeeb3077
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Oct 16 02:26:32 2018

NewMessageListView: Paint corners behind tray.

This CL implements TopCornerBorder hack for NewMessageListView. For the
detail of the hack, please refer https://crrev.com/c/1161710 .

NewUnifiedMessageCenterView will replace UnifiedMessageCenterView.
It's behind a flag: --enable-features=NewMessageListView

TEST=NewUnifiedMessageCenterViewTest
BUG= 769219 

Change-Id: I24ad795fe118c21ef40be6b4ecc2f8e4a5f85101
Reviewed-on: https://chromium-review.googlesource.com/c/1278068
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599822}
[modify] https://crrev.com/4a27980800275613d3a2fc5cf2f5256aeeeb3077/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/4a27980800275613d3a2fc5cf2f5256aeeeb3077/ash/system/message_center/new_unified_message_center_view.h
[modify] https://crrev.com/4a27980800275613d3a2fc5cf2f5256aeeeb3077/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/4a27980800275613d3a2fc5cf2f5256aeeeb3077/ash/system/unified/unified_system_tray_view.cc
Project Member

Comment 21 by bugdroid1@chromium.org, Oct 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e587b05b305c16185a373102acd9f0f9c3a930c0

commit e587b05b305c16185a373102acd9f0f9c3a930c0
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Oct 16 04:42:20 2018

NewMessageListView: Implement removing animation.

This CL adds animation to NewMessageListView when notifications are
removed.

Video: http://shortn/_hXnIGaYesF

UnifiedMessageListView will replace MessageListView. It's behind a flag:
--enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest.RemovingNotificationAnimation
BUG= 769219 

Change-Id: I06c909acb227f9bfa484083dca36a95d79ece7f5
Reviewed-on: https://chromium-review.googlesource.com/c/1280087
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599852}
[modify] https://crrev.com/e587b05b305c16185a373102acd9f0f9c3a930c0/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/e587b05b305c16185a373102acd9f0f9c3a930c0/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/e587b05b305c16185a373102acd9f0f9c3a930c0/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/e587b05b305c16185a373102acd9f0f9c3a930c0/ash/system/message_center/unified_message_list_view.h
[modify] https://crrev.com/e587b05b305c16185a373102acd9f0f9c3a930c0/ash/system/message_center/unified_message_list_view_unittest.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 17 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f087a32f386037d923c73350708453b22b115e0d

commit f087a32f386037d923c73350708453b22b115e0d
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Oct 17 02:09:32 2018

NewMessageListView: Enable by default.

NewMessageListView is new implementation of Notification Center. Actual
class names are as follows:
* new: UnifiedMessageListView, NewUnifiedMessageCenterView
* old: MessageListView, MessageCenterView, UnifiedMessageCenterView

Besides it has better test coverage, it supports new notification
animation for UnifiedSystemTray.

Missing items that were implemented in old MessageListView:
* Animation when Clear All is pressed (not in spec yet)
* Animation when a new notification is added (not in spec yet)

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest,NewUnifiedMessageCenterViewTest
BUG= 769219 

Change-Id: Ifbb8ea3fcfe2be61f50ff7fa95ed55f789d3bbfb
Reviewed-on: https://chromium-review.googlesource.com/c/1282520
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600244}
[modify] https://crrev.com/f087a32f386037d923c73350708453b22b115e0d/ash/public/cpp/ash_features.cc

Comment 23 by tetsui@chromium.org, Oct 17

Labels: -M-71 M-72
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a

commit 2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 18 02:30:58 2018

NewMessageListView: Implement slide out animation.

This CL adds sliding out animation to NewMessageListView when
notifications are removed. Animation on removal consists of two stages:
SLIDE_OUT and MOVE_DOWN. Previously, MOVE_DOWN was implemented in
https://crrev.com/c/1280087 .

Video: http://shortn/_HiEcuNpheU

UnifiedMessageListView will replace MessageListView. It's behind a flag:
--enable-features=NewMessageListView

Design doc: go/chrome-popup-refactoring

TEST=UnifiedMessageListViewTest
BUG= 769219 

Change-Id: I8abe9eedeeb7bd1bcef09e5cb5a072a0ab88fee2
Reviewed-on: https://chromium-review.googlesource.com/c/1282628
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600639}
[modify] https://crrev.com/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a/ash/system/message_center/new_unified_message_center_view_unittest.cc
[modify] https://crrev.com/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a/ash/system/message_center/unified_message_list_view.cc
[modify] https://crrev.com/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a/ash/system/message_center/unified_message_list_view.h
[modify] https://crrev.com/2d9a7fc8c166ba4545b83d188fa18ce9958a4c2a/ash/system/message_center/unified_message_list_view_unittest.cc
Project Member

Comment 25 by bugdroid1@chromium.org, Oct 18 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/25608981a7c25f29d3660e81d2b9eb449f60f4a9

commit 25608981a7c25f29d3660e81d2b9eb449f60f4a9
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 18 05:46:30 2018

NewMessageListView: Fix scroll position.

This CL fixes the bug that the scroll position is wrong in such cases:
1. Initially there's no notification.
2. Open SystemTray
3. Press CapsLock
4. Bottom position of notification is wrong

TEST=NewUnifiedMessageCenterTest.AddAndRemoveNotification
BUG= 769219 

Change-Id: I362b2497da1b89c9decca9cca8e8e2d735d1293d
Reviewed-on: https://chromium-review.googlesource.com/c/1286011
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600667}
[modify] https://crrev.com/25608981a7c25f29d3660e81d2b9eb449f60f4a9/ash/system/message_center/new_unified_message_center_view.cc
[modify] https://crrev.com/25608981a7c25f29d3660e81d2b9eb449f60f4a9/ash/system/message_center/new_unified_message_center_view_unittest.cc
Project Member

Comment 26 by bugdroid1@chromium.org, Oct 25 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79990673f4664f728fdbbc11be51396bd4e3aa52

commit 79990673f4664f728fdbbc11be51396bd4e3aa52
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 25 01:27:02 2018

NewMessageListView: Don't delete during iteration

DeleteRemovedNotifications should not delete its children while
iterating, because this may cause not all items to be visited during the
loop.

TEST=UnifiedMessageListViewTest
BUG= 769219 

Change-Id: Ib995119e0c838d03d03c7658572cdcf8d45f7ad1
Reviewed-on: https://chromium-review.googlesource.com/c/1297222
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602558}
[modify] https://crrev.com/79990673f4664f728fdbbc11be51396bd4e3aa52/ash/system/message_center/unified_message_list_view.cc
Project Member

Comment 27 by bugdroid1@chromium.org, Oct 25 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12a4b32bff456b701739c4d41edacabe2cf742d5

commit 12a4b32bff456b701739c4d41edacabe2cf742d5
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Oct 25 01:27:16 2018

NewMessageListView: Adjust motion curve.

UX updated the motion curve for notification list.

MOVE_DOWN: FAST_IN_SLOW_OUT
SLIDE_OUT: EASE_IN

TEST=manual
BUG= 769219 

Change-Id: I899cabef2c7ae3d02c81c1c736af28d8dabf0913
Reviewed-on: https://chromium-review.googlesource.com/c/1297225
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602559}
[modify] https://crrev.com/12a4b32bff456b701739c4d41edacabe2cf742d5/ash/system/message_center/unified_message_list_view.cc

Comment 28 by tetsui@chromium.org, Oct 25

Status: Fixed (was: Started)
►

Sign in to add a comment