arc: Support pinned apps across Arc-enabled and Arc-disabled platforms.

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 80035443341bb1581e4bcb8b9537cb7969606033..cd36eb6087dd45068980f76d0fdf172e9e74ae1b 100644
--- a/chrome/browser/ui/ash/chrome_launcher_prefs.cc
+++ b/chrome/browser/ui/ash/chrome_launcher_prefs.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
+#include "chrome/browser/ui/app_list/app_list_syncable_service.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "chrome/browser/ui/ash/launcher/launcher_controller_helper.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -21,6 +22,7 @@
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
+#include "sync/api/string_ordinal.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 
@@ -191,6 +193,20 @@ std::vector<std::string> GetActivitiesForPackage(
   return activities;
 }
 
+struct PinInfo {
+  PinInfo(const std::string& app_id, const syncer::StringOrdinal& item_ordinal)
+      : app_id(app_id), item_ordinal(item_ordinal) {}
+
+  std::string app_id;
+  syncer::StringOrdinal item_ordinal;
+};
+
+struct ComparePinInfo {
+  bool operator()(const PinInfo& pin1, const PinInfo& pin2) {
+    return pin1.item_ordinal.LessThan(pin2.item_ordinal);
+  }
+};
+
 }  // namespace
 
 const char kPinnedAppsPrefAppIDPath[] = "id";
@@ -292,29 +308,14 @@ void SetShelfAlignmentPref(PrefService* prefs,
   }
 }
 
-std::vector<std::string> GetPinnedAppsFromPrefs(
+// Helper that extracts app list from policy preferences.
+std::vector<std::string> GetAppsPinnedByPolicy(
     const PrefService* prefs,
     const LauncherControllerHelper* 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(!helper->IsValidIDForCurrentUser(kPinnedAppsPlaceholder));
-
   std::vector<std::string> apps;
-  const auto* pinned = prefs->GetList(prefs::kPinnedLauncherApps);
   const auto* policy = prefs->GetList(prefs::kPolicyPinnedLauncherApps);
-
-  // Get the sanitized preference value for the index of the Chrome app icon.
-  const size_t chrome_icon_index = std::max<size_t>(
-      0, std::min<size_t>(pinned->GetSize(),
-                          prefs->GetInteger(prefs::kShelfChromeIconIndex)));
-
-  // Check if Chrome is in either of the the preferences lists.
-  std::unique_ptr<base::Value> chrome_app(
-      ash::CreateAppDict(extension_misc::kChromeAppId));
-  bool chrome_listed =
-      (pinned->Find(*chrome_app.get()) != pinned->end() ||
-       (policy && policy->Find(*chrome_app.get()) != policy->end()));
+  if (!policy)
+    return apps;
 
   // Obtain here all ids of ARC apps because it takes linear time, and getting
   // them in the loop bellow would lead to quadratic complexity.
@@ -324,7 +325,7 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
                         : std::vector<std::string>());
 
   std::string app_id;
-  for (size_t i = 0; policy && (i < policy->GetSize()); ++i) {
+  for (size_t i = 0; i < policy->GetSize(); ++i) {
     const base::DictionaryValue* dictionary = nullptr;
     if (policy->GetDictionary(i, &dictionary) &&
         dictionary->GetString(kPinnedAppsPrefAppIDPath, &app_id) &&
@@ -349,6 +350,35 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
     }
   }
 
+  return apps;
+}
+
+std::vector<std::string> GetPinnedAppsFromPrefsDepricated(

[stevenjb, 2016/06/10 18:00:41]

s/Depricated/Deprecated/, but also maybe use Legacy instead of Deprecated since
we won't be able to fully deprecate the pref store for several releases.

[khmel, 2016/06/10 22:08:10]

Legacy sounds better, thanks

+    const PrefService* prefs,
+    const LauncherControllerHelper* 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(!helper->IsValidIDForCurrentUser(kPinnedAppsPlaceholder));
+
+  const auto* pinned = prefs->GetList(prefs::kPinnedLauncherApps);
+  const auto* policy = prefs->GetList(prefs::kPolicyPinnedLauncherApps);
+
+  // Get the sanitized preference value for the index of the Chrome app icon.
+  const size_t chrome_icon_index = std::max<size_t>(
+      0, std::min<size_t>(pinned->GetSize(),
+                          prefs->GetInteger(prefs::kShelfChromeIconIndex)));
+
+  // Check if Chrome is in either of the the preferences lists.
+  std::unique_ptr<base::Value> chrome_app(
+      ash::CreateAppDict(extension_misc::kChromeAppId));
+  bool chrome_listed =
+      (pinned->Find(*chrome_app.get()) != pinned->end() ||
+       (policy && policy->Find(*chrome_app.get()) != policy->end()));
+
+  std::vector<std::string> apps = GetAppsPinnedByPolicy(prefs, helper);
+
+  std::string app_id;
   for (size_t i = 0; i < pinned->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.
@@ -369,11 +399,6 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
     }
   }
 
-  if (arc::ArcAuthService::IsAllowedForProfile(helper->profile()) &&
-      helper->IsValidIDForCurrentUser(ArcSupportHost::kHostAppId)) {
-    apps.push_back(ArcSupportHost::kHostAppId);
-  }
-
   // If not added yet, the chrome item will be the last item in the list.
   if (!chrome_listed)
     apps.push_back(extension_misc::kChromeAppId);
@@ -385,4 +410,253 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
   return apps;
 }
 
