New issue
Advanced search Search tips

Issue 777493 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 578868



Sign in to add a comment

remove return values from NotifierSettingsProvider

Project Member Reported by est...@chromium.org, Oct 23 2017

Issue description

similar to  bug 776187 . On ChromeOS, NotifierSettingsProvider is going to require communicating with another process (Chrome), so the synchronous stuff in there has to go away.

NotifierHasAdvancedSettings and GetNotifierList will have to be adjusted.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 25 2017

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

commit 51f736fc9b906f7c705bf18de9a3734c00429a9a
Author: Evan Stade <estade@chromium.org>
Date: Wed Oct 25 05:26:18 2017

Remove NotifierSettingsProvider::NotifierHasAdvancedSettings()

Replace with a field in NotifierUiData (which is renamed from Notifier).

Bug:  777493 
Change-Id: I8ad8eb6ac1f018c5e54ce8de994db8529fe821bd
Reviewed-on: https://chromium-review.googlesource.com/733855
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511381}
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/ash/message_center/notifier_settings_view.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/ash/message_center/notifier_settings_view.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/ash/message_center/notifier_settings_view_unittest.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/BUILD.gn
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/arc_application_notifier_controller_chromeos.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/arc_application_notifier_controller_chromeos.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/extension_notifier_controller.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/extension_notifier_controller.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/message_center_settings_controller_chromeos.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/message_center_settings_controller_chromeos.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/message_center_settings_controller_chromeos_unittest.cc
[delete] https://crrev.com/f732be588c806b44eab63e1370a7914d61cc7705/chrome/browser/notifications/notifier_controller.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/notifier_controller.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/web_page_notifier_controller.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/chrome/browser/notifications/web_page_notifier_controller.h
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/ui/message_center/notifier_settings.cc
[modify] https://crrev.com/51f736fc9b906f7c705bf18de9a3734c00429a9a/ui/message_center/notifier_settings.h

Project Member

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

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

commit 55575d8f1b14f821068a53991b93933cd8970aba
Author: Evan Stade <estade@chromium.org>
Date: Thu Nov 02 00:52:36 2017

Replace NotifierSettingsProvider with additions to AshMessageCenterClient api.

Likewise, replace NotifierSettingsObserver with additions to
AshMessageCenterController api.

Adds new mojo type for NotifierId which maps to message_center::NotifierId.
message_center::NotifierUiData isn't used anywhere else, so it's replaced with
(not mapped to) a mojo struct.

NotificationPlatformBridgeChromeOsImpl is now the hub for more things, so it's
moved to its own file and renamed to ChromeAshMessageCenterClient.

Bug:  777649 , 578868 , 777493 
Change-Id: I04db0d854f0f5aae4ad3a514cc424e35e0e88127
Reviewed-on: https://chromium-review.googlesource.com/736026
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513345}
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/message_center_button_bar.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/message_center_button_bar.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/message_center_controller.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/message_center_controller.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/message_center_view.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/notifier_settings_view.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/notifier_settings_view.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/message_center/notifier_settings_view_unittest.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/public/interfaces/ash_message_center_controller.mojom
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ash/system/web_notification/web_notification_tray_unittest.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/BUILD.gn
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/chromeos/chrome_browser_main_chromeos.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/arc_application_notifier_controller_chromeos.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/arc_application_notifier_controller_chromeos.h
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/chrome_ash_message_center_client.cc
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/chrome_ash_message_center_client.h
[rename] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/chrome_ash_message_center_client_unittest.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/extension_notifier_controller.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/extension_notifier_controller.h
[delete] https://crrev.com/78609b273f8f69a0979c6d91238a47d94f5c4dd0/chrome/browser/notifications/message_center_settings_controller_chromeos.cc
[delete] https://crrev.com/78609b273f8f69a0979c6d91238a47d94f5c4dd0/chrome/browser/notifications/message_center_settings_controller_chromeos.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/notification_platform_bridge_chromeos.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/notifier_controller.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/web_page_notifier_controller.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/browser/notifications/web_page_notifier_controller.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/chrome/test/BUILD.gn
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/fake_message_center.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/fake_message_center.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/message_center.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/message_center_impl.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/message_center_impl_unittest.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/message_center_tray.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/BUILD.gn
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/notifier_id.mojom
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/notifier_id.typemap
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/notifier_id_struct_traits.cc
[add] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/notifier_id_struct_traits.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/mojo/typemaps.gni
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/notifier_settings.cc
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/notifier_settings.h
[modify] https://crrev.com/55575d8f1b14f821068a53991b93933cd8970aba/ui/message_center/views/message_center_controller.h

Comment 3 by estade@google.com, Nov 2 2017

Status: Fixed (was: Started)

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

Status: Archived (was: Fixed)

Comment 5 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment