Implement local storage for App List in case app sync is off.

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 f4b87d8398319b2a81a355dafb0a992e1b7997fe..80f09e9d50515b2d3d6e69a7237d2b4a1543de42 100644
--- a/chrome/browser/ui/ash/chrome_launcher_prefs.cc
+++ b/chrome/browser/ui/ash/chrome_launcher_prefs.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 #include "chrome/browser/prefs/pref_service_syncable_util.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service.h"
 #include "chrome/browser/ui/app_list/app_list_syncable_service_factory.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
@@ -270,6 +271,32 @@ class AppTracker {
   std::set<std::string> app_set_;
 };
 
+// If sync is running then return position from sync service, otherwise use
+// local copy.
+syncer::StringOrdinal GetPinPosition(
+    app_list::AppListSyncableService* app_service,
+    const std::string& app_id) {
+  DCHECK(app_service);
+  if (app_service->is_active())
+    return app_service->GetPinPosition(app_id);
+
+  const base::DictionaryValue* apps =
+      app_service->profile()->GetPrefs()->GetDictionary(prefs::kShelfPins);
+  DCHECK(apps);
+  std::string position_string;
+  if (!apps->GetString(app_id, &position_string))
+    return syncer::StringOrdinal();
+
+  const syncer::StringOrdinal position(position_string);
+  if (!position.IsValid()) {
+    LOG(ERROR) << "Found wrong pin position " << position_string
+               << " in local copy for " << app_id;
+    return syncer::StringOrdinal();
+  }
+
+  return position;
+}
+
 }  // namespace
 
 const char kPinnedAppsPrefAppIDPath[] = "id";
@@ -307,6 +334,7 @@ void RegisterChromeLauncherUserPrefs(
   registry->RegisterDictionaryPref(prefs::kShelfPreferences);
   registry->RegisterIntegerPref(prefs::kLogoutDialogDurationMs, 20000);
   registry->RegisterBooleanPref(prefs::kShowLogoutButtonInTray, false);
+  registry->RegisterDictionaryPref(prefs::kShelfPins);
 }
 
 base::DictionaryValue* CreateAppDict(const std::string& app_id) {
@@ -543,12 +571,17 @@ std::vector<std::string> ImportLegacyPinnedApps(
   app_list::AppListSyncableService* app_service =
       app_list::AppListSyncableServiceFactory::GetForProfile(helper->profile());
 
+  // Update pin info in local copy in order to keep it up to date.
+  DictionaryPrefUpdate pref_update(helper->profile()->GetPrefs(),
+                                   prefs::kShelfPins);
+
   syncer::StringOrdinal last_position =
       syncer::StringOrdinal::CreateInitialOrdinal();
   // Convert to sync item record.
   for (const auto& app_id : legacy_pins) {
     DCHECK_NE(kPinnedAppsPlaceholder, app_id);
     app_service->SetPinPosition(app_id, last_position);
+    pref_update->SetString(app_id, last_position.ToInternalValue());
     last_position = last_position.CreateAfter();
   }
 
@@ -556,18 +589,28 @@ std::vector<std::string> ImportLegacyPinnedApps(
   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())
+    if (GetPinPosition(app_service, app_id).IsValid())
       continue;
     // Check if it is present but not in legacy pin.
     if (helper->IsValidIDForCurrentUser(app_id))
       continue;
     app_service->SetPinPosition(app_id, last_position);
+    pref_update->SetString(app_id, last_position.ToInternalValue());
     last_position = last_position.CreateAfter();
   }
 
   return legacy_pins;
 }
 