+// Helper to creates pin position that stays before any synced app, even if

[stevenjb, 2016/06/10 18:00:40]

s/creates/create/

[khmel, 2016/06/10 22:08:10]

Done.

+// app is not currently visbile on a device.
+syncer::StringOrdinal CreateFirstPinPosition(Profile* profile) {
+  syncer::StringOrdinal position;
+  app_list::AppListSyncableService* app_service =
+      app_list::AppListSyncableService::Get(profile);
+  for (const auto& sync_peer : app_service->sync_items()) {
+    if (!sync_peer.second->item_pin_ordinal.IsValid())
+      continue;
+    if (!position.IsValid() ||
+        sync_peer.second->item_pin_ordinal.LessThan(position)) {
+      position = sync_peer.second->item_pin_ordinal;
+    }
+  }
+
+  return position.IsValid() ? position.CreateBefore()
+                            : syncer::StringOrdinal::CreateInitialOrdinal();
+}
+
+// Helper to creates pin position that stays before any synced app, even if
+// app is not currently visbile on a device.
+syncer::StringOrdinal CreateLastPinPosition(Profile* profile) {
+  syncer::StringOrdinal position;
+  app_list::AppListSyncableService* app_service =
+      app_list::AppListSyncableService::Get(profile);
+  for (const auto& sync_peer : app_service->sync_items()) {
+    if (!sync_peer.second->item_pin_ordinal.IsValid())
+      continue;
+    if (!position.IsValid() ||
+        sync_peer.second->item_pin_ordinal.GreaterThan(position)) {
+      position = sync_peer.second->item_pin_ordinal;
+    }
+  }
+
+  return position.IsValid() ? position.CreateAfter()
+                            : syncer::StringOrdinal::CreateInitialOrdinal();
+}
+
+std::vector<std::string> GetPinnedAppsFromPrefs(
+    const PrefService* prefs,
+    LauncherControllerHelper* helper) {
+  app_list::AppListSyncableService* app_service =
+      app_list::AppListSyncableService::Get(helper->profile());
+  // Some unit tests may not have it.
+  if (!app_service)
+    return std::vector<std::string>();
+
+  const std::vector<std::string> policy_apps =
+      GetAppsPinnedByPolicy(prefs, helper);
+
+  std::vector<PinInfo> pin_infos;
+
+  // Empty pins indicates that sync based pin model is used for the first
+  // time. In normal workflow we have at least Chrom browser pin info.

[stevenjb, 2016/06/10 18:00:41]

Chrome

[khmel, 2016/06/10 22:08:10]

Done.

+  bool first_run = true;
+
+  for (const auto& sync_peer : app_service->sync_items()) {
+    if (!sync_peer.second->item_pin_ordinal.IsValid())
+      continue;
+
+    first_run = false;
+    // Don't include apps that are currently do not exist on device.

[stevenjb, 2016/06/10 18:00:40]

s/are//

[khmel, 2016/06/10 22:08:10]

Done.

+    if (sync_peer.first != extension_misc::kChromeAppId &&
+        !helper->IsValidIDForCurrentUser(sync_peer.first)) {
+      continue;
+    }
+
+    const bool pin_by_policy = std::find(policy_apps.begin(), policy_apps.end(),
+                                         sync_peer.first) != policy_apps.end();

[stevenjb, 2016/06/10 18:00:40]

If the app list gets large this could be expensive. We should use a set to store
the policy app ids (or sort the vector).

+    // Check if item was pinned by policy only and currently is not in policy.
+    if (!pin_by_policy && sync_peer.second->item_pin_by_policy)
+      continue;
+
+    pin_infos.push_back(
+        PinInfo(sync_peer.first, sync_peer.second->item_pin_ordinal));
+  }
+
+  if (first_run) {

[stevenjb, 2016/06/10 18:00:41]

We should wrap this into a separate function, e.g.
if (first_run)
  return ImportLagacyPinnedApps(...);

[khmel, 2016/06/10 22:08:10]

Done.

+    // We need to import legacy pins model and convert it to sync based
+    // model.
+    std::vector<std::string> legacy_pins =
+        GetPinnedAppsFromPrefsDepricated(prefs, helper);
+    DCHECK(!legacy_pins.empty());
+
+    legacy_pins.erase(
+        std::remove_if(legacy_pins.begin(), legacy_pins.end(),
+                       [](const std::string& app_id) {
+                         return (app_id == kPinnedAppsPlaceholder);
+                       }),
+        legacy_pins.end());

[stevenjb, 2016/06/10 18:00:41]

We are already iterating over legacy_pins below, we should just use a
traditional iterator and erase the kPinnedAppsPlaceholder placeholder there.

[khmel, 2016/06/10 22:08:10]

Done.
Refactored GetPinnedAppsFromPrefsLegacy not to return kPinnedAppsPlaceholder,
GetPinnedAppsFromPrefsLegacy is now private helper and this won't affect other
code.

+    DCHECK(!legacy_pins.empty());
+
+    syncer::StringOrdinal last_position;
+    // Convert to sync item record.
+    for (const auto& app_id : legacy_pins) {
+      last_position = last_position.IsValid()
+                          ? last_position.CreateAfter()
+                          : syncer::StringOrdinal::CreateInitialOrdinal();

[stevenjb, 2016/06/10 18:00:40]

nit: Just initialize last_position to
syncer::StringOrdinal::CreateInitialOrdinal()

[khmel, 2016/06/10 22:08:11]

That is nicer, thanks

+      const bool pin_by_policy =
+          std::find(policy_apps.begin(), policy_apps.end(), app_id) !=
+          policy_apps.end();

[stevenjb, 2016/06/10 18:00:41]

This will also avoid an N^2 operation if policy_apps is a set.

[khmel, 2016/06/10 22:08:10]

Done.

+      app_service->SetPinPosition(app_id, last_position, pin_by_policy);
+    }
+
+    // Now process default apps.
+    for (size_t i = 0; i < arraysize(kDefaultPinnedApps); ++i) {
+      const std::string& app_id = kDefaultPinnedApps[i];
+      // Check if it is already imported.
+      if (app_service->GetPinPosition(app_id).IsValid())
+        continue;
+      // Check if it is present but not in legacy pin.
+      if (helper->IsValidIDForCurrentUser(app_id))
+        continue;
+      DCHECK(last_position.IsValid());

[stevenjb, 2016/06/10 18:00:41]

Just check this once above.

[khmel, 2016/06/10 22:08:10]

Done.

+      last_position = last_position.CreateAfter();
+      app_service->SetPinPosition(app_id, last_position, false);
+    }
+
+    return legacy_pins;
+  }
+
+  // Sort pins according its ordinals.

[stevenjb, 2016/06/10 18:00:40]

s/its/their/

[khmel, 2016/06/10 22:08:10]

Done.

+  std::sort(pin_infos.begin(), pin_infos.end(), ComparePinInfo());
+
+  // Now insert Chrome browser app if needed.
+  syncer::StringOrdinal chrome_position =
+      app_service->GetPinPosition(extension_misc::kChromeAppId);
+  if (!chrome_position.IsValid()) {
+    chrome_position = CreateFirstPinPosition(helper->profile());
+    pin_infos.insert(pin_infos.begin(),
+                     PinInfo(extension_misc::kChromeAppId, chrome_position));
+    app_service->SetPinPosition(extension_misc::kChromeAppId, chrome_position,
+                                false);
+  }
+
+  // Policy apps must preserve order of appearance and be first app in the list.
+  // Validate that there have correct position and fix if needed.

[stevenjb, 2016/06/10 18:00:40]

s/there have correct/they have a correct/

[khmel, 2016/06/10 22:08:10]

Done.

[khmel, 2016/06/10 22:08:11]

Done.

+  size_t shelf_index = 0;
+  for (const auto& app_id : policy_apps) {
+    if (app_id == kPinnedAppsPlaceholder)
+      continue;
+    if (app_id == extension_misc::kChromeAppId)
+      continue;
+
+    // Chrome browser app has right to appear between pinned by policy apps.
+    if (shelf_index < pin_infos.size() &&
+        pin_infos[shelf_index].app_id == extension_misc::kChromeAppId) {
+      ++shelf_index;
+    }
+
+    const bool need_position_item = shelf_index >= pin_infos.size() ||
+                                    app_id != pin_infos[shelf_index].app_id;
+    if (need_position_item) {
+      // Remove existing app that breaks the order from the list.
+      if (shelf_index < pin_infos.size()) {
+        pin_infos.erase(
+            std::remove_if(pin_infos.begin() + shelf_index + 1, pin_infos.end(),
+                           [app_id](const PinInfo& pin_info) {
+                             return (pin_info.app_id == app_id);
+                           }),
+            pin_infos.end());
+      }
+
+      syncer::StringOrdinal new_shelf_ordinal;
+      if (shelf_index == 0) {
+        // First app.
+        new_shelf_ordinal = pin_infos.empty()
+                                ? syncer::StringOrdinal::CreateInitialOrdinal()
+                                : pin_infos.front().item_ordinal.CreateBefore();
+      } else if (shelf_index == pin_infos.size()) {
+        new_shelf_ordinal = pin_infos.back().item_ordinal.CreateAfter();
+      } else {
+        new_shelf_ordinal =
+            pin_infos[shelf_index - 1].item_ordinal.CreateBetween(
+                pin_infos[shelf_index].item_ordinal);
+      }
+      pin_infos.insert(pin_infos.begin() + shelf_index,
+                       PinInfo(app_id, new_shelf_ordinal));
+      const bool pin_by_policy =
+          !app_service->GetPinPosition(app_id).IsValid() ||
+          app_service->GetPinByPolicy(app_id);
+      app_service->SetPinPosition(app_id, new_shelf_ordinal, pin_by_policy);
+    }
+    ++shelf_index;
+  }
+
+  if (arc::ArcAuthService::IsAllowedForProfile(helper->profile()) &&
+      helper->IsValidIDForCurrentUser(ArcSupportHost::kHostAppId)) {
+    if (!app_service->GetSyncItem(ArcSupportHost::kHostAppId)) {
+      const syncer::StringOrdinal arc_host_position =
+          CreateLastPinPosition(helper->profile());
+      pin_infos.insert(pin_infos.begin(),
+                       PinInfo(ArcSupportHost::kHostAppId, arc_host_position));
+      app_service->SetPinPosition(ArcSupportHost::kHostAppId, arc_host_position,
+                                  false);
+    }
+  }
+
+  // Convert to string array.
+  std::vector<std::string> pins(pin_infos.size());
+  for (size_t i = 0; i < pin_infos.size(); ++i)
+    pins[i] = pin_infos[i].app_id;
+
+  return pins;
+}
+
+void RemovePinPosition(Profile* profile, const std::string& app_id) {
+  DCHECK(profile);
+  DCHECK(!app_id.empty());
+  app_list::AppListSyncableService* app_service =
+      app_list::AppListSyncableService::Get(profile);
+  app_service->SetPinPosition(app_id, syncer::StringOrdinal(), false);
+}
+
+void SetPinPosition(Profile* profile,
+                    const std::string& app_id,
+                    const std::string& app_id_before,
+                    const std::string& app_id_after) {
+  DCHECK(profile);
+  DCHECK(!app_id.empty());
+  DCHECK_NE(app_id, app_id_before);
+  DCHECK_NE(app_id, app_id_after);
+  DCHECK(app_id_before.empty() || app_id_before != app_id_after);
+
+  app_list::AppListSyncableService* app_service =
+      app_list::AppListSyncableService::Get(profile);
+  // Some unit tests may not have this service.
+  if (!app_service)
+    return;
+
+  syncer::StringOrdinal position_before =
+      app_id_before.empty() ? syncer::StringOrdinal()
+                            : app_service->GetPinPosition(app_id_before);
+  syncer::StringOrdinal position_after =
+      app_id_after.empty() ? syncer::StringOrdinal()
+                           : app_service->GetPinPosition(app_id_after);
+
+  syncer::StringOrdinal pin_position;
+  if (position_before.IsValid() && position_after.IsValid())
+    pin_position = position_before.CreateBetween(position_after);
+  else if (position_before.IsValid())
+    pin_position = position_before.CreateAfter();
+  else if (position_after.IsValid())
+    pin_position = position_after.CreateBefore();
+  else
+    pin_position = syncer::StringOrdinal::CreateInitialOrdinal();
+  app_service->SetPinPosition(app_id, pin_position, false);
+}
+
 }  // namespace ash