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

Issue 863366 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 3
Type: Feature



Sign in to add a comment

Rewrite MessagePopupCollection

Project Member Reported by tetsui@chromium.org, Jul 13

Issue description

Project Member

Comment 1 by bugdroid1@chromium.org, Jul 31

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

commit d540c125c10e32575ff60069ee0ca91fa9979898
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Tue Jul 31 08:48:35 2018

New implementation of MessagePopupCollection

This CL completely rewrites MessagePopupCollection which manages popup
notifications. The requirement for the class was changed over time, and
that made the class fragile. Also, there were too many internal states.

For simplicity and full test coverage, MessagePopupCollection manages
all animation states, and only single animation runs at one time.
(This is different from ToastContentsView. The class corresponds to
MessagePopupView in this CL, and it has fewer responsibilities.)

Also this CL has new Chrome OS popup animation in mind. go/qs-proto

As a further refactoring, merging of PopupAlignmentDelegate to
MessagePopupCollection is planned.

Design doc: go/chrome-popup-refactoring

BUG= 863366 
TEST=MessagePopupCollectionTest

Change-Id: I77c908423c03869d4ac3fa04133f41b73877bd3b
Reviewed-on: https://chromium-review.googlesource.com/1132720
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579349}
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ash/system/message_center/ash_popup_alignment_delegate.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ash/system/message_center/ash_popup_alignment_delegate.h
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ash/system/message_center/notification_tray.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ash/system/unified/unified_system_tray.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/chrome/browser/ui/views/message_center/popups_only_ui_delegate.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/BUILD.gn
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/message_center.h
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/desktop_popup_alignment_delegate.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/desktop_popup_alignment_delegate.h
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/message_popup_collection.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/message_popup_collection.h
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/message_popup_collection_unittest.cc
[add] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/message_popup_view.cc
[add] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/message_popup_view.h
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/popup_alignment_delegate.cc
[modify] https://crrev.com/d540c125c10e32575ff60069ee0ca91fa9979898/ui/message_center/views/popup_alignment_delegate.h
[delete] https://crrev.com/2a3e6c9dca8748d6c58323c4033e6df01aa92fc9/ui/message_center/views/toast_contents_view.cc
[delete] https://crrev.com/2a3e6c9dca8748d6c58323c4033e6df01aa92fc9/ui/message_center/views/toast_contents_view.h

Status: Fixed (was: Started)
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 8

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

commit 07e94dcfeee4e08e2a41992b9361e52206c3a867
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Wed Aug 08 02:20:09 2018

Remove bug workaround in MessageView.

We had MessagePopupCollection bug workaround in MessageView. As
MessagePopupCollection is rewritten, we don't need the workaround
anymore.

TEST=manual
BUG= 805208 , 863366 

Change-Id: I93ce95f3cf5648f9bf8f9761a78d08e93d1fa676
Reviewed-on: https://chromium-review.googlesource.com/1163415
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581444}
[modify] https://crrev.com/07e94dcfeee4e08e2a41992b9361e52206c3a867/ui/message_center/views/message_view.cc

Cc: est...@chromium.org tetsui@chromium.org yoshiki@chromium.org
 Issue 818731  has been merged into this issue.

Sign in to add a comment