+void SavePinnedAppsInLocalCopy(Profile* profile,
+                               const std::vector<PinInfo>& pins) {
+  DictionaryPrefUpdate pref_update(profile->GetPrefs(), prefs::kShelfPins);
+  pref_update->Clear();
+  for (const auto pin : pins) {
+    pref_update->SetString(pin.app_id, pin.item_ordinal.ToInternalValue());
+  }
+}
+
 std::vector<std::string> GetPinnedAppsFromPrefs(
     const PrefService* prefs,
     LauncherControllerHelper* helper) {
@@ -582,23 +625,51 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
   AppTracker policy_apps;
   GetAppsPinnedByPolicy(prefs, helper, &policy_apps);
 
+  const base::DictionaryValue* apps_in_local_copy =
+      helper->profile()->GetPrefs()->GetDictionary(prefs::kShelfPins);
+  DCHECK(apps_in_local_copy);
+
   // Empty pins indicates that sync based pin model is used for the first
   // time. In normal workflow we have at least Chrome browser pin info.
-  bool first_run = true;
+  // In case sync is not started, first run is detected by absence of local
+  // pins copy.
+  bool first_run = apps_in_local_copy->empty();
 
-  for (const auto& sync_peer : app_service->sync_items()) {
-    if (!sync_peer.second->item_pin_ordinal.IsValid())
-      continue;
+  if (app_service->is_active()) {
+    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 currently do not exist on device.
-    if (sync_peer.first != extension_misc::kChromeAppId &&
-        !helper->IsValidIDForCurrentUser(sync_peer.first)) {
-      continue;
+      first_run = false;
+      // Don't include apps that currently do not exist on device.
+      if (sync_peer.first != extension_misc::kChromeAppId &&
+          !helper->IsValidIDForCurrentUser(sync_peer.first)) {
+        continue;
+      }
+
+      pin_infos.push_back(
+          PinInfo(sync_peer.first, sync_peer.second->item_pin_ordinal));
     }
+  } else {
+    // If sync is not active, use information from local copy.
+    for (base::DictionaryValue::Iterator app(*apps_in_local_copy);
+         !app.IsAtEnd(); app.Advance()) {
+      // Don't include apps that currently do not exist on device.
+      if (app.key() != extension_misc::kChromeAppId &&
+          !helper->IsValidIDForCurrentUser(app.key())) {
+        continue;
+      }
+
+      std::string position_string;
+      app.value().GetAsString(&position_string);
+      const syncer::StringOrdinal position(position_string);
+      if (!position.IsValid()) {
+        NOTREACHED();
+        continue;
+      }

[stevenjb, 2016/10/14 20:27:25]

This should just be:
DCHECK(position.IsValid());

DCHECK / NOTREACHED should only be for situations that are logically impossible
and serve primarily to document the code, and secondarily to catch bugs early if
the logic is somehow incorrect.

If it is logically possible but an error for this to occur, use LOG(ERROR)
instead of NOTREACHED().

[khmel, 2016/10/17 21:10:57]

Yes, I agree with your point of view, thanks for explanation.

 
-    pin_infos.push_back(
-        PinInfo(sync_peer.first, sync_peer.second->item_pin_ordinal));
+      pin_infos.push_back(PinInfo(app.key(), position));
+    }
   }
 
   if (first_run) {
@@ -620,7 +691,7 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
       continue;
 
     // Check if we already processed current app.
-    if (app_service->GetPinPosition(app_id).IsValid())
+    if (GetPinPosition(app_service, app_id).IsValid())
       continue;
 
     // Now move it to the front.
@@ -630,14 +701,17 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
   }
 
   // Now insert Chrome browser app if needed.
-  if (!app_service->GetPinPosition(extension_misc::kChromeAppId).IsValid()) {
+  if (!GetPinPosition(app_service, extension_misc::kChromeAppId).IsValid()) {
     pin_infos.insert(pin_infos.begin(),
                      PinInfo(extension_misc::kChromeAppId, front_position));
     app_service->SetPinPosition(extension_misc::kChromeAppId, front_position);
   }
 
   if (helper->IsValidIDForCurrentUser(ArcSupportHost::kHostAppId)) {
-    if (!app_service->GetSyncItem(ArcSupportHost::kHostAppId)) {
+    // TODO(khmel) Find the better way to initial pin PlayStore item when sync
+    // is off.

[stevenjb, 2016/10/14 20:27:24]

'find a way' ? It looks like we currently don't handle inserting the play store
item if app list sync is disabled?

[khmel, 2016/10/17 21:10:57]

With new approach this is not required.

+    if (app_service->is_active() &&
+        !app_service->GetSyncItem(ArcSupportHost::kHostAppId)) {
       const syncer::StringOrdinal arc_host_position =
           GetLastPinPosition(helper->profile());
       pin_infos.insert(pin_infos.begin(),
@@ -652,12 +726,21 @@ std::vector<std::string> GetPinnedAppsFromPrefs(
   for (size_t i = 0; i < pin_infos.size(); ++i)
     pins[i] = pin_infos[i].app_id;
 
+  // Sync current pins with local copy if required.
+  if (app_service->is_active())
+    SavePinnedAppsInLocalCopy(helper->profile(), pin_infos);
+
   return pins;
 }
 
 void RemovePinPosition(Profile* profile, const std::string& app_id) {
   DCHECK(profile);
   DCHECK(!app_id.empty());
+
+  // Remove pin info from local copy in order to keep it up to date.
+  DictionaryPrefUpdate pref_update(profile->GetPrefs(), prefs::kShelfPins);
+  pref_update->Remove(app_id, nullptr);
+
   app_list::AppListSyncableService* app_service =
       app_list::AppListSyncableServiceFactory::GetForProfile(profile);
   app_service->SetPinPosition(app_id, syncer::StringOrdinal());
@@ -679,12 +762,12 @@ void SetPinPosition(Profile* profile,
 
   syncer::StringOrdinal position_before =
       app_id_before.empty() ? syncer::StringOrdinal()
-                            : app_service->GetPinPosition(app_id_before);
+                            : GetPinPosition(app_service, app_id_before);
   syncer::StringOrdinal position_after;
   for (const auto& app_id_after : app_ids_after) {
     DCHECK_NE(app_id_after, app_id);
     DCHECK_NE(app_id_after, app_id_before);
-    syncer::StringOrdinal position = app_service->GetPinPosition(app_id_after);
+    syncer::StringOrdinal position = GetPinPosition(app_service, app_id_after);
     DCHECK(position.IsValid());
     if (!position.IsValid()) {
       LOG(ERROR) << "Sync pin position was not found for " << app_id_after;
@@ -705,6 +788,11 @@ void SetPinPosition(Profile* profile,
     pin_position = position_after.CreateBefore();
   else
     pin_position = syncer::StringOrdinal::CreateInitialOrdinal();
+
+  // Update pin info in local copy in order to keep it up to date.
+  DictionaryPrefUpdate pref_update(profile->GetPrefs(), prefs::kShelfPins);
+  pref_update->SetString(app_id, pin_position.ToInternalValue());
+
   app_service->SetPinPosition(app_id, pin_position);
 }