New issue
Advanced search Search tips

Issue 783018 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 578868
issue 734095



Sign in to add a comment

Remove direct access to NotificationUiManager and MessageCenter from chrome/

Project Member Reported by est...@chromium.org, Nov 9 2017

Issue description

instead, notifications should be using NotificationDisplayService.

As an example, see the newly updated EolNotification.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 14 2017

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

commit 29036d575e3553958abcac0f8e5aebcd98e06bdd
Author: Evan Stade <estade@chromium.org>
Date: Tue Nov 14 18:11:42 2017

Migrate three more Chrome OS system notifications to use of
NotificationDisplayService.

Bug:  783018 
Change-Id: I8fbf77513867bf091aafc0404128fe230f07eebf
Reviewed-on: https://chromium-review.googlesource.com/759680
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516358}
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/extensions/file_system_provider/file_system_provider_apitest.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/file_system_provider/notification_manager.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/file_system_provider/notification_manager.h
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/file_system_provider/service.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/file_system_provider/service.h
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/file_system_provider/service_factory.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/hats/hats_notification_controller.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/chromeos/login/quick_unlock/quick_unlock_notification_controller.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/notifications/notification_display_service_tester.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/notifications/notification_display_service_tester.h
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/notifications/stub_notification_display_service.cc
[modify] https://crrev.com/29036d575e3553958abcac0f8e5aebcd98e06bdd/chrome/browser/notifications/stub_notification_display_service.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 17 2017

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

commit 61774b2154d62b882d1dda53c72b74dbf2441398
Author: Evan Stade <estade@chromium.org>
Date: Fri Nov 17 03:18:40 2017

Update Privet notification to use NotificationDisplayService.

This one is a little more complicated because "AddOrUpdate" is not
sufficient --- in some cases an existing notification should be updated
but a new one should not be added.

Bug:  783018 
Change-Id: I7517b7125dd6e06d7f00fcfd4a678b876c96054a
Reviewed-on: https://chromium-review.googlesource.com/772352
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517272}
[modify] https://crrev.com/61774b2154d62b882d1dda53c72b74dbf2441398/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/61774b2154d62b882d1dda53c72b74dbf2441398/chrome/browser/printing/cloud_print/privet_notifications.h
[modify] https://crrev.com/61774b2154d62b882d1dda53c72b74dbf2441398/chrome/browser/printing/cloud_print/privet_notifications_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 18 2017

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

commit e9374ae205116caa8c5bcea63b01ae53724b2961
Author: Evan Stade <estade@chromium.org>
Date: Sat Nov 18 01:48:30 2017

[CrOS] Make LowDiskNotification use NotificationDisplayService instead
of direct access to MessageCenter.

Manual test: add `LowDiskSpace(10000);` to end of constructor.

Bug:  783018 
Change-Id: I3c1f7dd591e4c84b9d41742e6c042e63c7003138
Reviewed-on: https://chromium-review.googlesource.com/775059
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517673}
[modify] https://crrev.com/e9374ae205116caa8c5bcea63b01ae53724b2961/chrome/browser/chromeos/ui/low_disk_notification.cc
[modify] https://crrev.com/e9374ae205116caa8c5bcea63b01ae53724b2961/chrome/browser/chromeos/ui/low_disk_notification.h
[modify] https://crrev.com/e9374ae205116caa8c5bcea63b01ae53724b2961/chrome/browser/chromeos/ui/low_disk_notification_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 20 2017

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

commit 3224cfecf5bcdd37378f54ec1751d790c79cb00f
Author: Evan Stade <estade@chromium.org>
Date: Mon Nov 20 19:11:03 2017

Make ArcAuthNotification use NotificationDisplayService.

Also stop observing the MessageCenter, which is in another process (in
--mash).

The UMA is a little wonky, even before this change, because manually
closing the notification is not explicitly logged. It will need to be
reworked in the new style of notifications because if closing doesn't
count as "decline" then nothing does. For now this more or less
maintains existing behavior.

Bug:  783018 
Change-Id: I69278c0653d8548ccd1fe6a66015b7cf08a0d08c
Reviewed-on: https://chromium-review.googlesource.com/777739
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517883}
[modify] https://crrev.com/3224cfecf5bcdd37378f54ec1751d790c79cb00f/chrome/browser/chromeos/arc/arc_auth_notification.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 20 2017

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

commit 3487c988af4fdd3267d4eae52f75733f8f6d51be
Author: Evan Stade <estade@chromium.org>
Date: Mon Nov 20 21:51:30 2017

Revert "Update Privet notification to use NotificationDisplayService."

This reverts commit 61774b2154d62b882d1dda53c72b74dbf2441398.

Interacting with TRANSIENT notifications doesn't yet work on !cros
platforms.

TBR=thestig@chromium.org

Bug:  783018 
Change-Id: Ifdbf166204c31613ff7fc67b70fd26aa3f495407
Reviewed-on: https://chromium-review.googlesource.com/779966
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517938}
[modify] https://crrev.com/3487c988af4fdd3267d4eae52f75733f8f6d51be/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/3487c988af4fdd3267d4eae52f75733f8f6d51be/chrome/browser/printing/cloud_print/privet_notifications.h
[modify] https://crrev.com/3487c988af4fdd3267d4eae52f75733f8f6d51be/chrome/browser/printing/cloud_print/privet_notifications_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21 2017

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

commit 8a8162d891c6feb32d353cfcab58bb31cf6b9d41
Author: Evan Stade <estade@chromium.org>
Date: Tue Nov 21 04:32:42 2017

Move another arc notification to use of NotificationDisplayService.

Bug:  783018 
Change-Id: Ica94debdae5f9fc3ef39203d5272cb7b0d0eb1a7
Reviewed-on: https://chromium-review.googlesource.com/777997
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518111}
[modify] https://crrev.com/8a8162d891c6feb32d353cfcab58bb31cf6b9d41/chrome/browser/chromeos/arc/arc_migration_guide_notification.cc
[modify] https://crrev.com/8a8162d891c6feb32d353cfcab58bb31cf6b9d41/chrome/browser/notifications/message_center_display_service.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 21 2017

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

