When app shortcuts are enabled, do a once-off creation of all shortcuts.

apps/shortcut_manager.cc

Index: apps/shortcut_manager.cc
diff --git a/apps/shortcut_manager.cc b/apps/shortcut_manager.cc
index 13aba1826d05d081a147cc67b11d1277a9095c44..65e372fe58b3f42f2ecdcf8f8dcff55711d17b23 100644
--- a/apps/shortcut_manager.cc
+++ b/apps/shortcut_manager.cc
@@ -4,16 +4,22 @@
 
 #include "apps/shortcut_manager.h"
 
+#include "apps/pref_names.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/ui/web_applications/web_app_ui.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_set.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 
@@ -32,17 +38,29 @@ void CreateShortcutsInApplicationsMenu(
   web_app::CreateShortcuts(shortcut_info, creation_locations);
 }
 
+bool ShouldCreateShortcutFor(const extensions::Extension* extension) {

[Matt Giuca, 2013/06/20 10:19:18]

Thanks for doing this.

+  return extension->is_platform_app() &&
+      extension->location() != extensions::Manifest::COMPONENT &&
+      extension->ShouldDisplayInAppLauncher();
+}
+
 }  // namespace
 
 namespace apps {
 
 ShortcutManager::ShortcutManager(Profile* profile)
     : profile_(profile),
+      prefs_(profile->GetPrefs()),
       weak_factory_(this) {
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED,
                  content::Source<Profile>(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
                  content::Source<Profile>(profile_));
+  // Wait for the profile to be added before running OnceOffCreateShortucts
+  // because the profile's ExtensionService is not available when BCKSs are

[Matt Giuca, 2013/06/20 10:19:18]

BCKSs?

[jackhou1, 2013/06/20 11:03:22]

BrowserContextKeyedServices. The abbreviation has been used elsewhere. But I've
changed the comment to something that makes more sense with
NOTIFICATION_EXTENSIONS_READY.

+  // created.
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,

[benwells, 2013/06/20 08:54:32]

I think you should use a different notification like EXTENSIONS_READY.

[jackhou1, 2013/06/20 11:03:22]

Done.

+                 content::Source<Profile>(profile_));
 }
 
 ShortcutManager::~ShortcutManager() {}
@@ -51,6 +69,10 @@ void ShortcutManager::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   switch (type) {
+    case chrome::NOTIFICATION_PROFILE_ADDED: {
+      OnceOffCreateShortcuts();
+      break;
+    }
     case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
 #if defined(OS_MACOSX)
       if (!CommandLine::ForCurrentProcess()->
@@ -62,9 +84,7 @@ void ShortcutManager::Observe(int type,
           content::Details<const extensions::InstalledExtensionInfo>(details)
               .ptr();
       const Extension* extension = installed_info->extension;
-      if (extension->is_platform_app() &&
-          extension->location() != extensions::Manifest::COMPONENT &&
-          extension->ShouldDisplayInAppLauncher()) {
+      if (ShouldCreateShortcutFor(extension)) {
         // If the app is being updated, update any existing shortcuts but do not
         // create new ones. If it is being installed, automatically create a
         // shortcut in the applications menu (e.g., Start Menu).
@@ -94,6 +114,34 @@ void ShortcutManager::Observe(int type,
   }
 }
 
+void ShortcutManager::OnceOffCreateShortcuts() {
+  bool was_enabled = prefs_->GetBoolean(apps::prefs::kAppShortcutsEnabled);

[benwells, 2013/06/20 08:54:32]

The preference name doesn't feel right. Can you make it HaveCreatedAppShortcuts?
I think it would be clearer. Or perhaps, if you prefer, AppShortcutsWereEnabled.

[jackhou1, 2013/06/20 11:03:22]

Went with Matt's suggestion: ShortcutsHaveBeenCreated.

+
+#if defined(OS_MACOSX)
+  bool is_now_enabled =

[Matt Giuca, 2013/06/20 10:19:18]

Add a comment explaining why Mac is special and that this is a temporary
measure.

[jackhou1, 2013/06/20 11:03:22]

Done.

+      CommandLine::ForCurrentProcess()->HasSwitch(switches::kEnableAppShims);
+#else
+  bool is_now_enabled = true;
+#endif  // defined(OS_MACOSX)
+
+  if (was_enabled != is_now_enabled)

[Matt Giuca, 2013/06/20 10:19:18]

I don't think you want to set this to false once it has been set to true (for
some reason). If you just put at the start of this function:

if (prefs_->GetBoolean(...))
  return;

Then you will be able to simplify a lot of this logic. Then it will be all about
turning it on, not possibly turning it on or off.

[jackhou1, 2013/06/20 11:03:22]

Yeah that's how I had it initially, but it needs to turn off on Mac. E.g. if the
user enables and later disables app shims, this should turn off so that when we
enable them for good, the shortcuts are created again.

+    prefs_->SetBoolean(apps::prefs::kAppShortcutsEnabled, is_now_enabled);
+
+  if (was_enabled || !is_now_enabled)
+    return;
+
+  ExtensionServiceInterface* extension_service =

[Matt Giuca, 2013/06/20 10:19:18]

// Create an applications menu shortcut for each app in this profile.

[jackhou1, 2013/06/20 11:03:22]

Done.

+      extensions::ExtensionSystem::Get(profile_)->extension_service();
+
+  const ExtensionSet* apps = extension_service->extensions();
+  for (ExtensionSet::const_iterator it = apps->begin();
+       it != apps->end(); ++it) {
+    if (ShouldCreateShortcutFor(*it))
+      web_app::UpdateShortcutInfoAndIconForApp(
+          *(*it), profile_, base::Bind(&CreateShortcutsInApplicationsMenu));
+  }
+}
+
 void ShortcutManager::DeleteApplicationShortcuts(
     const Extension* extension) {
   ShellIntegration::ShortcutInfo delete_info =