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

Issue 697359 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Aug 2017
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 719625



Sign in to add a comment

Add tests for ArcCustomNotificationView

Project Member Reported by yoshiki@chromium.org, Mar 1 2017

Issue description

We need unittests for ArcCustomNotificationView
 
Status: Started (was: Assigned)
https://codereview.chromium.org/2723143002/
Project Member

Comment 2 by bugdroid1@chromium.org, May 8 2017

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

commit 0a0ea6a0c083e27e574b2a872ce6b810d2c69360
Author: yoshiki <yoshiki@chromium.org>
Date: Mon May 08 14:36:23 2017

Merge ArcNotificationItem and ArcCustomNotificationItem

We have introduced ArcNotificationItem for ARC notifications but it's no longer used because currently all ARC notifications use ArcCustomNotificationItem. This patch merges ArcCustomNotificationItem into ArcNotificationItem.

This patch also removes the unused code in ArcNotificationItem, which was used for non-custom ARC notifications.

In addition, this CL splits the class into an interface and an implementation.

BUG= 697359 
TEST=none

Review-Url: https://codereview.chromium.org/2845003002
Cr-Commit-Position: refs/heads/master@{#469982}

[modify] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/BUILD.gn
[delete] https://crrev.com/5e1fc76d3dd8959f41e6b5ebe4a0d0a38e0c2770/ui/arc/notification/arc_custom_notification_item.cc
[delete] https://crrev.com/5e1fc76d3dd8959f41e6b5ebe4a0d0a38e0c2770/ui/arc/notification/arc_custom_notification_item.h
[modify] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_custom_notification_view.h
[add] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_delegate.cc
[add] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_delegate.h
[delete] https://crrev.com/5e1fc76d3dd8959f41e6b5ebe4a0d0a38e0c2770/ui/arc/notification/arc_notification_item.cc
[modify] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_item.h
[add] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_item_impl.cc
[add] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_item_impl.h
[modify] https://crrev.com/0a0ea6a0c083e27e574b2a872ce6b810d2c69360/ui/arc/notification/arc_notification_manager.cc

Blocking: 719625
Project Member

Comment 4 by bugdroid1@chromium.org, May 25 2017

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

commit 4e9fc10cf30e41afad38069847381cd725c2acbe
Author: yoshiki <yoshiki@chromium.org>
Date: Thu May 25 17:46:07 2017

Move message_center::CustomNotificationView to arc::ArcNotificationView

We're having message_center::CustomNotificationView in ui/message_center separated from ui/arc, but it causes a lot of complexity of delegate. In other words, we have two abstraction layer: MessageView and CustomNotificationContentViewDelegate. But CustomNotificationView and CustomNotificationContentViewDelegate are only used by ARC so the later abstraction layer can be removed..

Currently CustomNotificationView is only used by ARC. So this CL moves it to ui/arc and renames it to ArcNotificationView to make the code simper in succeeding CLs.

This CL is just a refactoring so doesn't change any behavior.

BUG= 697359 
TEST=none

Review-Url: https://codereview.chromium.org/2870283002
Cr-Commit-Position: refs/heads/master@{#474693}

[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/BUILD.gn
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/DEPS
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_custom_notification_view.cc
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_custom_notification_view.h
[add] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_content_view_delegate.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_delegate.cc
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_delegate.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_item.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_item_impl.cc
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_item_impl.h
[add] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_view.cc
[add] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_view.h
[rename] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/notification/arc_notification_view_unittest.cc
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/arc/test/run_all_unittests.cc
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/message_center/BUILD.gn
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/message_center/notification_delegate.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/message_center/notification_delegate_views.cc
[delete] https://crrev.com/7f03309fde80e0cf33cb99519d2716cb5160aac6/ui/message_center/views/custom_notification_content_view_delegate.cc
[delete] https://crrev.com/7f03309fde80e0cf33cb99519d2716cb5160aac6/ui/message_center/views/custom_notification_content_view_delegate.h
[delete] https://crrev.com/7f03309fde80e0cf33cb99519d2716cb5160aac6/ui/message_center/views/custom_notification_view.cc
[delete] https://crrev.com/7f03309fde80e0cf33cb99519d2716cb5160aac6/ui/message_center/views/custom_notification_view.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/message_center/views/message_view.h
[modify] https://crrev.com/4e9fc10cf30e41afad38069847381cd725c2acbe/ui/message_center/views/message_view_factory.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 22 2017

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

commit 5817e8d93be3f4224c921c1c1d0499d4e27c2508
Author: yoshiki <yoshiki@chromium.org>
Date: Thu Jun 22 07:19:57 2017

Add unittest for ArcNotificationContentView

Major changes
- Add an unittest for ArcNotificationContentView
- Create a wrapper of NotificationSurface
- Separate the implementation of ArcNotificationSurfaceManager from the interface

BUG= 697359 
TEST=none

Review-Url: https://codereview.chromium.org/2935893004
Cr-Commit-Position: refs/heads/master@{#481473}

[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/chrome/browser/chrome_browser_main_extra_parts_exo.cc
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/chrome/browser/chrome_browser_main_extra_parts_exo.h
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/BUILD.gn
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/DEPS
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_content_view.cc
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_content_view.h
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_content_view_unittest.cc
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface.h
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_impl.cc
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_impl.h
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_manager.cc
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_manager.h
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_manager_impl.cc
[add] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_surface_manager_impl.h
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/notification/arc_notification_view.h
[modify] https://crrev.com/5817e8d93be3f4224c921c1c1d0499d4e27c2508/ui/arc/test/run_all_unittests.cc

Status: Fixed (was: Started)

Comment 8 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment