Discard pinning an app with non-empty launcher id.

chrome/browser/ui/ash/chrome_launcher_prefs.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 "chrome/browser/ui/ash/chrome_launcher_prefs.h"
 
 #include <stddef.h>
 
 #include <set>
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/app_mode/app_mode_utils.h"
 #include "chrome/browser/chromeos/arc/arc_support_host.h"
 #include "chrome/browser/prefs/pref_service_syncable_util.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"
+#include "chrome/browser/ui/ash/app_launcher_id.h"
 #include "chrome/browser/ui/ash/launcher/launcher_controller_helper.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/sync/model/string_ordinal.h"
 #include "components/sync_preferences/pref_service_syncable.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/display/types/display_constants.h"
 
 namespace ash {
 namespace launcher {
 
 namespace {
 
+std::unique_ptr<base::DictionaryValue> CreateAppDict(
+    const std::string& app_id) {
+  auto app_value = base::MakeUnique<base::DictionaryValue>();
+  app_value->SetString(kPinnedAppsPrefAppIDPath, app_id);
+  return app_value;
+}
+
 // App ID of default pinned apps.
 const char* kDefaultPinnedApps[] = {
     extension_misc::kGmailAppId, extension_misc::kGoogleDocAppId,
     extension_misc::kYoutubeAppId, ArcSupportHost::kHostAppId};
 
 base::ListValue* CreateDefaultPinnedAppsList() {
   std::unique_ptr<base::ListValue> apps(new base::ListValue);
   for (size_t i = 0; i < arraysize(kDefaultPinnedApps); ++i)
-    apps->Append(CreateAppDict(AppLauncherId(kDefaultPinnedApps[i])));
+    apps->Append(CreateAppDict(kDefaultPinnedApps[i]));
 
   return apps.release();
 }
 
 // Returns the preference value for the display with the given |display_id|.
 // The pref value is stored in |local_path| and |path|, but the pref service may
 // have per-display preferences and the value can be specified by policy.
 // Here is the priority:
 //  * A value managed by policy. This is a single value that applies to all
 //    displays.
@@ skipping 154 matching lines @@
 // If no user-set value exists at |local_path|, the value from |synced_path| is
 // copied to |local_path|.
 void PropagatePrefToLocalIfNotSet(
     sync_preferences::PrefServiceSyncable* pref_service,
     const char* local_path,
     const char* synced_path) {
   if (!pref_service->FindPreference(local_path)->HasUserSetting())
     pref_service->SetString(local_path, pref_service->GetString(synced_path));
 }
 
+std::vector<AppLauncherId> AppIdsToAppLauncherIds(
+    const std::vector<std::string> app_ids) {
+  std::vector<AppLauncherId> app_launcher_ids(app_ids.size(), AppLauncherId());
+  for (size_t i = 0; i < app_ids.size(); ++i)
+    app_launcher_ids[i] = AppLauncherId(app_ids[i]);
+  return app_launcher_ids;
+}
+
 struct PinInfo {
-  PinInfo(const AppLauncherId& app_launcher_id,
-          const syncer::StringOrdinal& item_ordinal)
-      : app_launcher_id(app_launcher_id), item_ordinal(item_ordinal) {}
+  PinInfo(const std::string& app_id, const syncer::StringOrdinal& item_ordinal)
+      : app_id(app_id), item_ordinal(item_ordinal) {}
 
-  AppLauncherId app_launcher_id;
+  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);
   }
 };
 
 // Helper class to keep apps in order of appearance and to provide fast way
 // to check if app exists in the list.
 class AppTracker {
  public:
-  bool HasApp(const AppLauncherId& app_launcher_id) const {
-    return app_set_.find(app_launcher_id) != app_set_.end();
+  bool HasApp(const std::string& app_id) const {
+    return app_set_.find(app_id) != app_set_.end();
   }
 
-  void AddApp(const AppLauncherId& app_launcher_id) {
-    if (HasApp(app_launcher_id))
+  void AddApp(const std::string& app_id) {
+    if (HasApp(app_id))
       return;
-    app_list_.push_back(app_launcher_id);
-    app_set_.insert(app_launcher_id);
+    app_list_.push_back(app_id);
+    app_set_.insert(app_id);
   }
 
-  void MaybeAddApp(const AppLauncherId& app_launcher_id,
+  void MaybeAddApp(const std::string& app_id,
                    const LauncherControllerHelper* helper,
                    bool check_for_valid_app) {
-    DCHECK_NE(kPinnedAppsPlaceholder, app_launcher_id.ToString());
-    if (check_for_valid_app &&
-        !helper->IsValidIDForCurrentUser(app_launcher_id.ToString())) {
+    DCHECK_NE(kPinnedAppsPlaceholder, app_id);
+    if (check_for_valid_app && !helper->IsValidIDForCurrentUser(app_id)) {
       return;
     }
-    AddApp(app_launcher_id);
+    AddApp(app_id);
   }
 
   void MaybeAddAppFromPref(const base::DictionaryValue* app_pref,
                            const LauncherControllerHelper* helper,
                            bool check_for_valid_app) {
     std::string app_id;
     if (!app_pref->GetString(kPinnedAppsPrefAppIDPath, &app_id)) {
       LOG(ERROR) << "Cannot get app id from app pref entry.";
       return;
     }
 
     if (app_id == kPinnedAppsPlaceholder)
       return;
 
     bool pinned_by_policy = false;
     if (app_pref->GetBoolean(kPinnedAppsPrefPinnedByPolicy,
                              &pinned_by_policy) &&
         pinned_by_policy) {
       return;
     }
 
-    MaybeAddApp(AppLauncherId(app_id), helper, check_for_valid_app);
+    MaybeAddApp(app_id, helper, check_for_valid_app);
   }
 
-  const std::vector<AppLauncherId>& app_list() const { return app_list_; }
+  const std::vector<std::string>& app_list() const { return app_list_; }
 
  private:
-  std::vector<AppLauncherId> app_list_;
-  std::set<AppLauncherId> app_set_;
+  std::vector<std::string> app_list_;
+  std::set<std::string> app_set_;
 };
 
 }  // namespace
 
 const char kPinnedAppsPrefAppIDPath[] = "id";
 const char kPinnedAppsPrefPinnedByPolicy[] = "pinned_by_policy";
 const char kPinnedAppsPlaceholder[] = "AppShelfIDPlaceholder--------";
 
 const char kShelfAutoHideBehaviorAlways[] = "Always";
 const char kShelfAutoHideBehaviorNever[] = "Never";
 
 const char kShelfAlignmentBottom[] = "Bottom";
 const char kShelfAlignmentLeft[] = "Left";
 const char kShelfAlignmentRight[] = "Right";
 
-AppLauncherId::AppLauncherId(const std::string& app_id,
-                             const std::string& launch_id)
-    : app_id_(app_id), launch_id_(launch_id) {
-  DCHECK(!app_id.empty());
-}
-
-AppLauncherId::AppLauncherId(const std::string& app_id) : app_id_(app_id) {
-  DCHECK(!app_id.empty());
-}
-
-AppLauncherId::AppLauncherId() {}
-
-AppLauncherId::~AppLauncherId() {}
-
-std::string AppLauncherId::ToString() const {
-  return app_id_ + launch_id_;
-}
-
-bool AppLauncherId::operator<(const AppLauncherId& other) const {
-  return ToString() < other.ToString();
-}
-
 void RegisterChromeLauncherUserPrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   // TODO: If we want to support multiple profiles this will likely need to be
   // pushed to local state and we'll need to track profile per item.
   registry->RegisterIntegerPref(
       prefs::kShelfChromeIconIndex,
       0,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterListPref(prefs::kPinnedLauncherApps,
                              CreateDefaultPinnedAppsList(),
                              user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterListPref(prefs::kPolicyPinnedLauncherApps);
   registry->RegisterStringPref(prefs::kShelfAutoHideBehavior,
                                kShelfAutoHideBehaviorNever,
                                user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterStringPref(prefs::kShelfAutoHideBehaviorLocal,
                                std::string());
   registry->RegisterStringPref(prefs::kShelfAlignment,
                                kShelfAlignmentBottom,
                                user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterStringPref(prefs::kShelfAlignmentLocal, std::string());
   registry->RegisterDictionaryPref(prefs::kShelfPreferences);
   registry->RegisterIntegerPref(prefs::kLogoutDialogDurationMs, 20000);
   registry->RegisterBooleanPref(prefs::kShowLogoutButtonInTray, false);
 }
 
-std::unique_ptr<base::DictionaryValue> CreateAppDict(
-    const AppLauncherId& app_launcher_id) {
-  auto app_value = base::MakeUnique<base::DictionaryValue>();
-  app_value->SetString(kPinnedAppsPrefAppIDPath, app_launcher_id.ToString());
-  return app_value;
-}
-
 ShelfAutoHideBehavior GetShelfAutoHideBehaviorPref(PrefService* prefs,
                                                    int64_t display_id) {
   DCHECK_NE(display_id, display::kInvalidDisplayId);
 
   // Don't show the shelf in app mode.
   if (chrome::IsRunningInAppMode())
     return SHELF_AUTO_HIDE_ALWAYS_HIDDEN;
 
   // See comment in |kShelfAlignment| as to why we consider two prefs.
   return AutoHideBehaviorFromPref(
@@ skipping 74 matching lines @@
       if (!arc_app_list_pref)
         continue;
 
       // We are dealing with package name, not with 32 characters ID.
       const std::string& arc_package = app_id;
       const std::vector<std::string> activities = GetActivitiesForPackage(
           arc_package, all_arc_app_ids, *arc_app_list_pref);
       for (const auto& activity : activities) {
         const std::string arc_app_id =
             ArcAppListPrefs::GetAppId(arc_package, activity);
-        apps->MaybeAddApp(AppLauncherId(arc_app_id), helper,
-                          check_for_valid_app);
+        apps->MaybeAddApp(arc_app_id, helper, check_for_valid_app);
       }
     } else {
-      apps->MaybeAddApp(AppLauncherId(app_id), helper, check_for_valid_app);
+      apps->MaybeAddApp(app_id, helper, check_for_valid_app);
     }
   }
 }
 
-std::vector<AppLauncherId> GetPinnedAppsFromPrefsLegacy(
+std::vector<std::string> GetPinnedAppsFromPrefsLegacy(
     const PrefService* prefs,
     const LauncherControllerHelper* helper,
     bool check_for_valid_app) {
   // 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_apps = prefs->GetList(prefs::kPinnedLauncherApps);
 
   // 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_apps->GetSize(),
                           prefs->GetInteger(prefs::kShelfChromeIconIndex)));
 
   // Check if Chrome is in either of the the preferences lists.
   std::unique_ptr<base::Value> chrome_app(
-      CreateAppDict(AppLauncherId(extension_misc::kChromeAppId)));
+      CreateAppDict(extension_misc::kChromeAppId));
 
   AppTracker apps;
   GetAppsPinnedByPolicy(prefs, helper, check_for_valid_app, &apps);
 
   std::string app_id;
   for (size_t i = 0; i < pinned_apps->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)
-      apps.AddApp(AppLauncherId(extension_misc::kChromeAppId));
+      apps.AddApp(extension_misc::kChromeAppId);
     const base::DictionaryValue* app_pref = nullptr;
     if (!pinned_apps->GetDictionary(i, &app_pref)) {
       LOG(ERROR) << "There is no dictionary for app entry.";
       continue;
     }
     apps.MaybeAddAppFromPref(app_pref, helper, check_for_valid_app);
   }
 
   // If not added yet, the chrome item will be the last item in the list.
-  apps.AddApp(AppLauncherId(extension_misc::kChromeAppId));
+  apps.AddApp(extension_misc::kChromeAppId);
   return apps.app_list();
 }
 
 // static
 std::unique_ptr<ChromeLauncherPrefsObserver>
 ChromeLauncherPrefsObserver::CreateIfNecessary(Profile* profile) {
   sync_preferences::PrefServiceSyncable* prefs =
       PrefServiceSyncableFromProfile(profile);
   if (!prefs->FindPreference(prefs::kShelfAlignmentLocal)->HasUserSetting() ||
       !prefs->FindPreference(prefs::kShelfAutoHideBehaviorLocal)
@@ skipping 58 matching lines @@
     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<AppLauncherId> ImportLegacyPinnedApps(
+std::vector<std::string> ImportLegacyPinnedApps(
     const PrefService* prefs,
     LauncherControllerHelper* helper) {
-  std::vector<AppLauncherId> legacy_pins_all =
+  const std::vector<std::string> legacy_pins_all =
       GetPinnedAppsFromPrefsLegacy(prefs, helper, false);
   DCHECK(!legacy_pins_all.empty());
 
   app_list::AppListSyncableService* app_service =
       app_list::AppListSyncableServiceFactory::GetForProfile(helper->profile());
 
-  std::vector<AppLauncherId> legacy_pins_valid;
+  std::vector<std::string> legacy_pins_valid;
   syncer::StringOrdinal last_position =
       syncer::StringOrdinal::CreateInitialOrdinal();
   // Convert to sync item record.
-  for (const auto& app_launcher_id : legacy_pins_all) {
-    DCHECK_NE(kPinnedAppsPlaceholder, app_launcher_id.ToString());
-    app_service->SetPinPosition(app_launcher_id.ToString(), last_position);
+  for (const auto& app_id : legacy_pins_all) {
+    DCHECK_NE(kPinnedAppsPlaceholder, app_id);
+    app_service->SetPinPosition(app_id, last_position);
     last_position = last_position.CreateAfter();
-    if (helper->IsValidIDForCurrentUser(app_launcher_id.ToString()))
-      legacy_pins_valid.push_back(app_launcher_id);
+    if (helper->IsValidIDForCurrentUser(app_id))
+      legacy_pins_valid.push_back(app_id);
   }
 
   // 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))
@@ skipping 24 matching lines @@
     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;
     }
 
-    pin_infos.push_back(PinInfo(AppLauncherId(sync_peer.first),
-                                sync_peer.second->item_pin_ordinal));
+    pin_infos.push_back(
+        PinInfo(sync_peer.first, sync_peer.second->item_pin_ordinal));
   }
 
   if (first_run) {
     // Return default apps in case profile is not synced yet.
     sync_preferences::PrefServiceSyncable* const pref_service_syncable =
         PrefServiceSyncableFromProfile(helper->profile());
     if (!pref_service_syncable->IsSyncing())
-      return GetPinnedAppsFromPrefsLegacy(prefs, helper, true);
+      return AppIdsToAppLauncherIds(
+          GetPinnedAppsFromPrefsLegacy(prefs, helper, true));
 
     // We need to import legacy pins model and convert it to sync based
     // model.
-    return ImportLegacyPinnedApps(prefs, helper);
+    return AppIdsToAppLauncherIds(ImportLegacyPinnedApps(prefs, helper));
   }
 
   // Sort pins according their ordinals.
   std::sort(pin_infos.begin(), pin_infos.end(), ComparePinInfo());
 
   AppTracker policy_apps;
   GetAppsPinnedByPolicy(prefs, helper, true, &policy_apps);
 
   // Pinned by policy apps appear first, if they were not shown before.
   syncer::StringOrdinal front_position = GetFirstPinPosition(helper->profile());
-  std::vector<AppLauncherId>::const_reverse_iterator it;
+  std::vector<std::string>::const_reverse_iterator it;
   for (it = policy_apps.app_list().rbegin();
        it != policy_apps.app_list().rend(); ++it) {
-    const std::string& app_launcher_id_str = (*it).ToString();
-    if (app_launcher_id_str == kPinnedAppsPlaceholder)
+    const std::string& app_id = *it;
+    if (app_id == kPinnedAppsPlaceholder)
       continue;
 
     // Check if we already processed current app.
-    if (app_service->GetPinPosition(app_launcher_id_str).IsValid())
+    if (app_service->GetPinPosition(app_id).IsValid())
       continue;
 
     // Now move it to the front.
-    pin_infos.insert(pin_infos.begin(),
-                     PinInfo(AppLauncherId((*it).app_id(), (*it).launch_id()),
-                             front_position));
-    app_service->SetPinPosition(app_launcher_id_str, front_position);
+    pin_infos.insert(pin_infos.begin(), PinInfo(*it, front_position));
+    app_service->SetPinPosition(app_id, front_position);
     front_position = front_position.CreateBefore();
   }
 
   // Now insert Chrome browser app if needed.
   if (!app_service->GetPinPosition(extension_misc::kChromeAppId).IsValid()) {
-    pin_infos.insert(
-        pin_infos.begin(),
-        PinInfo(AppLauncherId(extension_misc::kChromeAppId), front_position));
+    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)) {
       const syncer::StringOrdinal arc_host_position =
           GetLastPinPosition(helper->profile());
       pin_infos.insert(pin_infos.begin(),
-                       PinInfo(AppLauncherId(ArcSupportHost::kHostAppId),
-                               arc_host_position));
+                       PinInfo(ArcSupportHost::kHostAppId, arc_host_position));
       app_service->SetPinPosition(ArcSupportHost::kHostAppId,
                                   arc_host_position);
     }
   }
 
   // Convert to AppLauncherId array.
-  std::vector<AppLauncherId> pins(pin_infos.size(), AppLauncherId());
+  std::vector<std::string> pins(pin_infos.size());
   for (size_t i = 0; i < pin_infos.size(); ++i)
-    pins[i] = pin_infos[i].app_launcher_id;
+    pins[i] = pin_infos[i].app_id;
 
-  return pins;
+  return AppIdsToAppLauncherIds(pins);
 }
 
 void RemovePinPosition(Profile* profile, const AppLauncherId& app_launcher_id) {
   DCHECK(profile);
+
+  const std::string& app_id = app_launcher_id.app_id();
+  if (!app_launcher_id.launch_id().empty()) {
+    VLOG(2) << "Unpinning app '" << app_id << "' with non-empty launch id '"

[stevenjb, 2017/02/08 20:32:37]

nit: Unpinning works, just syncing does not, maybe "Syncing remove pin for "

[khmel, 2017/02/08 21:20:39]

That is better, thanks!

+            << app_launcher_id.launch_id() << "' is not supported.";
+    return;
+  }
+  DCHECK(!app_id.empty());
+
   app_list::AppListSyncableService* app_service =
       app_list::AppListSyncableServiceFactory::GetForProfile(profile);
-  app_service->SetPinPosition(app_launcher_id.ToString(),
-                              syncer::StringOrdinal());
+  app_service->SetPinPosition(app_id, syncer::StringOrdinal());
 }
 
 void SetPinPosition(Profile* profile,
                     const AppLauncherId& app_launcher_id,
                     const AppLauncherId& app_launcher_id_before,
                     const std::vector<AppLauncherId>& app_launcher_ids_after) {
   DCHECK(profile);
-  const std::string app_launcher_id_str = app_launcher_id.ToString();
-  const std::string app_launcher_id_before_str =
-      app_launcher_id_before.ToString();
-  DCHECK_NE(app_launcher_id_str, app_launcher_id_before_str);
+
+  const std::string& app_id = app_launcher_id.app_id();
+  if (!app_launcher_id.launch_id().empty()) {
+    VLOG(2) << "Pinning app '" << app_id << "' with non-empty launch id '"

[stevenjb, 2017/02/08 20:32:37]

ditto

[khmel, 2017/02/08 21:20:40]

Done.

+            << app_launcher_id.launch_id() << "' is not supported.";
+    return;
+  }
+
+  const std::string& app_id_before = app_launcher_id_before.app_id();
+
+  DCHECK(!app_id.empty());
+  DCHECK_NE(app_id, app_id_before);
 
   app_list::AppListSyncableService* app_service =
       app_list::AppListSyncableServiceFactory::GetForProfile(profile);
   // Some unit tests may not have this service.
   if (!app_service)
     return;
 
   syncer::StringOrdinal position_before =
-      app_launcher_id_before_str.empty()
-          ? syncer::StringOrdinal()
-          : app_service->GetPinPosition(app_launcher_id_before_str);
+      app_id_before.empty() ? syncer::StringOrdinal()
+                            : app_service->GetPinPosition(app_id_before);
   syncer::StringOrdinal position_after;
-  for (const auto& app_launcher_id_after : app_launcher_ids_after) {
-    const std::string app_launcher_id_after_str =
-        app_launcher_id_after.ToString();
-    DCHECK_NE(app_launcher_id_after_str, app_launcher_id_str);
-    DCHECK_NE(app_launcher_id_after_str, app_launcher_id_before_str);
-    syncer::StringOrdinal position =
-        app_service->GetPinPosition(app_launcher_id_after_str);
+  for (const auto& app_launher_id_after : app_launcher_ids_after) {

[stevenjb, 2017/02/08 20:32:37]

app_launcher_id_after

[khmel, 2017/02/08 21:20:39]

Done.

+    const std::string& app_id_after = app_launher_id_after.app_id();
+    DCHECK_NE(app_id_after, app_id);
+    DCHECK_NE(app_id_after, app_id_before);
+    syncer::StringOrdinal position = app_service->GetPinPosition(app_id_after);
     DCHECK(position.IsValid());
     if (!position.IsValid()) {
-      LOG(ERROR) << "Sync pin position was not found for "
-                 << app_launcher_id_after_str;
+      LOG(ERROR) << "Sync pin position was not found for " << app_id_after;
       continue;
     }
     if (!position_before.IsValid() || !position.Equals(position_before)) {
       position_after = position;
       break;
     }
   }
 
   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_launcher_id_str, pin_position);
+  app_service->SetPinPosition(app_id, pin_position);
 }
 
 }  // namespace launcher
 }  // namespace ash