commit da4d191461456ec32709aba5722ea66cd7111a14
Author: Evan Stade <estade@chromium.org>
Date: Tue Nov 21 23:00:09 2017

Notifications: Keep using MessageCenterDisplayService instead of the
NotificationPlatformBridge for TRANSIENT notifications until such time
that platform bridges can be updated to handle TRANSIENT notifications
via their delegates.

This should not have any immediate effect as would-be TRANSIENT
notifications do not use NotificationDisplayService yet, but it allows
them to be updated to use NDS while the platform bridges are still a
work in progress.

Bug:  783018 
Change-Id: I224da7b03172152fbd9a5ee2cd4d5017a4de5f74
Reviewed-on: https://chromium-review.googlesource.com/779067
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518427}
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/native_notification_display_service.cc
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/native_notification_display_service.h
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge.h
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge_android.cc
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge_chromeos.cc
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge_linux.cc
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge_mac.mm
[modify] https://crrev.com/da4d191461456ec32709aba5722ea66cd7111a14/chrome/browser/notifications/notification_platform_bridge_win.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 29 2017

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

commit 9a1b9a938ce00d260e314ebf6bdb89982dfe2a82
Author: Evan Stade <estade@chromium.org>
Date: Wed Nov 29 23:21:32 2017

Make DesktopNotificationBalloon use the NotificationDisplayService.

It's safe to remove the 6 second timeout because there's already an 8
second default timeout.

Bug: 771839, 783018 
Change-Id: Idf122dfa2367844be7020d13a8c4d0d750561e0c
Reviewed-on: https://chromium-review.googlesource.com/772783
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520300}
[modify] https://crrev.com/9a1b9a938ce00d260e314ebf6bdb89982dfe2a82/chrome/browser/status_icons/desktop_notification_balloon.cc
[modify] https://crrev.com/9a1b9a938ce00d260e314ebf6bdb89982dfe2a82/chrome/browser/status_icons/desktop_notification_balloon.h

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 2 2017

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

commit cf003ede2455374930c094b425ba81d5bfe0763e
Author: Evan Stade <estade@chromium.org>
Date: Sat Dec 02 02:24:55 2017

Migrate some more uses of g_browser_process->notification_ui_manager()
to NotificationDisplayService.

Also combine the two generic click handling NotificationDelegates.

Bug:  783018 ,  755413 
Change-Id: I180350bfa0507851f658d619f2918c04a81c5d7a
Reviewed-on: https://chromium-review.googlesource.com/765047
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521182}
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/ash/system/screen_layout_observer.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/chromeos/net/network_state_notifier.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/chromeos/status/data_promo_notification.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/chromeos/ui/low_disk_notification.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/extensions/extension_storage_monitor.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/extensions/extension_storage_monitor.h
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/notifications/native_notification_display_service.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/signin/signin_error_notifier_ash.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/sync/sync_error_notifier_ash.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/ui/ash/chrome_screenshot_grabber.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/chrome/browser/ui/extensions/extension_installed_notification.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/ui/message_center/notification.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/ui/message_center/notification_delegate.cc
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/ui/message_center/notification_delegate.h
[modify] https://crrev.com/cf003ede2455374930c094b425ba81d5bfe0763e/ui/message_center/notification_delegate_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5 2017

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

commit 4d7a46317fe16e6738d8d93a64359f418409c0c3
Author: Evan Stade <estade@chromium.org>
Date: Tue Dec 05 17:30:08 2017

Update Privet notification to use NotificationDisplayService.

This re-lands 61774b2154d62b882d1dda53c. Now that
NativeNotificationDisplay passes off to the MessageCenter for platform
bridges that don't yet handle TRANSIENT, there will be no net change
for those platforms.

Bug:  783018 
Change-Id: Iec09815faf6e71cc3622edddad299c357af4ee67
Reviewed-on: https://chromium-review.googlesource.com/807400
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521733}
[modify] https://crrev.com/4d7a46317fe16e6738d8d93a64359f418409c0c3/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/4d7a46317fe16e6738d8d93a64359f418409c0c3/chrome/browser/printing/cloud_print/privet_notifications.h
[modify] https://crrev.com/4d7a46317fe16e6738d8d93a64359f418409c0c3/chrome/browser/printing/cloud_print/privet_notifications_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 6 2017

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 6 2017

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

commit f3e0fc090050e49e6371e37fd25d2cb1ad20b329
Author: Evan Stade <estade@chromium.org>
Date: Wed Dec 06 23:33:23 2017

Revert "Chrome OS - Update captive portal notification to use"

This reverts commit 6e56b9520e8e42f92518314ffa9e3f4325416693.

Reason for revert: I realized this crashes if a captive portal is
detected during login (before there's an active profile).

Original change's description:
> Chrome OS - Update captive portal notification to use
> NotificationDisplayService.
> 
> Test by commenting out most of OnPortalDetectionCompleted() (in addition
> to unit tests).
> 
> Bug:  783018 
> Change-Id: I403967f28883c588d9c2e5fd43effe05e41f7eda
> Reviewed-on: https://chromium-review.googlesource.com/807445
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522214}

TBR=stevenjb@chromium.org,sky@chromium.org,estade@chromium.org

