Commit 16d77eb3 authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Remove NotificationService usage from ChromeAshMessageCenterClient

Registration for NOTIFICATION_PROFILE_ADDED is replaced with
User::AddProfileCreatedObserver.

Note that before this patch, the NotificationRegistrar was never
actually used because it was broken by d5f8becb.
Thus, this patch also fixes the (relatively minor) bug that while a
profile was loading, GetNotifiers would no-op, rather than perform
a delayed op.

Bug: 268984
Change-Id: I3c68b5bb9610e486088e3e0c5cb23255453dab3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857538Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705713}
parent bcaa7d7e
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "ash/public/cpp/notifier_settings_observer.h" #include "ash/public/cpp/notifier_settings_observer.h"
#include "base/i18n/string_compare.h" #include "base/i18n/string_compare.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/notifications/arc_application_notifier_controller.h" #include "chrome/browser/notifications/arc_application_notifier_controller.h"
#include "chrome/browser/notifications/extension_notifier_controller.h" #include "chrome/browser/notifications/extension_notifier_controller.h"
...@@ -16,8 +15,8 @@ ...@@ -16,8 +15,8 @@
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/settings_window_manager_chromeos.h" #include "chrome/browser/ui/settings_window_manager_chromeos.h"
#include "chrome/common/webui_url_constants.h" #include "chrome/common/webui_url_constants.h"
#include "components/user_manager/user.h"
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "content/public/browser/notification_service.h"
#include "ui/message_center/message_center.h" #include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notifier_id.h" #include "ui/message_center/public/cpp/notifier_id.h"
...@@ -32,8 +31,8 @@ ChromeAshMessageCenterClient* g_chrome_ash_message_center_client = nullptr; ...@@ -32,8 +31,8 @@ ChromeAshMessageCenterClient* g_chrome_ash_message_center_client = nullptr;
// All notifier actions are performed on the notifiers for the currently active // All notifier actions are performed on the notifiers for the currently active
// profile, so this just returns the active profile. // profile, so this just returns the active profile.
Profile* GetProfileForNotifiers() { Profile* GetProfileForNotifiers() {
const auto* user = user_manager::UserManager::Get()->GetActiveUser(); return chromeos::ProfileHelper::Get()->GetProfileByUser(
return chromeos::ProfileHelper::Get()->GetProfileByUser(user); user_manager::UserManager::Get()->GetActiveUser());
} }
class NotifierComparator { class NotifierComparator {
...@@ -145,8 +144,16 @@ void ChromeAshMessageCenterClient::Close(const std::string& notification_id) { ...@@ -145,8 +144,16 @@ void ChromeAshMessageCenterClient::Close(const std::string& notification_id) {
} }
void ChromeAshMessageCenterClient::GetNotifiers() { void ChromeAshMessageCenterClient::GetNotifiers() {
if (!notifier_observers_.might_have_observers())
return;
Profile* profile = GetProfileForNotifiers(); Profile* profile = GetProfileForNotifiers();
if (!profile) { if (!profile) {
user_manager::UserManager::Get()
->GetActiveUser()
->AddProfileCreatedObserver(
base::BindOnce(&ChromeAshMessageCenterClient::GetNotifiers,
weak_ptr_.GetWeakPtr()));
LOG(ERROR) << "GetNotifiers called before profile fully loaded, see " LOG(ERROR) << "GetNotifiers called before profile fully loaded, see "
"https://crbug.com/968825"; "https://crbug.com/968825";
return; return;
...@@ -171,9 +178,8 @@ void ChromeAshMessageCenterClient::GetNotifiers() { ...@@ -171,9 +178,8 @@ void ChromeAshMessageCenterClient::GetNotifiers() {
void ChromeAshMessageCenterClient::SetNotifierEnabled( void ChromeAshMessageCenterClient::SetNotifierEnabled(
const NotifierId& notifier_id, const NotifierId& notifier_id,
bool enabled) { bool enabled) {
Profile* profile = GetProfileForNotifiers(); sources_[notifier_id.type]->SetNotifierEnabled(GetProfileForNotifiers(),
CHECK(profile); notifier_id, enabled);
sources_[notifier_id.type]->SetNotifierEnabled(profile, notifier_id, enabled);
} }
void ChromeAshMessageCenterClient::AddNotifierSettingsObserver( void ChromeAshMessageCenterClient::AddNotifierSettingsObserver(
...@@ -199,15 +205,3 @@ void ChromeAshMessageCenterClient::OnNotifierEnabledChanged( ...@@ -199,15 +205,3 @@ void ChromeAshMessageCenterClient::OnNotifierEnabledChanged(
if (!enabled) if (!enabled)
MessageCenter::Get()->RemoveNotificationsForNotifierId(notifier_id); MessageCenter::Get()->RemoveNotificationsForNotifierId(notifier_id);
} }
void ChromeAshMessageCenterClient::Observe(
int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
DCHECK_EQ(type, chrome::NOTIFICATION_PROFILE_ADDED);
Profile* profile = GetProfileForNotifiers();
if (profile) {
GetNotifiers();
registrar_.RemoveAll();
}
}
...@@ -6,12 +6,11 @@ ...@@ -6,12 +6,11 @@
#define CHROME_BROWSER_NOTIFICATIONS_CHROME_ASH_MESSAGE_CENTER_CLIENT_H_ #define CHROME_BROWSER_NOTIFICATIONS_CHROME_ASH_MESSAGE_CENTER_CLIENT_H_
#include "ash/public/cpp/notifier_settings_controller.h" #include "ash/public/cpp/notifier_settings_controller.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "chrome/browser/notifications/notification_platform_bridge.h" #include "chrome/browser/notifications/notification_platform_bridge.h"
#include "chrome/browser/notifications/notification_platform_bridge_chromeos.h" #include "chrome/browser/notifications/notification_platform_bridge_chromeos.h"
#include "chrome/browser/notifications/notifier_controller.h" #include "chrome/browser/notifications/notifier_controller.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
// This class serves as Chrome's AshMessageCenterClient, as well as the // This class serves as Chrome's AshMessageCenterClient, as well as the
// NotificationPlatformBridge for ChromeOS. It dispatches notifications to Ash // NotificationPlatformBridge for ChromeOS. It dispatches notifications to Ash
...@@ -19,8 +18,7 @@ ...@@ -19,8 +18,7 @@
// NotifierControllers to provide notifier settings information to Ash (visible // NotifierControllers to provide notifier settings information to Ash (visible
// in NotifierSettingsView). // in NotifierSettingsView).
class ChromeAshMessageCenterClient : public ash::NotifierSettingsController, class ChromeAshMessageCenterClient : public ash::NotifierSettingsController,
public NotifierController::Observer, public NotifierController::Observer {
public content::NotificationObserver {
public: public:
explicit ChromeAshMessageCenterClient( explicit ChromeAshMessageCenterClient(
NotificationPlatformBridgeDelegate* delegate); NotificationPlatformBridgeDelegate* delegate);
...@@ -46,11 +44,6 @@ class ChromeAshMessageCenterClient : public ash::NotifierSettingsController, ...@@ -46,11 +44,6 @@ class ChromeAshMessageCenterClient : public ash::NotifierSettingsController,
bool enabled) override; bool enabled) override;
private: private:
// content::NotificationObserver override.
void Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;
NotificationPlatformBridgeDelegate* delegate_; NotificationPlatformBridgeDelegate* delegate_;
// Notifier source for each notifier type. // Notifier source for each notifier type.
...@@ -59,7 +52,7 @@ class ChromeAshMessageCenterClient : public ash::NotifierSettingsController, ...@@ -59,7 +52,7 @@ class ChromeAshMessageCenterClient : public ash::NotifierSettingsController,
base::ObserverList<ash::NotifierSettingsObserver> notifier_observers_; base::ObserverList<ash::NotifierSettingsObserver> notifier_observers_;
content::NotificationRegistrar registrar_; base::WeakPtrFactory<ChromeAshMessageCenterClient> weak_ptr_{this};
DISALLOW_COPY_AND_ASSIGN(ChromeAshMessageCenterClient); DISALLOW_COPY_AND_ASSIGN(ChromeAshMessageCenterClient);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment