Pin apps from prefs on the mash shelf.

chrome/browser/ui/ash/chrome_launcher_prefs.cc

Index: chrome/browser/ui/ash/chrome_launcher_prefs.cc
diff --git a/chrome/browser/ui/ash/chrome_launcher_prefs.cc b/chrome/browser/ui/ash/chrome_launcher_prefs.cc
index 555487d1287cfbd43c9527ff1f7a0bff6cb7fd10..fc90af2054a5c4d4dca103e9a555e61099faa043 100644
--- a/chrome/browser/ui/ash/chrome_launcher_prefs.cc
+++ b/chrome/browser/ui/ash/chrome_launcher_prefs.cc
@@ -12,6 +12,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
+#include "chrome/browser/ui/ash/launcher/app_tab_helper.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -167,10 +168,30 @@ const char* AutoHideBehaviorToPref(ShelfAutoHideBehavior behavior) {
   return nullptr;
 }
 
+void UpdatePinnedApps(const std::string& app_id,

[sky, 2016/04/27 21:01:33]

I see you're just moving this code, but have some random comments.

+                      bool valid_for_current_user,
+                      std::vector<std::string>* pinned_apps,
+                      bool* chrome_icon_added,
+                      bool* app_list_icon_added) {
+  if (app_id == extension_misc::kChromeAppId) {
+    *chrome_icon_added = true;

[sky, 2016/04/27 21:01:33]

nit: always set to false early on to avoid possible ordering problems. Same goes
for app_list_icon_added.

[msw, 2016/04/29 22:30:49]

This function was called repeatedly with the same arguments (to determine if
they're ever added), so their values are deliberately not cleared on each call.
I've refactored GetPinnedAppsFromPrefs further to make this less confusing.

+    pinned_apps->push_back(extension_misc::kChromeAppId);
+  } else if (app_id == kPinnedAppsPlaceholder) {
+    *app_list_icon_added = true;
+    pinned_apps->push_back(kPinnedAppsPlaceholder);
+  } else if (valid_for_current_user) {
+    // Note: In multi profile scenarios we only want to show pinnable apps
+    // here which is correct. Running applications from the other users will
+    // continue to run. So no need for multi profile modifications.
+    pinned_apps->push_back(app_id);
+  }
+}
+
 }  // namespace
 
 const char kPinnedAppsPrefAppIDPath[] = "id";
 const char kPinnedAppsPrefPinnedByPolicy[] = "pinned_by_policy";
+const char kPinnedAppsPlaceholder[] = "AppShelfIDPlaceholder--------";
 
 const char kShelfAutoHideBehaviorAlways[] = "Always";
 const char kShelfAutoHideBehaviorNever[] = "Never";
@@ -268,4 +289,75 @@ void SetShelfAlignmentPref(PrefService* prefs,
   }
 }
 
+std::vector<std::string> GetPinnedAppsFromPrefs(PrefService* prefs,

[sky, 2016/04/27 21:01:33]

I'm assuming this is a straight move.

[msw, 2016/04/29 22:30:49]

Not quite... I refactored this a bit, my next PS goes further.
I can revert to the original code with very minimal changes if you prefer.

+                                                AppTabHelper* app_tab_helper) {
+  // Adding the app list item to the list of items requires that the ID is not
+  // a valid and known ID for the extension system. The ID was constructed that
+  // way - but just to make sure...
+  DCHECK(!app_tab_helper->IsValidIDForCurrentUser(kPinnedAppsPlaceholder));
+
+  std::vector<std::string> pinned_apps;
+
+  const base::ListValue* pinned_apps_pref =
+      prefs->GetList(prefs::kPinnedLauncherApps);
+  const base::ListValue* policy_pinned_apps_pref =
+      prefs->GetList(prefs::kPolicyPinnedLauncherApps);
+
+  // Keep track of the addition of the chrome and the app list icon.
+  bool chrome_icon_added = false;
+  bool app_list_icon_added = false;
+
+  const size_t chrome_icon_index = std::max<size_t>(
+      0, std::min<size_t>(pinned_apps_pref->GetSize(),
+                          prefs->GetInteger(prefs::kShelfChromeIconIndex)));
+
+  if (policy_pinned_apps_pref) {
+    for (size_t i = 0; i < policy_pinned_apps_pref->GetSize(); ++i) {
+      const base::DictionaryValue* app = nullptr;
+      std::string app_id;
+      if (policy_pinned_apps_pref->GetDictionary(i, &app) &&
+          app->GetString(kPinnedAppsPrefAppIDPath, &app_id) &&
+          (std::find(pinned_apps.begin(), pinned_apps.end(), app_id) ==
+           pinned_apps.end()))
+        UpdatePinnedApps(
+            app_id, app_tab_helper->IsValidIDForCurrentUser(app_id),
+            &pinned_apps, &chrome_icon_added, &app_list_icon_added);
+    }
+  }
+
+  for (size_t i = 0; i < pinned_apps_pref->GetSize(); ++i) {
+    // We need to position the chrome icon relative to its place in the pinned
+    // preference list - even if an item of that list isn't shown yet.
+    if (i == chrome_icon_index && !chrome_icon_added) {
+      pinned_apps.push_back(extension_misc::kChromeAppId);
+      chrome_icon_added = true;
+    }
+    const base::DictionaryValue* app = nullptr;
+    std::string app_id;
+    if (pinned_apps_pref->GetDictionary(i, &app) &&
+        app->GetString(kPinnedAppsPrefAppIDPath, &app_id) &&
+        (std::find(pinned_apps.begin(), pinned_apps.end(), app_id) ==
+         pinned_apps.end())) {
+      bool pinned_by_policy = false;
+      if (app->GetBoolean(kPinnedAppsPrefPinnedByPolicy, &pinned_by_policy) &&
+          pinned_by_policy) {
+        continue;
+      }
+      UpdatePinnedApps(app_id, app_tab_helper->IsValidIDForCurrentUser(app_id),
+                       &pinned_apps, &chrome_icon_added, &app_list_icon_added);
+    }
+  }
+
+  // If not added yet, the chrome item will be the last item in the list.
+  if (!chrome_icon_added)
+    pinned_apps.push_back(extension_misc::kChromeAppId);
+
+  // If not added yet, add the app list item either at the end or at the
+  // beginning - depending on the shelf layout.
+  if (!app_list_icon_added)
+    pinned_apps.insert(pinned_apps.begin(), kPinnedAppsPlaceholder);
+
+  return pinned_apps;
+}
+
 }  // namespace ash