Change-Id: I951a56b8bb5a9c44e6aeed157cb6cbda88e2be93
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783018 
Reviewed-on: https://chromium-review.googlesource.com/812470
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522251}
[modify] https://crrev.com/f3e0fc090050e49e6371e37fd25d2cb1ad20b329/chrome/browser/chromeos/net/network_portal_detector_impl_browsertest.cc
[modify] https://crrev.com/f3e0fc090050e49e6371e37fd25d2cb1ad20b329/chrome/browser/chromeos/net/network_portal_notification_controller.cc
[modify] https://crrev.com/f3e0fc090050e49e6371e37fd25d2cb1ad20b329/chrome/browser/chromeos/net/network_portal_notification_controller_unittest.cc
[modify] https://crrev.com/f3e0fc090050e49e6371e37fd25d2cb1ad20b329/chrome/browser/extensions/api/networking_config_chromeos_apitest_chromeos.cc
[modify] https://crrev.com/f3e0fc090050e49e6371e37fd25d2cb1ad20b329/chrome/browser/signin/signin_error_notifier_factory_ash.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 8 2017

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

commit 5d6e4694731cf80a222354f10a99fc467536da99
Author: Evan Stade <estade@chromium.org>
Date: Fri Dec 08 17:43:52 2017

Chrome OS - Update captive portal notification to use
NotificationDisplayService.

Test by commenting out most of OnPortalDetectionCompleted() (in addition
to unit tests). Works with and without --login-manager.

The difference from 6e56b9520e8e42f92518314ff is that now the profile
used for the NotificationDisplayService is the sign in profile, i.e.
a profile that's safe to use on the login screen, rather than the active
user profile, which may not exist yet. This will be reused by other
system notifications that live in chrome/ as well, although in the
distant future all such notifications should probably move to ash/,
where a profile/user isn't needed to add a notification to the message
center.

TBR=sky@chromium.org

Bug:  783018 
Change-Id: I077d08c641e436d3581d8dfbc2d506eef9c6135d
Reviewed-on: https://chromium-review.googlesource.com/812435
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522813}
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/chromeos/net/network_portal_detector_impl_browsertest.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/chromeos/net/network_portal_notification_controller.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/chromeos/net/network_portal_notification_controller_unittest.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/extensions/api/networking_config_chromeos_apitest_chromeos.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/notifications/notification_display_service.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/notifications/notification_display_service.h
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/notifications/notification_display_service_tester.h
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/notifications/notification_platform_bridge_chromeos.cc
[modify] https://crrev.com/5d6e4694731cf80a222354f10a99fc467536da99/chrome/browser/signin/signin_error_notifier_factory_ash.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 8 2017

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

commit 37bd8dbf991d69ecf3bc38047cb8772d3fd715d2
Author: Evan Stade <estade@chromium.org>
Date: Fri Dec 08 20:59:23 2017

Use NotificationDisplayService for ExtensionStorageMonitor notification.

Bug:  783018 
Change-Id: If8fe05a25344ce61f5388e13b2d65a58aa24e5ae
Reviewed-on: https://chromium-review.googlesource.com/816051
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522872}
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/extensions/extension_storage_monitor.cc
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/extensions/extension_storage_monitor.h
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/extensions/extension_storage_monitor_browsertest.cc
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/extensions/extension_storage_monitor_factory.cc
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/notifications/notification_display_service_tester.cc
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/notifications/notification_display_service_tester.h
[modify] https://crrev.com/37bd8dbf991d69ecf3bc38047cb8772d3fd715d2/chrome/browser/notifications/stub_notification_display_service.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 11 2017

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

commit f517916d62ef6493a59b6a97cd43cde8b2b0f6be
Author: Maxim Kolosovskiy <kolos@chromium.org>
Date: Mon Dec 11 09:28:16 2017

Revert "Use NotificationDisplayService for ExtensionStorageMonitor notification."

This reverts commit 37bd8dbf991d69ecf3bc38047cb8772d3fd715d2.

Reason for revert: browser test failures
ExtensionStorageMonitorTest.ThrottleNotifications
ExtensionStorageMonitorTest.DisableForInstalledExtensions
ExtensionStorageMonitorTest.DoubleInitialThreshold
ExtensionStorageMonitorTest.UnderThreshold
ExtensionStorageMonitorTest.TwoHostedAppsInSameOrigin
ExtensionStorageMonitorTest.HostedAppTemporaryFilesystem
ExtensionStorageMonitorTest.UserDisabledNotifications
ExtensionStorageMonitorTest.ExceedInitialThreshold

first failed build https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20CFI/builds/4334

Original change's description:
> Use NotificationDisplayService for ExtensionStorageMonitor notification.
> 
> Bug:  783018 
> Change-Id: If8fe05a25344ce61f5388e13b2d65a58aa24e5ae
> Reviewed-on: https://chromium-review.googlesource.com/816051
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Nick Carter <nick@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#522872}

TBR=stevenjb@chromium.org,nick@chromium.org,rdevlin.cronin@chromium.org,estade@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  783018 
Change-Id: Ia32a0edadd1858c024a67ef80f247edfc1a4d04e
Reviewed-on: https://chromium-review.googlesource.com/817297
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523063}
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/extensions/extension_storage_monitor.cc
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/extensions/extension_storage_monitor.h
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/extensions/extension_storage_monitor_browsertest.cc
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/extensions/extension_storage_monitor_factory.cc
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/notifications/notification_display_service_tester.cc
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/notifications/notification_display_service_tester.h
[modify] https://crrev.com/f517916d62ef6493a59b6a97cd43cde8b2b0f6be/chrome/browser/notifications/stub_notification_display_service.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Dec 12 2017

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

commit 71d3dd9929ad6b19938009ffa9d8747384b74b1c
Author: Evan Stade <estade@chromium.org>
Date: Tue Dec 12 18:42:53 2017

Remove MessageCenter's NotificationDelegate::Display().

It's used very rarely and is redundant with the MessageCenterObserver
method. To continue to support it would require extra work on many
platforms (for native notifications) and some platforms might not be
able to support it anyway.

TBR=tapted@chromium.org

Bug:  783018 
Change-Id: Iab3def37e0ee7c7dc56178ccbcc0146d712805ea
Reviewed-on: https://chromium-review.googlesource.com/811366
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523488}
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/chrome/browser/notifications/message_center_notification_manager.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/chrome/browser/notifications/message_center_notifications_browsertest.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/chrome/browser/notifications/notification_test_util.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac.h
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/ui/message_center/notification.h
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/ui/message_center/notification_delegate.cc
[modify] https://crrev.com/71d3dd9929ad6b19938009ffa9d8747384b74b1c/ui/message_center/notification_delegate.h

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 18 2017

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

commit c59c671a6e9a21c50c210552dec25474c90bf78e
Author: Evan Stade <estade@chromium.org>
Date: Mon Dec 18 19:39:34 2017

Update ArcBootErrorNotification for NotificationDisplayService.

Bug:  783018 
Change-Id: Icb9c062ecbefbe90538f6e87e35cde20a8eadfc3
Reviewed-on: https://chromium-review.googlesource.com/823525
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524765}
[modify] https://crrev.com/c59c671a6e9a21c50c210552dec25474c90bf78e/chrome/browser/chromeos/arc/notification/arc_boot_error_notification.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Dec 19 2017

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

commit 188faf2ac78aba56b4e8cc65a16b16f1c4118392
Author: Evan Stade <estade@chromium.org>
Date: Tue Dec 19 00:12:50 2017

Make TetherNotificationPresenter use NotificationDisplayService.

Also make it use NotificationDelegate instead of MessageCenterObserver
(notifiers should keep track of interaction with their notifications
via the former, not the latter).

Also make the unit tests a good deal more streamlined.

Bug:  783018 , 784494 
Change-Id: I9e0d22ced745f650796ff3abc81c9288dce7feec
Reviewed-on: https://chromium-review.googlesource.com/828468
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524874}
[modify] https://crrev.com/188faf2ac78aba56b4e8cc65a16b16f1c4118392/chrome/browser/chromeos/net/tether_notification_presenter.cc
[modify] https://crrev.com/188faf2ac78aba56b4e8cc65a16b16f1c4118392/chrome/browser/chromeos/net/tether_notification_presenter.h
[modify] https://crrev.com/188faf2ac78aba56b4e8cc65a16b16f1c4118392/chrome/browser/chromeos/net/tether_notification_presenter_unittest.cc
[modify] https://crrev.com/188faf2ac78aba56b4e8cc65a16b16f1c4118392/chrome/browser/chromeos/tether/tether_service.cc
[modify] https://crrev.com/188faf2ac78aba56b4e8cc65a16b16f1c4118392/chrome/browser/notifications/stub_notification_display_service.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Dec 19 2017

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

commit 627f0f0e3ca12a1e37e8724fc3ca3df33443267c
Author: Evan Stade <estade@chromium.org>
Date: Tue Dec 19 14:17:04 2017

Update ArcProvisionNotification for NotificationDisplayService.

also,
- simplify production code by removing unit test delegate
- streamline unit test with use of BrowserWithTestWindowTest

Bug:  783018 
Change-Id: I997312523f48bdd7a0cdeb20c563412afac526d6
Reviewed-on: https://chromium-review.googlesource.com/823134
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525016}
[modify] https://crrev.com/627f0f0e3ca12a1e37e8724fc3ca3df33443267c/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.cc
[modify] https://crrev.com/627f0f0e3ca12a1e37e8724fc3ca3df33443267c/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h
[modify] https://crrev.com/627f0f0e3ca12a1e37e8724fc3ca3df33443267c/chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Dec 19 2017

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

commit dd75c2543912123c4353e0d125351797d7d53c53
Author: Christian Dullweber <dullweber@chromium.org>
Date: Tue Dec 19 15:53:42 2017

Revert "Update ArcProvisionNotification for NotificationDisplayService."

This reverts commit 627f0f0e3ca12a1e37e8724fc3ca3df33443267c.

Reason for revert: The test is failing on linux-chromeos-rel  https://crbug.com/796198 

Original change's description:
> Update ArcProvisionNotification for NotificationDisplayService.
> 
> also,
> - simplify production code by removing unit test delegate
> - streamline unit test with use of BrowserWithTestWindowTest
> 
> Bug:  783018 
> Change-Id: I997312523f48bdd7a0cdeb20c563412afac526d6
> Reviewed-on: https://chromium-review.googlesource.com/823134
> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525016}

TBR=yoshiki@chromium.org,estade@chromium.org,hidehiko@chromium.org

Change-Id: Ie3964e9c4c810c24e13751ec5733f27d2e5bd211
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783018 
Reviewed-on: https://chromium-review.googlesource.com/832689
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525042}
[modify] https://crrev.com/dd75c2543912123c4353e0d125351797d7d53c53/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.cc
[modify] https://crrev.com/dd75c2543912123c4353e0d125351797d7d53c53/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h
[modify] https://crrev.com/dd75c2543912123c4353e0d125351797d7d53c53/chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Dec 20 2017

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

commit 6af3467a4253e35299ae990fbe707dae60bef616
Author: Evan Stade <estade@chromium.org>
Date: Wed Dec 20 20:08:07 2017

Reland "Update ArcProvisionNotification for NotificationDisplayService."

Originally landed as 627f0f0e3ca12a1e3. Fix is in
StubNotificationService, where it should not be assumed that the
NotificationDelegate is non-null.

ArcProvision* files are unchanged from original CL. It made it past the
CQ before because the crash only reproduces on debug runs of unit_tests.

Bug:  783018 
Change-Id: Iaf6a4b59a4c0daefe95901934dd90ab5694f70f9
Reviewed-on: https://chromium-review.googlesource.com/833971
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525422}
[modify] https://crrev.com/6af3467a4253e35299ae990fbe707dae60bef616/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.cc
[modify] https://crrev.com/6af3467a4253e35299ae990fbe707dae60bef616/chrome/browser/chromeos/arc/notification/arc_provision_notification_service.h
[modify] https://crrev.com/6af3467a4253e35299ae990fbe707dae60bef616/chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc
[modify] https://crrev.com/6af3467a4253e35299ae990fbe707dae60bef616/chrome/browser/notifications/stub_notification_display_service.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Dec 20 2017

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

