Introduce ProfileManagerObserver.
Currently there are two methods, OnProfileAdded (to replace
NOTIFICATION_PROFILE_ADDED and NOTIFICATION_PROFILE_CREATED) and
OnProfileMarkedForPermanentDeletion (to replace
NOTIFICATION_PROFILE_DESTRUCTION_STARTED).
Use the new interface in NoteTakingHelper and ExtensionService.
Another option would be to have OnProfileMarkedForPermanentDeletion
be a part of a ProfileObserver interface, but it seems best to
minimize the number of observer interfaces.
Bug: 268984
Change-Id: I96ba72e558ee3da30729207adcd8e22e0755293a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776682
Commit-Queue: Evan Stade <[email protected]>
Reviewed-by: Scott Violet <[email protected]>
Cr-Commit-Position: refs/heads/master@{#693858}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index e0d99c9..0bc6da64 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1472,6 +1472,7 @@
"profiles/profile_key.h",
"profiles/profile_manager.cc",
"profiles/profile_manager.h",
+ "profiles/profile_manager_observer.h",
"profiles/profile_metrics.cc",
"profiles/profile_metrics.h",
"profiles/profile_shortcut_manager_win.cc",
diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h
index d6f87fb..fa06a890 100644
--- a/chrome/browser/chrome_notification_types.h
+++ b/chrome/browser/chrome_notification_types.h
@@ -119,29 +119,18 @@
// The details are none and the source is the new profile.
NOTIFICATION_PROFILE_CREATED,
+ // Use ProfileManagerObserver::OnProfileAdded instead of this notification.
// Sent after a Profile has been added to ProfileManager.
// The details are none and the source is the new profile.
NOTIFICATION_PROFILE_ADDED,
// Use KeyedServiceShutdownNotifier instead this notification type (you did
// read the comment at the top of the file, didn't you?).
- // Sent early in the process of destroying a Profile, at the time a user
- // initiates the deletion of a profile versus the much later time when the
- // profile object is actually destroyed (use NOTIFICATION_PROFILE_DESTROYED).
- // The details are none and the source is a Profile*.
- NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
-
- // Use KeyedServiceShutdownNotifier instead this notification type (you did
- // read the comment at the top of the file, didn't you?).
// Sent before a Profile is destroyed. This notification is sent both for
// normal and OTR profiles.
// The details are none and the source is a Profile*.
NOTIFICATION_PROFILE_DESTROYED,
- // Sent after the URLRequestContextGetter for a Profile has been initialized.
- // The details are none and the source is a Profile*.
- NOTIFICATION_PROFILE_URL_REQUEST_CONTEXT_GETTER_INITIALIZED,
-
// Printing ----------------------------------------------------------------
// Notification from PrintJob that an event occurred. It can be that a page
diff --git a/chrome/browser/chromeos/note_taking_helper.cc b/chrome/browser/chromeos/note_taking_helper.cc
index 797801c..8a9b52a6 100644
--- a/chrome/browser/chromeos/note_taking_helper.cc
+++ b/chrome/browser/chromeos/note_taking_helper.cc
@@ -18,7 +18,6 @@
#include "base/stl_util.h"
#include "base/strings/string_split.h"
#include "chrome/browser/browser_process.h"
-#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/chromeos/arc/arc_util.h"
#include "chrome/browser/chromeos/file_manager/path_util.h"
#include "chrome/browser/chromeos/lock_screen_apps/state_controller.h"
@@ -34,7 +33,6 @@
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_service.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/api/app_runtime.h"
#include "extensions/common/extension.h"
@@ -329,6 +327,24 @@
observer.OnAvailableNoteTakingAppsUpdated();
}
+void NoteTakingHelper::OnProfileAdded(Profile* profile) {
+ auto* registry = extensions::ExtensionRegistry::Get(profile);
+ DCHECK(!extension_registry_observer_.IsObserving(registry));
+ extension_registry_observer_.Add(registry);
+
+ // TODO(derat): Remove this once OnArcPlayStoreEnabledChanged() is always
+ // called after an ARC-enabled user logs in: http://b/36655474
+ if (!play_store_enabled_ && arc::IsArcPlayStoreEnabledForProfile(profile)) {
+ play_store_enabled_ = true;
+ for (Observer& observer : observers_)
+ observer.OnAvailableNoteTakingAppsUpdated();
+ }
+
+ auto* bridge = arc::ArcIntentHelperBridge::GetForBrowserContext(profile);
+ if (bridge)
+ bridge->AddObserver(this);
+}
+
void NoteTakingHelper::SetProfileWithEnabledLockScreenApps(Profile* profile) {
DCHECK(!profile_with_enabled_lock_screen_apps_);
profile_with_enabled_lock_screen_apps_ = profile;
@@ -361,8 +377,7 @@
kExtensionIds + base::size(kExtensionIds));
// Track profiles so we can observe their extension registries.
- registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
- content::NotificationService::AllBrowserContextsAndSources());
+ g_browser_process->profile_manager()->AddObserver(this);
play_store_enabled_ = false;
for (Profile* profile :
g_browser_process->profile_manager()->GetLoadedProfiles()) {
@@ -399,6 +414,8 @@
NoteTakingHelper::~NoteTakingHelper() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+ g_browser_process->profile_manager()->RemoveObserver(this);
+
// ArcSessionManagerTest shuts down ARC before NoteTakingHelper.
if (arc::ArcSessionManager::Get())
arc::ArcSessionManager::Get()->RemoveObserver(this);
@@ -538,30 +555,6 @@
NOTREACHED();
}
-void NoteTakingHelper::Observe(int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) {
- DCHECK_EQ(type, chrome::NOTIFICATION_PROFILE_ADDED);
- Profile* profile = content::Source<Profile>(source).ptr();
- DCHECK(profile);
-
- auto* registry = extensions::ExtensionRegistry::Get(profile);
- DCHECK(!extension_registry_observer_.IsObserving(registry));
- extension_registry_observer_.Add(registry);
-
- // TODO(derat): Remove this once OnArcPlayStoreEnabledChanged() is always
- // called after an ARC-enabled user logs in: http://b/36655474
- if (!play_store_enabled_ && arc::IsArcPlayStoreEnabledForProfile(profile)) {
- play_store_enabled_ = true;
- for (Observer& observer : observers_)
- observer.OnAvailableNoteTakingAppsUpdated();
- }
-
- auto* bridge = arc::ArcIntentHelperBridge::GetForBrowserContext(profile);
- if (bridge)
- bridge->AddObserver(this);
-}
-
void NoteTakingHelper::OnExtensionLoaded(
content::BrowserContext* browser_context,
const extensions::Extension* extension) {
diff --git a/chrome/browser/chromeos/note_taking_helper.h b/chrome/browser/chromeos/note_taking_helper.h
index 8af050e..dd7868b 100644
--- a/chrome/browser/chromeos/note_taking_helper.h
+++ b/chrome/browser/chromeos/note_taking_helper.h
@@ -15,13 +15,10 @@
#include "base/observer_list.h"
#include "base/scoped_observer.h"
#include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
#include "components/arc/intent_helper/arc_intent_helper_observer.h"
#include "components/arc/mojom/intent_helper.mojom.h"
#include "components/prefs/pref_change_registrar.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_source.h"
#include "extensions/browser/extension_registry_observer.h"
#include "extensions/common/extension.h"
@@ -92,8 +89,8 @@
// Singleton class used to launch a note-taking app.
class NoteTakingHelper : public arc::ArcIntentHelperObserver,
public arc::ArcSessionManager::Observer,
- public content::NotificationObserver,
- public extensions::ExtensionRegistryObserver {
+ public extensions::ExtensionRegistryObserver,
+ public ProfileManagerObserver {
public:
// Interface for observing changes to the list of available apps.
class Observer {
@@ -202,6 +199,9 @@
// arc::ArcSessionManager::Observer:
void OnArcPlayStoreEnabledChanged(bool enabled) override;
+ // ProfileManagerObserver:
+ void OnProfileAdded(Profile* profile) override;
+
// Sets the profile which supports note taking apps on the lock screen.
void SetProfileWithEnabledLockScreenApps(Profile* profile);
@@ -246,11 +246,6 @@
const std::string& app_id,
const base::FilePath& path);
- // content::NotificationObserver:
- void Observe(int type,
- const content::NotificationSource& source,
- const content::NotificationDetails& details) override;
-
// extensions::ExtensionRegistryObserver:
void OnExtensionLoaded(content::BrowserContext* browser_context,
const extensions::Extension* extension) override;
@@ -322,8 +317,6 @@
base::ObserverList<Observer>::Unchecked observers_;
- content::NotificationRegistrar registrar_;
-
std::unique_ptr<NoteTakingControllerClient> note_taking_controller_client_;
base::WeakPtrFactory<NoteTakingHelper> weak_ptr_factory_{this};
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index d2fd26b6..3ea9b0cd 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -32,6 +32,7 @@
#include "base/time/time.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_custom_extension_provider.h"
#include "chrome/browser/extensions/api/content_settings/content_settings_service.h"
@@ -322,9 +323,9 @@
content::NotificationService::AllBrowserContextsAndSources());
registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
content::NotificationService::AllBrowserContextsAndSources());
- registrar_.Add(this,
- chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
- content::Source<Profile>(profile_));
+ // The ProfileManager may be null in unit tests.
+ if (g_browser_process->profile_manager())
+ profile_manager_observer_.Add(g_browser_process->profile_manager());
UpgradeDetector::GetInstance()->AddObserver(this);
@@ -1842,10 +1843,6 @@
system_->info_map(), process->GetID()));
break;
}
- case chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED: {
- OnProfileDestructionStarted();
- break;
- }
default:
NOTREACHED() << "Unexpected notification type.";
@@ -1999,6 +1996,15 @@
return !extension || CanBlockExtension(extension);
}
+void ExtensionService::OnProfileMarkedForPermanentDeletion(Profile* profile) {
+ if (profile != profile_)
+ return;
+
+ ExtensionIdSet ids_to_unload = registry_->enabled_extensions().GetIDs();
+ for (auto it = ids_to_unload.begin(); it != ids_to_unload.end(); ++it)
+ UnloadExtension(*it, UnloadedExtensionReason::PROFILE_SHUTDOWN);
+}
+
void ExtensionService::ManageBlacklist(
const Blacklist::BlacklistStateMap& state_map) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -2166,13 +2172,6 @@
// been disabled or uninstalled.
}
-void ExtensionService::OnProfileDestructionStarted() {
- ExtensionIdSet ids_to_unload = registry_->enabled_extensions().GetIDs();
- for (auto it = ids_to_unload.begin(); it != ids_to_unload.end(); ++it) {
- UnloadExtension(*it, UnloadedExtensionReason::PROFILE_SHUTDOWN);
- }
-}
-
void ExtensionService::OnInstalledExtensionsLoaded() {
if (updater_)
updater_->Start();
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 8564a67..4153a22 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -18,12 +18,15 @@
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
+#include "base/scoped_observer.h"
#include "base/strings/string16.h"
#include "chrome/browser/extensions/blacklist.h"
#include "chrome/browser/extensions/extension_management.h"
#include "chrome/browser/extensions/forced_extensions/installation_tracker.h"
#include "chrome/browser/extensions/install_gate.h"
#include "chrome/browser/extensions/pending_extension_manager.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
#include "chrome/browser/upgrade_detector/upgrade_observer.h"
#include "components/sync/model/string_ordinal.h"
#include "content/public/browser/notification_observer.h"
@@ -159,7 +162,8 @@
public Blacklist::Observer,
public ExtensionManagement::Observer,
public UpgradeObserver,
- public ExtensionRegistrar::Delegate {
+ public ExtensionRegistrar::Delegate,
+ public ProfileManagerObserver {
public:
// Constructor stores pointers to |profile| and |extension_prefs| but
// ownership remains at caller.
@@ -448,6 +452,9 @@
bool CanDisableExtension(const Extension* extension) override;
bool ShouldBlockExtension(const Extension* extension) override;
+ // ProfileManagerObserver implementation.
+ void OnProfileMarkedForPermanentDeletion(Profile* profile) override;
+
// For the extension in |version_path| with |id|, check to see if it's an
// externally managed extension. If so, uninstall it.
void CheckExternalUninstall(const std::string& id);
@@ -666,6 +673,9 @@
// Tracker of enterprise policy forced installation.
InstallationTracker forced_extensions_tracker_;
+ ScopedObserver<ProfileManager, ProfileManagerObserver>
+ profile_manager_observer_{this};
+
using InstallGateRegistry =
std::map<ExtensionPrefs::DelayReason, InstallGate*>;
InstallGateRegistry install_delayer_registry_;
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 80e3abd..8026104 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -7262,9 +7262,7 @@
EXPECT_EQ(0u, registry()->terminated_extensions().size());
EXPECT_EQ(0u, registry()->blacklisted_extensions().size());
- service()->Observe(chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
- content::Source<Profile>(profile()),
- content::NotificationService::NoDetails());
+ service()->OnProfileMarkedForPermanentDeletion(profile());
EXPECT_EQ(UnloadedExtensionReason::PROFILE_SHUTDOWN, unloaded_reason_);
EXPECT_EQ(0u, registry()->enabled_extensions().size());
EXPECT_EQ(0u, registry()->disabled_extensions().size());
diff --git a/chrome/browser/profiles/profile_manager.cc b/chrome/browser/profiles/profile_manager.cc
index 8b9e5e2..5fb5b84 100644
--- a/chrome/browser/profiles/profile_manager.cc
+++ b/chrome/browser/profiles/profile_manager.cc
@@ -53,6 +53,7 @@
#include "chrome/browser/profiles/profile_avatar_icon_util.h"
#include "chrome/browser/profiles/profile_destroyer.h"
#include "chrome/browser/profiles/profile_info_cache.h"
+#include "chrome/browser/profiles/profile_manager_observer.h"
#include "chrome/browser/profiles/profile_metrics.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/signin/account_reconcilor_factory.h"
@@ -484,6 +485,14 @@
return profile;
}
+void ProfileManager::AddObserver(ProfileManagerObserver* observer) {
+ observers_.AddObserver(observer);
+}
+
+void ProfileManager::RemoveObserver(ProfileManagerObserver* observer) {
+ observers_.RemoveObserver(observer);
+}
+
Profile* ProfileManager::GetProfile(const base::FilePath& profile_dir) {
TRACE_EVENT0("browser", "ProfileManager::GetProfile");
@@ -1171,6 +1180,9 @@
// GetProfileByPath().
profile_info->created = true;
+ for (auto& observer : observers_)
+ observer.OnProfileAdded(profile);
+
content::NotificationService::current()->Notify(
chrome::NOTIFICATION_PROFILE_ADDED,
content::Source<Profile>(profile),
@@ -1471,12 +1483,10 @@
// TODO(sail): Due to bug 88586 we don't delete the profile instance. Once we
// start deleting the profile instance we need to close background apps too.
if (profile) {
- // TODO: Migrate additional code in this block to observe this notification
- // instead of being implemented here.
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_PROFILE_DESTRUCTION_STARTED,
- content::Source<Profile>(profile),
- content::NotificationService::NoDetails());
+ // TODO(estade): Migrate additional code in this block to observe
+ // ProfileManager instead of handling shutdown here.
+ for (auto& observer : observers_)
+ observer.OnProfileMarkedForPermanentDeletion(profile);
// Disable sync for doomed profile.
if (ProfileSyncServiceFactory::HasSyncService(profile)) {
diff --git a/chrome/browser/profiles/profile_manager.h b/chrome/browser/profiles/profile_manager.h
index bcfb7d7..87ecf1f 100644
--- a/chrome/browser/profiles/profile_manager.h
+++ b/chrome/browser/profiles/profile_manager.h
@@ -18,6 +18,7 @@
#include "base/files/file_path.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
+#include "base/observer_list.h"
#include "base/threading/thread_checker.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile.h"
@@ -30,6 +31,7 @@
class ProfileAttributesStorage;
class ProfileInfoCache;
+class ProfileManagerObserver;
class ProfileManager : public content::NotificationObserver,
public Profile::Delegate {
@@ -86,6 +88,9 @@
// platforms, this returns the default profile.
static Profile* CreateInitialProfile();
+ void AddObserver(ProfileManagerObserver* observer);
+ void RemoveObserver(ProfileManagerObserver* observer);
+
// Returns a profile for a specific profile directory within the user data
// dir. This will return an existing profile it had already been created,
// otherwise it will create and manage it.
@@ -445,6 +450,8 @@
// Controls whether to initialize some services. Only disabled for testing.
bool do_final_services_init_ = true;
+ base::ObserverList<ProfileManagerObserver> observers_;
+
// TODO(chrome/browser/profiles/OWNERS): Usage of this in profile_manager.cc
// should likely be turned into DCHECK_CURRENTLY_ON(BrowserThread::UI) for
// consistency with surrounding code in the same file but that wasn't trivial
diff --git a/chrome/browser/profiles/profile_manager_observer.h b/chrome/browser/profiles/profile_manager_observer.h
new file mode 100644
index 0000000..5f15d1d
--- /dev/null
+++ b/chrome/browser/profiles/profile_manager_observer.h
@@ -0,0 +1,24 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PROFILES_PROFILE_MANAGER_OBSERVER_H_
+#define CHROME_BROWSER_PROFILES_PROFILE_MANAGER_OBSERVER_H_
+
+#include "base/observer_list_types.h"
+
+class Profile;
+
+class ProfileManagerObserver : public base::CheckedObserver {
+ public:
+ // Called when a profile is added to the manager. The profile is fully created
+ // and registered with the ProfileManager.
+ virtual void OnProfileAdded(Profile* profile) {}
+
+ // Called when the user deletes a profile and all associated data should be
+ // erased. Note that the Profile object will not be destroyed until Chrome
+ // shuts down. See https://crbug.com/88586
+ virtual void OnProfileMarkedForPermanentDeletion(Profile* profile) {}
+};
+
+#endif // CHROME_BROWSER_PROFILES_PROFILE_MANAGER_OBSERVER_H_
