Replace OnceOffCreateShortcuts with UpdateShortcutsForAllAppsIfNeeded.

chrome/browser/apps/shortcut_manager.cc

Index: chrome/browser/apps/shortcut_manager.cc
diff --git a/chrome/browser/apps/shortcut_manager.cc b/chrome/browser/apps/shortcut_manager.cc
index 326722cf0c64c6ade62503ad19d8e9b3c388b257..66eaf23f9adc94042c4eb9734456a8668e2a7e00 100644
--- a/chrome/browser/apps/shortcut_manager.cc
+++ b/chrome/browser/apps/shortcut_manager.cc
@@ -12,7 +12,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -24,8 +23,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
-#include "extensions/browser/extension_system.h"
-#include "extensions/common/extension_set.h"
 
 #if defined(OS_MACOSX)
 #include "apps/app_shim/app_shim_mac.h"
@@ -35,6 +32,8 @@ using extensions::Extension;
 
 namespace {
 
+const int kCurrentAppShortcutsVersion = 1;

[Matt Giuca, 2014/05/07 05:21:51]

This needs a comment. Please say something about exactly what conditions this
should be bumped for (e.g., whenever a new version of Chrome would change the
way app shortcuts are written to disk, bump this number to force them all to be
recreated).

Also say that 0 means that no app shortcuts have ever been created.

[jackhou1, 2014/05/22 05:26:47]

Done.

The 0 case should be gone soon. I'll put up a CL to remove --disable-app-shims.

+
 // Creates a shortcut for an application in the applications menu, if there is
 // not already one present.
 void CreateShortcutsInApplicationsMenu(Profile* profile,
@@ -47,20 +46,14 @@ void CreateShortcutsInApplicationsMenu(Profile* profile,
       web_app::SHORTCUT_CREATION_AUTOMATED, creation_locations, profile, app);
 }
 
-bool ShouldCreateShortcutFor(const Extension* extension) {
-  return extension->is_platform_app() &&
-      extension->location() != extensions::Manifest::COMPONENT &&
-      extension->ShouldDisplayInAppLauncher();
-}
-
 }  // namespace
 
 // static
 void AppShortcutManager::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   // Indicates whether app shortcuts have been created.
-  registry->RegisterBooleanPref(
-      prefs::kAppShortcutsHaveBeenCreated, false,
+  registry->RegisterIntegerPref(
+      prefs::kAppShortcutsVersion, 0,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
@@ -78,7 +71,8 @@ AppShortcutManager::AppShortcutManager(Profile* profile)
                  content::Source<Profile>(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNINSTALLED,
                  content::Source<Profile>(profile_));
-  // Wait for extensions to be ready before running OnceOffCreateShortcuts.
+  // Wait for extensions to be ready before running
+  // UpdateShortcutsForAllAppsIfNeeded.
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSIONS_READY,
                  content::Source<Profile>(profile_));
 
@@ -104,7 +98,7 @@ void AppShortcutManager::Observe(int type,
                                  const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_EXTENSIONS_READY: {
-      OnceOffCreateShortcuts();
+      UpdateShortcutsForAllAppsIfNeeded();
       break;
     }
     case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
@@ -123,7 +117,7 @@ void AppShortcutManager::Observe(int type,
       if (installed_info->is_update) {
         web_app::UpdateAllShortcuts(
             base::UTF8ToUTF16(installed_info->old_name), profile_, extension);
-      } else if (ShouldCreateShortcutFor(extension)) {
+      } else {
         CreateShortcutsInApplicationsMenu(profile_, extension);
       }
       break;
@@ -149,36 +143,20 @@ void AppShortcutManager::OnProfileWillBeRemoved(
                  profile_path));
 }
 
-void AppShortcutManager::OnceOffCreateShortcuts() {
-  bool was_enabled = prefs_->GetBoolean(prefs::kAppShortcutsHaveBeenCreated);
+void AppShortcutManager::UpdateShortcutsForAllAppsIfNeeded() {
+  int last_version = prefs_->GetInteger(prefs::kAppShortcutsVersion);
+  int current_version = kCurrentAppShortcutsVersion;
 
   // Creation of shortcuts on Mac currently can be disabled with
   // --disable-app-shims, so check the flag, and set the pref accordingly.
 #if defined(OS_MACOSX)
-  bool is_now_enabled = apps::IsAppShimsEnabled();
-#else
-  bool is_now_enabled = true;
+  if (!apps::IsAppShimsEnabled())
+    current_version = 0;
 #endif  // defined(OS_MACOSX)
 
-  if (was_enabled != is_now_enabled)
-    prefs_->SetBoolean(prefs::kAppShortcutsHaveBeenCreated, is_now_enabled);
-
-  if (was_enabled || !is_now_enabled)
+  if (last_version == current_version)

[Matt Giuca, 2014/05/07 05:21:51]

Should probably be (last_version >= current_version), so you don't rewrite
shortcuts that are somehow from the future.

Also, since you're using a special value 0 for --disable-app-shims, your logic
will write a shortcut if there is an existing shortcut and --disable-app-shims,
which you don't want.

[jackhou1, 2014/05/22 05:26:47]

Done.

     return;
 
-  // Check if extension system/service are available. They might not be in
-  // tests.
-  extensions::ExtensionSystem* extension_system;
-  ExtensionServiceInterface* extension_service;
-  if (!(extension_system = extensions::ExtensionSystem::Get(profile_)) ||
-      !(extension_service = extension_system->extension_service()))
-    return;
-
-  // Create an applications menu shortcut for each app in this profile.
-  const extensions::ExtensionSet* apps = extension_service->extensions();
-  for (extensions::ExtensionSet::const_iterator it = apps->begin();
-       it != apps->end(); ++it) {
-    if (ShouldCreateShortcutFor(it->get()))
-      CreateShortcutsInApplicationsMenu(profile_, it->get());
-  }
+  web_app::UpdateShortcutsForAllApps(profile_);

[calamity, 2014/05/07 06:00:19]

Maybe this should support a callback so that the pref is set after all IO
operations are done. Of course, that would require plumbing callback all the way
down. I wouldn't worry about it for this patch.

+  prefs_->SetInteger(prefs::kAppShortcutsVersion, current_version);
 }