commit 5bef379adf5183d915afb2044d04427e7b297a28
Author: Evan Stade <estade@chromium.org>
Date: Wed Dec 20 20:14:09 2017

Update NetworkStateNotifier to use NotificationDisplayService.

Bug:  783018 
Change-Id: I03716fbc14a490ba9a3478007936a599df5780bd
Reviewed-on: https://chromium-review.googlesource.com/831231
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525424}
[modify] https://crrev.com/5bef379adf5183d915afb2044d04427e7b297a28/chrome/browser/chromeos/net/network_state_notifier.cc
[modify] https://crrev.com/5bef379adf5183d915afb2044d04427e7b297a28/chrome/browser/chromeos/net/network_state_notifier_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Jan 10 2018

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

commit 5f13ab4ffc26b6beccbe3015f6582430446e746b
Author: Evan Stade <estade@chromium.org>
Date: Wed Jan 10 23:17:27 2018

Move notification blocking functionality for arc kiosk mode

Add the functionality to SessionStateNotificationBlocker, which is
renamed from LoginStateNotificationBlocker, and extend to other
app (non-arc) kiosk mode.

Test with --force-android-app-mode

Bug:  783018 
Change-Id: Icface86d52fb2090acc596c20ac146ffec214e85
Reviewed-on: https://chromium-review.googlesource.com/838702
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528468}
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/BUILD.gn
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/message_center/message_center_controller.h
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/session/test_session_controller_client.cc
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/session/test_session_controller_client.h
[rename] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/system/web_notification/session_state_notification_blocker.cc
[rename] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/system/web_notification/session_state_notification_blocker.h
[rename] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/system/web_notification/session_state_notification_blocker_unittest.cc
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/test/ash_test_base.cc
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/ash/test/ash_test_base.h
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/chrome/browser/chromeos/app_mode/arc/arc_kiosk_app_service.cc
[modify] https://crrev.com/5f13ab4ffc26b6beccbe3015f6582430446e746b/chrome/browser/chromeos/app_mode/arc/arc_kiosk_app_service.h

Project Member

Comment 26 by bugdroid1@chromium.org, Jan 16 2018

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

commit c4f7b0c874f9220b873c568fcd8ded6a637e9814
Author: Evan Stade <estade@chromium.org>
Date: Tue Jan 16 19:04:20 2018

Update RequestFileSystemNotification to use NotificationDisplayService.

Also, there's no reason for RequestFileSystemNotification to be a
NotificationDelegate, so remove that.

Bug:  783018 
Change-Id: I8961a3457295bf6e3f1ff7b91ab9d82751f2e58d
Reviewed-on: https://chromium-review.googlesource.com/840420
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529480}
[modify] https://crrev.com/c4f7b0c874f9220b873c568fcd8ded6a637e9814/chrome/browser/extensions/api/file_system/consent_provider.cc
[modify] https://crrev.com/c4f7b0c874f9220b873c568fcd8ded6a637e9814/chrome/browser/extensions/api/file_system/request_file_system_notification.cc
[modify] https://crrev.com/c4f7b0c874f9220b873c568fcd8ded6a637e9814/chrome/browser/extensions/api/file_system/request_file_system_notification.h

Project Member

Comment 28 by bugdroid1@chromium.org, Jan 17 2018

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

commit 470d3a6be2f380affa8de3dc0dd1508f3eeba9f7
Author: Evan Stade <estade@chromium.org>
Date: Wed Jan 17 02:42:29 2018

Update data promo notifications to use NotificationDisplayService.

Also change the code to be more consistent about where it's setting
prefs: whereas before some prefs went to the primary user and some to
the active user (with another set on the local state, i.e. device), now
all the promos and prefs hang off the active user.

This also fixes a bug: when you click the notification it shows for
the active user. In the past it always showed for the primary user,
which meant that we were silently showing a browser window on a
hidden desktop when active != primary. This probably didn't come up
very often, but could happen if you signed into user 1, signed into
a user 2, then connected to a cell network.

Test: unit tests, --enable-datasaver-prompt=demo
Bug:  783018 
Change-Id: Ie69aa88d00ae650d70ec7fe4d0a6911bb5a48974
Reviewed-on: https://chromium-review.googlesource.com/828147
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529554}
[modify] https://crrev.com/470d3a6be2f380affa8de3dc0dd1508f3eeba9f7/chrome/browser/ui/ash/network/data_promo_notification.cc
[modify] https://crrev.com/470d3a6be2f380affa8de3dc0dd1508f3eeba9f7/chrome/browser/ui/ash/network/data_promo_notification_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Jan 18 2018

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

commit 0db12204060a031b40916c5924e17a9b8d6bf7cb
Author: Evan Stade <estade@chromium.org>
Date: Thu Jan 18 17:48:03 2018

Update BackgroundContentsService to use NotificationDisplayService.

This also hit a check in NotificationSystemObserver due to changing
the construction time of its owning class, NotificationUiManager*. Since
the latter is constructed lazily, it's hard to predict when it will
happen and we should be fine just turning the check into a no-op.

*This class is deprecated and we're actively working to remove it.

Bug:  783018 
Change-Id: I6c8fda0418b3ccbb6e34e88664e8e4ef14e04802
Reviewed-on: https://chromium-review.googlesource.com/786251
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Drew Wilson <atwilson@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530192}
[modify] https://crrev.com/0db12204060a031b40916c5924e17a9b8d6bf7cb/chrome/browser/background/background_contents_service.cc
[modify] https://crrev.com/0db12204060a031b40916c5924e17a9b8d6bf7cb/chrome/browser/notifications/notification_system_observer.cc
[modify] https://crrev.com/0db12204060a031b40916c5924e17a9b8d6bf7cb/chrome/browser/notifications/notification_system_observer.h

Project Member

Comment 31 by bugdroid1@chromium.org, Jan 18 2018

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

commit bd6d837378a0e877de90fb4c5b8131ddab21250f
Author: Evan Stade <estade@chromium.org>
Date: Thu Jan 18 18:10:52 2018

Update DriveFirstRunController to use NotificationDisplayService.

Bug:  783018 
Change-Id: I0441e8474f519b42251e0ae72ef3e19cd1ffdb95
Reviewed-on: https://chromium-review.googlesource.com/867993
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530202}
[modify] https://crrev.com/bd6d837378a0e877de90fb4c5b8131ddab21250f/chrome/browser/chromeos/first_run/drive_first_run_controller.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Jan 19 2018

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

commit b2ca5e69971b1eeef8a480767acc59d334824966
Author: Evan Stade <estade@chromium.org>
Date: Fri Jan 19 03:08:40 2018

Update download notification browser tests.

Use NotificationDisplayService instead of MessageCenter in tests.

Bug:  783018 
Change-Id: I2bc849e90f5f899144d015c83755f8eade62e185
Reviewed-on: https://chromium-review.googlesource.com/867291
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530411}
[modify] https://crrev.com/b2ca5e69971b1eeef8a480767acc59d334824966/chrome/browser/download/notification/download_notification_browsertest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jan 19 2018

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

commit dce1f36a262507a70ba9130d9049759ed6877ee6
Author: Evan Stade <estade@chromium.org>
Date: Fri Jan 19 18:13:36 2018

Revert "Update download notification browser tests."

This reverts commit b2ca5e69971b1eeef8a480767acc59d334824966.

Reason for revert: findit identified it as a cause of test flakiness

Original change's description:
> Update download notification browser tests.
> 
> Use NotificationDisplayService instead of MessageCenter in tests.
> 
> Bug:  783018 
> Change-Id: I2bc849e90f5f899144d015c83755f8eade62e185
> Reviewed-on: https://chromium-review.googlesource.com/867291
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#530411}

TBR=dtrainor@chromium.org,estade@chromium.org

Change-Id: Id0df60959dde79702423141bf57fbfd795b65053
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783018 
Reviewed-on: https://chromium-review.googlesource.com/876562
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530563}
[modify] https://crrev.com/dce1f36a262507a70ba9130d9049759ed6877ee6/chrome/browser/download/notification/download_notification_browsertest.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Jan 19 2018

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

commit dce1f36a262507a70ba9130d9049759ed6877ee6
Author: Evan Stade <estade@chromium.org>
Date: Fri Jan 19 18:13:36 2018

Revert "Update download notification browser tests."

This reverts commit b2ca5e69971b1eeef8a480767acc59d334824966.

Reason for revert: findit identified it as a cause of test flakiness

Original change's description:
> Update download notification browser tests.
> 
> Use NotificationDisplayService instead of MessageCenter in tests.
> 
> Bug:  783018 
> Change-Id: I2bc849e90f5f899144d015c83755f8eade62e185
> Reviewed-on: https://chromium-review.googlesource.com/867291
> Commit-Queue: Evan Stade <estade@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#530411}

TBR=dtrainor@chromium.org,estade@chromium.org

Change-Id: Id0df60959dde79702423141bf57fbfd795b65053
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783018 
Reviewed-on: https://chromium-review.googlesource.com/876562
Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530563}
[modify] https://crrev.com/dce1f36a262507a70ba9130d9049759ed6877ee6/chrome/browser/download/notification/download_notification_browsertest.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Jan 22 2018

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

commit acd216cde6d44cf5f361d387486063429d27badd
Author: Evan Stade <estade@chromium.org>
Date: Mon Jan 22 16:29:36 2018

Remove obsolete NotificationUiManager calls in download_browsertest.

Bug:  783018 
Change-Id: Iac96501f179dc9c2eb251393acbada13160f6817
Reviewed-on: https://chromium-review.googlesource.com/872173
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530877}
[modify] https://crrev.com/acd216cde6d44cf5f361d387486063429d27badd/chrome/browser/download/download_browsertest.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jan 22 2018

Project Member

Comment 38 by bugdroid1@chromium.org, Jan 23 2018

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

commit f7542fab4c1dca672e169af01ed65f3ca131224b
Author: Takashi Sakamoto <tasak@google.com>
Date: Tue Jan 23 04:32:23 2018

Revert "Update more tests to use NotificationDisplayServiceTester."

This reverts commit 2a3c7c8469960af9a860096f6e88a433ee67f34f.

Reason for revert: 
This causes "ExtensionCrashRecoveryTest.Basic and 13 other(s) in interactive_ui_tests failing on chromium.memory/Linux CFI".

FYI:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_CFI%2F5380%2F%2B%2Frecipes%2Fsteps%2Finteractive_ui_tests%2F0%2Flogs%2FExtensionCrashRecoveryTest.Basic%2F0

../../components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26: runtime error: control flow integrity check for type 'content::BrowserContext' failed during base-to-derived cast (vtable address 0xffffd7cae990ace0)
0xffffd7cae990ace0: note: invalid vtable
<memory cannot be printed>
    #0 0xaee1bdc in BrowserContextKeyedServiceFactory::ContextShutdown(base::SupportsUserData*) components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26
    #1 0xa492d83 in KeyedServiceFactory::SetTestingFactory(base::SupportsUserData*, std::__1::function<std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (base::SupportsUserData*)>) components/keyed_service/core/keyed_service_factory.cc:42:3
    #2 0xaee14fc in BrowserContextKeyedServiceFactory::SetTestingFactory(content::BrowserContext*, std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (*)(content::BrowserContext*)) components/keyed_service/content/browser_context_keyed_service_factory.cc:24:24
    #3 0x50a5898 in operator() buildtools/third_party/libc++/trunk/include/memory:2233:5
    #4 0x50a5898 in reset buildtools/third_party/libc++/trunk/include/memory:2546:0
    #5 0x50a5898 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2500:0
    #6 0x50a5898 in ExtensionCrashRecoveryTest::~ExtensionCrashRecoveryTest() chrome/browser/extensions/extension_crash_recovery_browsertest.cc:41:0
    #7 0x50a3f42 in ExtensionCrashRecoveryTest_Basic_Test::~ExtensionCrashRecoveryTest_Basic_Test() chrome/browser/extensions/extension_crash_recovery_browsertest.cc:146:1
    #8 0x50a3f6d in ExtensionCrashRecoveryTest_Basic_Test::~ExtensionCrashRecoveryTest_Basic_Test() chrome/browser/extensions/extension_crash_recovery_browsertest.cc:146:1
    #9 0x52a455b in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2656:3
    #10 0x52a4d71 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2769:28
    #11 0x52aa2b2 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4665:43
    #12 0x52a9f4c in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4277:10
    #13 0x8fea5ac in base::TestSuite::Run() base/test/test_suite.cc:272:16
    #14 0x5161ef1 in InteractiveUITestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/interactive_ui_tests_main.cc:117:47
    #15 0x851a902 in content::LaunchTests(content::TestLauncherDelegate*, unsigned long, int, char**) content/public/test/test_launcher.cc:632:31
    #16 0x7dda07c in LaunchChromeTests(unsigned long, content::TestLauncherDelegate*, int, char**) chrome/test/base/chrome_test_launcher.cc:177:10
    #17 0x5161e1f in main chrome/test/base/interactive_ui_tests_main.cc:141:10
    #18 0x7f4f04eddf44 in __libc_start_main /build/eglibc-SvCtMH/eglibc-2.19/csu/libc-start.c:287:0
    #19 0x4f09029 in _start ??:0:0



Original change's description:
> Update more tests to use NotificationDisplayServiceTester.
> 
> For the changes to FileManagerBrowserTestBase, you can run affected
> tests with --gtest_filter=DriveSpecific*
> 
> Bug:  783018 
> Change-Id: I30c06c731181a31197edef5e856ac3e6dd8d5554
> Reviewed-on: https://chromium-review.googlesource.com/877004
> Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531061}

TBR=stevenjb@chromium.org,rdevlin.cronin@chromium.org,estade@chromium.org

Change-Id: I44c355645374eae034f1e42f2cc4d0a33a267ebb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  783018 
Reviewed-on: https://chromium-review.googlesource.com/880241
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#531155}
[modify] https://crrev.com/f7542fab4c1dca672e169af01ed65f3ca131224b/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager_unittest.cc
[modify] https://crrev.com/f7542fab4c1dca672e169af01ed65f3ca131224b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/f7542fab4c1dca672e169af01ed65f3ca131224b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/f7542fab4c1dca672e169af01ed65f3ca131224b/chrome/browser/chromeos/hats/hats_notification_controller_unittest.cc
[modify] https://crrev.com/f7542fab4c1dca672e169af01ed65f3ca131224b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Jan 24 2018

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

commit 34884bd42f96440088b08806f90db028dd1b1324
Author: Evan Stade <estade@chromium.org>
Date: Wed Jan 24 20:31:05 2018

Change PopupsOnlyUiDelegateTest into a unit test (from browser test)

Remove dependency on NotificationUiManager but keep dependency on
MessageCenter (this test is testing code that has to run in the same
process as MessageCenter).

This should make one flaky test more consistent. One other test is
deleted because it's a duplicate of another test also named
WebNotifications (currently lives in ash/ with a todo to move to
ui/message_center).

Bug:  783018 
Change-Id: I9c6f6e5aa149fe4d65c526d7ad8c4f5d6eeb074f
Reviewed-on: https://chromium-review.googlesource.com/882883
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531666}
[modify] https://crrev.com/34884bd42f96440088b08806f90db028dd1b1324/chrome/browser/ui/views/message_center/popups_only_ui_delegate.cc
[delete] https://crrev.com/f768e4235f1caf07586f524488ebb323c4b1cc57/chrome/browser/ui/views/message_center/popups_only_ui_delegate_browsertest.cc
[add] https://crrev.com/34884bd42f96440088b08806f90db028dd1b1324/chrome/browser/ui/views/message_center/popups_only_ui_delegate_unittest.cc
[modify] https://crrev.com/34884bd42f96440088b08806f90db028dd1b1324/chrome/test/BUILD.gn
[modify] https://crrev.com/34884bd42f96440088b08806f90db028dd1b1324/ui/message_center/message_center_impl_unittest.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Jan 26 2018

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

commit bde4409086e5973bfd0de6ec2930edce78e36add
Author: Evan Stade <estade@chromium.org>
Date: Fri Jan 26 20:10:34 2018

Update more tests to use NotificationDisplayServiceTester. (try 2)

For the changes to FileManagerBrowserTestBase, you can run affected
tests with --gtest_filter=DriveSpecific*

This is identical to 2a3c7c8469960 but should work now after the fix
which landed as 7a379f73fa1b62e48659119e

Bug:  783018 
Change-Id: Ie77f44e2e7a5af15e24e630c412bebe5788cb5a0
Reviewed-on: https://chromium-review.googlesource.com/881681
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532050}
[modify] https://crrev.com/bde4409086e5973bfd0de6ec2930edce78e36add/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager_unittest.cc
[modify] https://crrev.com/bde4409086e5973bfd0de6ec2930edce78e36add/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/bde4409086e5973bfd0de6ec2930edce78e36add/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/bde4409086e5973bfd0de6ec2930edce78e36add/chrome/browser/chromeos/hats/hats_notification_controller_unittest.cc
[modify] https://crrev.com/bde4409086e5973bfd0de6ec2930edce78e36add/chrome/browser/extensions/extension_crash_recovery_browsertest.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Jan 31 2018

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

commit 5ca1233e21033c1be3a6201ebf07a19868c28be9
Author: Evan Stade <estade@chromium.org>
Date: Wed Jan 31 19:53:30 2018

