chromeos: Notify about Chrome note-taking apps.

chrome/browser/chromeos/note_taking_helper.cc

Index: chrome/browser/chromeos/note_taking_helper.cc
diff --git a/chrome/browser/chromeos/note_taking_helper.cc b/chrome/browser/chromeos/note_taking_helper.cc
index cfba53e151595ed7896d8ceafe881fe8bd6ae14d..d778d9555280cf563c88dc4d798d973222635378 100644
--- a/chrome/browser/chromeos/note_taking_helper.cc
+++ b/chrome/browser/chromeos/note_taking_helper.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/chromeos/note_taking_helper.h"
 
+#include <algorithm>
 #include <utility>
 
 #include "apps/launcher.h"
@@ -15,14 +16,18 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string_split.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/arc/arc_bridge_service.h"
 #include "components/arc/common/intent_helper.mojom.h"
 #include "components/prefs/pref_service.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"
@@ -72,37 +77,6 @@ arc::mojom::IntentInfoPtr CreateIntentInfo(const GURL& clip_data_uri) {
   return intent;
 }
 
-// Returns all installed and enabled whitelisted Chrome note-taking apps.
-std::vector<const extensions::Extension*> GetChromeApps(Profile* profile) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // TODO(derat): Query for additional Chrome apps once http://crbug.com/657139
-  // is resolved.
-  std::vector<extensions::ExtensionId> ids;
-  const std::string switch_value =
-      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-          switches::kNoteTakingAppIds);
-  if (!switch_value.empty()) {
-    ids = base::SplitString(switch_value, ",", base::TRIM_WHITESPACE,
-                            base::SPLIT_WANT_NONEMPTY);
-  }
-  ids.insert(ids.end(), kExtensionIds,
-             kExtensionIds + arraysize(kExtensionIds));
-
-  const extensions::ExtensionRegistry* extension_registry =
-      extensions::ExtensionRegistry::Get(profile);
-  const extensions::ExtensionSet& enabled_extensions =
-      extension_registry->enabled_extensions();
-
-  std::vector<const extensions::Extension*> extensions;
-  for (const auto& id : ids) {
-    if (enabled_extensions.Contains(id)) {
-      extensions.push_back(extension_registry->GetExtensionById(
-          id, extensions::ExtensionRegistry::ENABLED));
-    }
-  }
-  return extensions;
-}
-
 }  // namespace
 
 const char NoteTakingHelper::kIntentAction[] =
@@ -218,9 +192,32 @@ void NoteTakingHelper::OnArcOptInChanged(bool enabled) {
 NoteTakingHelper::NoteTakingHelper()
     : launch_chrome_app_callback_(
           base::Bind(&apps::LaunchPlatformAppWithAction)),
+      extension_registry_observer_(this),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  const std::string switch_value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kNoteTakingAppIds);
+  if (!switch_value.empty()) {
+    whitelisted_chrome_app_ids_ = base::SplitString(
+        switch_value, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+  }
+  whitelisted_chrome_app_ids_.insert(whitelisted_chrome_app_ids_.end(),
+                                     kExtensionIds,
+                                     kExtensionIds + arraysize(kExtensionIds));
+
+  // Track profiles so we can observe their extension registries.
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  for (Profile* profile :
+       g_browser_process->profile_manager()->GetLoadedProfiles()) {
+    extension_registry_observer_.Add(
+        extensions::ExtensionRegistry::Get(profile));
+  }
+
   // Check if the primary profile has already enabled ARC and watch for changes.
   auto session_manager = arc::ArcSessionManager::Get();
   session_manager->AddObserver(this);
@@ -244,6 +241,7 @@ NoteTakingHelper::NoteTakingHelper()
 
 NoteTakingHelper::~NoteTakingHelper() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
   // ArcSessionManagerTest shuts down ARC before NoteTakingHelper.
   if (arc::ArcServiceManager::Get())
     arc::ArcServiceManager::Get()->RemoveObserver(this);
@@ -251,6 +249,34 @@ NoteTakingHelper::~NoteTakingHelper() {
     arc::ArcSessionManager::Get()->RemoveObserver(this);
 }
 
+bool NoteTakingHelper::IsWhitelistedChromeApp(
+    const extensions::Extension* extension) const {
+  DCHECK(extension);
+  return std::find(whitelisted_chrome_app_ids_.begin(),
+                   whitelisted_chrome_app_ids_.end(),
+                   extension->id()) != whitelisted_chrome_app_ids_.end();
+}
+
+std::vector<const extensions::Extension*> NoteTakingHelper::GetChromeApps(
+    Profile* profile) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  const extensions::ExtensionRegistry* extension_registry =
+      extensions::ExtensionRegistry::Get(profile);
+  const extensions::ExtensionSet& enabled_extensions =
+      extension_registry->enabled_extensions();
+
+  // TODO(derat): Query for additional Chrome apps once http://crbug.com/657139
+  // is resolved.
+  std::vector<const extensions::Extension*> extensions;
+  for (const auto& id : whitelisted_chrome_app_ids_) {
+    if (enabled_extensions.Contains(id)) {
+      extensions.push_back(extension_registry->GetExtensionById(
+          id, extensions::ExtensionRegistry::ENABLED));
+    }
+  }
+  return extensions;
+}
+
 void NoteTakingHelper::UpdateAndroidApps() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   auto* helper = GetIntentHelper("RequestIntentHandlerList", 12);
@@ -327,4 +353,49 @@ bool NoteTakingHelper::LaunchAppInternal(Profile* profile,
   return true;
 }
 
+void NoteTakingHelper::Observe(int type,
+                               const content::NotificationSource& source,
+                               const content::NotificationDetails& details) {
+  switch (type) {
+    case chrome::NOTIFICATION_PROFILE_ADDED: {
+      auto registry = extensions::ExtensionRegistry::Get(
+          content::Source<Profile>(source).ptr());
+      CHECK(!extension_registry_observer_.IsObserving(registry));

[xiyuan, 2016/12/21 17:11:17]

nit: DCHECK should be enough. NOTIFICATION_PROFILE_ADDED should only be fired
once for a profile.

[Daniel Erat, 2016/12/21 18:08:51]

thanks, makes sense!

i realized that i probably have a problem here due to ExtensionRegistry objects
being shared between regular and incognito profiles:

  - i see _ADDED for regular profiles but not incognito ones (luckily, or else
this check would be hit)
  - i see _DESTROYED for both regular and incognito profiles
  - there's a _CREATED notification that's presumably sent for both

so if an incognito window is opened and then closed, i think i'll incorrectly
stop observing the regular profile's registry when i see _DESTROYED for the
incognito profile.

one workaround might be not unregistering when _DESTROYED is received for an
incognito profile, but i'm wondering if there's a cleaner way to handle this.
any thoughts?

[xiyuan, 2016/12/21 18:19:41]

ExtensionRegistryObserver has an "OnShutdown" method. Maybe we clean up there
instead of observing _DESTROYED?

[Daniel Erat, 2016/12/21 18:41:27]

thanks for pointing it out; that's much cleaner.

+      extension_registry_observer_.Add(registry);
+      break;
+    }
+    case chrome::NOTIFICATION_PROFILE_DESTROYED: {
+      // Some browser tests appear to destroy profiles before fully initializing
+      // them, resulting in NOTIFICATION_PROFILE_DESTROYED being sent without an
+      // earlier NOTIFICATION_PROFILE_ADDED.
+      auto registry = extensions::ExtensionRegistry::Get(
+          content::Source<Profile>(source).ptr());
+      if (extension_registry_observer_.IsObserving(registry))
+        extension_registry_observer_.Remove(registry);
+      break;
+    }
+    default:
+      NOTREACHED();
+  }
+}
+
+void NoteTakingHelper::OnExtensionLoaded(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension) {
+  if (IsWhitelistedChromeApp(extension)) {
+    for (auto& observer : observers_)
+      observer.OnAvailableNoteTakingAppsUpdated();
+  }
+}
+
+void NoteTakingHelper::OnExtensionUnloaded(
+    content::BrowserContext* browser_context,
+    const extensions::Extension* extension,
+    extensions::UnloadedExtensionInfo::Reason reason) {
+  if (IsWhitelistedChromeApp(extension)) {
+    for (auto& observer : observers_)
+      observer.OnAvailableNoteTakingAppsUpdated();
+  }
+}
+
 }  // namespace chromeos