Fix Use-after-free in AppShortcutManager destruction

The AppShortcutManager was not correctly unregistering as a
ProfileAttributesStorage observer.
The reason is that the AppShortcutManager is destroyed as part of the
ProfileManager destruction.
At this time, g_browser_process()->profile_manager() already returns
nullptr.

AppShortcutManager was happily skipping the unregistration in that case,
because of test-related logic.

I did not repro the crash and verified the fix, but this CL should solve
the issue.

Fixed: 1145906
Change-Id: I2ad4e574ad128ec920f94978446ae33c8dc73a46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520776Reviewed-by: ccameron <ccameron@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825756}

chrome/browser/apps/platform_apps/shortcut_manager.cc

@@ -89,8 +89,7 @@ void AppShortcutManager::RegisterProfilePrefs(
   registry->RegisterStringPref(prefs::kAppShortcutsArch, "");
 }
 
-AppShortcutManager::AppShortcutManager(Profile* profile)
-    : profile_(profile), is_profile_attributes_storage_observer_(false) {
+AppShortcutManager::AppShortcutManager(Profile* profile) : profile_(profile) {
   // Use of g_browser_process requires that we are either on the UI thread, or
   // there are no threads initialized (such as in unit tests).
   DCHECK(!content::BrowserThread::IsThreadInitialized(
@@ -109,19 +108,12 @@ AppShortcutManager::AppShortcutManager(Profile* profile)
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   // profile_manager might be NULL in testing environments.
   if (profile_manager) {
-    profile_manager->GetProfileAttributesStorage().AddObserver(this);
-    is_profile_attributes_storage_observer_ = true;
+    profile_storage_observer_.Add(
+        &profile_manager->GetProfileAttributesStorage());
   }
 }
 
-AppShortcutManager::~AppShortcutManager() {
-  if (g_browser_process && is_profile_attributes_storage_observer_) {
-    ProfileManager* profile_manager = g_browser_process->profile_manager();
-    // profile_manager might be NULL in testing environments or during shutdown.
-    if (profile_manager)
-      profile_manager->GetProfileAttributesStorage().RemoveObserver(this);
-  }
-}
+AppShortcutManager::~AppShortcutManager() = default;
 
 void AppShortcutManager::OnExtensionWillBeInstalled(
     content::BrowserContext* browser_context,

chrome/browser/apps/platform_apps/shortcut_manager.h

@@ -59,8 +59,8 @@ class AppShortcutManager : public KeyedService,
   void DeleteApplicationShortcuts(const extensions::Extension* extension);
 
   Profile* profile_;
-  bool is_profile_attributes_storage_observer_;
-
+  ScopedObserver<ProfileAttributesStorage, ProfileAttributesStorage::Observer>
+      profile_storage_observer_{this};
   ScopedObserver<extensions::ExtensionRegistry,
                  extensions::ExtensionRegistryObserver>
       extension_registry_observer_{this};