Update Privet Notifications unit test to use NotificationDisplayService.

Bug:  783018 
Change-Id: I25cad9fae605346d13821fbd6afe300ce2abe9a8
Reviewed-on: https://chromium-review.googlesource.com/889438
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533365}
[modify] https://crrev.com/5ca1233e21033c1be3a6201ebf07a19868c28be9/chrome/browser/printing/cloud_print/privet_notifications_unittest.cc

Project Member

Comment 42 by bugdroid1@chromium.org, Feb 6 2018

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

commit 424c9a889abc8e93c9058734d1a2f3d17b217b50
Author: Evan Stade <estade@chromium.org>
Date: Tue Feb 06 18:13:02 2018

Remove some more dependencies on MessageCenter.

Various changes:
- Start adding DEPS exceptions in preparation for banning
ui/message_center dependencies more broadly.
- Remove a file full of constants in ui/message_center
- delete some unnecessary message_center.h includes
- remove some message center color constants in favor of direct
  use of Label::SetAutoColorReadabilityEnabled.

BUG= 783018 , 723882 
TBR=rogerta@chromium.org

Change-Id: Ia7b9332d575a77581691f65f98c945754820c212
Reviewed-on: https://chromium-review.googlesource.com/879501
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534721}
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ash/sidebar/sidebar_widget.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/background/background_mode_manager_unittest.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/chromeos/display/display_prefs_unittest.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/chromeos/system/automatic_reboot_manager_unittest.cc
[add] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/extensions/api/autotest_private/DEPS
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/extensions/api/autotest_private/autotest_private_api.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/extensions/extension_storage_monitor.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/notifications/DEPS
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/signin/easy_unlock_notification_controller_chromeos.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/ui/ash/chrome_screenshot_grabber_browsertest.cc
[add] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/chrome/browser/ui/views/message_center/DEPS
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/BUILD.gn
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/bounded_label.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/bounded_label.h
[delete] https://crrev.com/8b3db9c5ab3b8a659cb93caed0e276d7452d4ddc/ui/message_center/views/constants.h
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/notification_button.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/notification_view.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/notification_view_md.cc
[modify] https://crrev.com/424c9a889abc8e93c9058734d1a2f3d17b217b50/ui/message_center/views/notification_view_unittest.cc

Project Member

Comment 43 by bugdroid1@chromium.org, Feb 6 2018

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

commit ea9be28fa03084beb3137881a77ce6f3cd11ddc2
Author: Evan Stade <estade@chromium.org>
Date: Tue Feb 06 18:32:31 2018

Support user-agnostic notifications in NotificationDisplayService

Create an NDS for a generic/"system" profile on demand and
asynchronously to service notification requests that aren't tied to
a particular user. Apply this to WebUsbDetector, and also update
some Chrome OS specific notifications which had been doing pretty
much the same thing, but had assumed/forced synchronicity.

Bug:  783018 ,  793877 
Change-Id: I95f4434bd6557bf846f5ed89052d4cff7f6bcf21
Reviewed-on: https://chromium-review.googlesource.com/891488
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Jun Cai <juncai@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534729}
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/BUILD.gn
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/chromeos/net/network_portal_notification_controller.cc
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/chromeos/ui/low_disk_notification.cc
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/notifications/notification_platform_bridge_chromeos.cc
[add] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/notifications/system_notification_helper.cc
[add] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/notifications/system_notification_helper.h
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/ui/ash/network/network_state_notifier.cc
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/usb/web_usb_detector.cc
[modify] https://crrev.com/ea9be28fa03084beb3137881a77ce6f3cd11ddc2/chrome/browser/usb/web_usb_detector_unittest.cc

Project Member

Comment 44 by bugdroid1@chromium.org, Feb 6 2018

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

commit 88326687e1b5ecef1f6e9ac90ad371756427fc93
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Feb 06 21:16:23 2018

Jumbo build fix: Use unique names of constants in message_center

A recent CL got rid of the shared constants and instead copied
them to relevant files. To work in jumbo builds they must then
have unique names.

(The alternative to find a shared location for the constants
seems not applicable in this case)

Bug:  783018 , 723882 
Change-Id: I8e64dcf56bfe74d30279549291e89bdec80bc3d2
Reviewed-on: https://chromium-review.googlesource.com/905304
Commit-Queue: Daniel Bratell <bratell@opera.com>
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534793}
[modify] https://crrev.com/88326687e1b5ecef1f6e9ac90ad371756427fc93/ui/message_center/views/notification_view_md.cc

Project Member

Comment 45 by bugdroid1@chromium.org, Feb 7 2018

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

commit 15a143d1c2ea768aca56ca790bf02dd6167c3bf0
Author: Evan Stade <estade@chromium.org>
Date: Wed Feb 07 18:17:10 2018

Remove NotificationHandler::Type::DOWNLOAD, use TRANSIENT instead.

The DOWNLOAD type was added before TRANSIENT existed. Downloads
probably don't deserve their own type. Without this patch, the browser
tests became flaky after conversion to NotificationDisplayServiceTester.
While I couldn't repro locally or get a stack from the bots, my best
guess is that there was a race in shutdown. Making downloads use
TRANSIENT should mean there are fewer, more frequently exercised code
paths.

Bug:  783018 
Change-Id: Ic7a665a34c34aa6618a6ad9358df05d0883c6adc
Reviewed-on: https://chromium-review.googlesource.com/892419
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535065}
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/download/notification/download_item_notification.cc
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/download/notification/download_item_notification.h
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/download/notification/download_item_notification_unittest.cc
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/download/notification/download_notification_browsertest.cc
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/download/notification/download_notification_manager.cc
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/notifications/notification_display_service_impl.cc
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/notifications/notification_display_service_impl.h
[modify] https://crrev.com/15a143d1c2ea768aca56ca790bf02dd6167c3bf0/chrome/browser/notifications/notification_handler.h

Status: Fixed (was: Started)

Sign in to add a comment