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

apps/shortcut_manager.cc

 // Copyright (c) 2012 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.
 
 #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"
 
 using extensions::Extension;
 
 namespace {
 
 // Creates a shortcut for an application in the applications menu.
 void CreateShortcutsInApplicationsMenu(
     const ShellIntegration::ShortcutInfo& shortcut_info) {
   ShellIntegration::ShortcutLocations creation_locations;
   creation_locations.in_applications_menu = true;
   // Create the shortcut in the Chrome Apps subdir.
   creation_locations.applications_menu_subdir =
       web_app::GetAppShortcutsSubdirName();
   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() {}
 
 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()->
           HasSwitch(switches::kEnableAppShims))
         break;
 #endif  // defined(OS_MACOSX)
 
       const extensions::InstalledExtensionInfo* installed_info =
           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).
         base::Callback<void(const ShellIntegration::ShortcutInfo&)>
             create_or_update;
         if (installed_info->is_update) {
           string16 old_title = UTF8ToUTF16(installed_info->old_name);
           create_or_update = base::Bind(&web_app::UpdateAllShortcuts,
                                         old_title);
         } else {
           create_or_update = base::Bind(&CreateShortcutsInApplicationsMenu);
         }
 
         web_app::UpdateShortcutInfoAndIconForApp(*extension, profile_,
                                                  create_or_update);
       }
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_UNINSTALLED: {
       const Extension* extension = content::Details<const Extension>(
           details).ptr();
       DeleteApplicationShortcuts(extension);
       break;
     }
     default:
       NOTREACHED();
   }
 }
 
+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 =
       web_app::ShortcutInfoForExtensionAndProfile(extension, profile_);
   web_app::DeleteAllShortcuts(delete_info);
 }
 
 }  // namespace apps