Convert app_launch_index and page_index from int to StringOrdinal.

chrome/browser/ui/webui/ntp/app_launcher_handler.cc

Index: chrome/browser/ui/webui/ntp/app_launcher_handler.cc
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
index 71d2ca82a3aa4568027c3f88a285b9370c94c056..7aa0f21a38c125e2e136c669614134698c18b127 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
@@ -160,27 +160,32 @@ void AppLauncherHandler::CreateAppInfo(const Extension* extension,
   if (notification)
     value->Set("notification", SerializeNotification(*notification));
 
-  int app_launch_index = prefs->GetAppLaunchIndex(extension->id());
-  if (app_launch_index == -1) {
-    // Make sure every app has a launch index (some predate the launch index).
-    // The webstore's app launch index is set to -2 to make sure it's first.
-    // The next time the user drags (any) app this will be set to something
-    // sane (i.e. >= 0).
-    app_launch_index = extension->id() == extension_misc::kWebStoreAppId ?
-        -2 : prefs->GetNextAppLaunchIndex(0);
-    prefs->SetAppLaunchIndex(extension->id(), app_launch_index);
-  }
-  value->SetInteger("app_launch_index", app_launch_index);
-
-  int page_index = prefs->GetPageIndex(extension->id());
-  if (page_index < 0) {
-    // Make sure every app has a page index (some predate the page index).
+  StringOrdinal page_ordinal = prefs->GetPageOrdinal(extension->id());
+  if (!page_ordinal.IsValid()) {
+    // Make sure every app has a page ordinal (some predate the page ordinal).
     // The webstore app should be on the first page.
-    page_index = extension->id() == extension_misc::kWebStoreAppId ?
-        0 : prefs->GetNaturalAppPageIndex();
-    prefs->SetPageIndex(extension->id(), page_index);
+    page_ordinal = extension->id() == extension_misc::kWebStoreAppId ?
+        prefs->GetFirstAppPage() : prefs->GetNaturalAppPageOrdinal();
+    prefs->SetPageOrdinal(extension->id(), page_ordinal);
   }
-  value->SetInteger("page_index", page_index);
+  // We convert the page_ordinal to an integer becuase the pages are referenced
+  // from within an array in the javascript code, which can't be easily
+  // changed to handle the StringOrdinal values, so we do the conversion here.
+  value->SetInteger("page_index",
+                    prefs->PageStringOrdinalAsInteger(page_ordinal));
+
+  StringOrdinal app_launch_ordinal =
+      prefs->GetAppLaunchOrdinal(extension->id());
+  if (!app_launch_ordinal.IsValid()) {
+    // Make sure every app has a launch ordinal (some predate the launch
+    // ordinal). The webstore's app launch ordinal is always set to the first
+    // position.
+    app_launch_ordinal = extension->id() == extension_misc::kWebStoreAppId ?
+        prefs->CreateFirstAppLaunchOrdinal(page_ordinal) :
+        prefs->GetNextAppLaunchOrdinal(page_ordinal);
+    prefs->SetAppLaunchOrdinal(extension->id(), app_launch_ordinal);
+  }
+  value->SetString("app_launch_ordinal", app_launch_ordinal.ToString());
 }
 
 WebUIMessageHandler* AppLauncherHandler::Attach(WebUI* web_ui) {
@@ -341,7 +346,7 @@ void AppLauncherHandler::Observe(int type,
 }
 
 void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
-  // CreateAppInfo and ClearPageIndex can change the extension prefs.
+  // CreateAppInfo and ClearPageOrdinal can change the extension prefs.
   AutoReset<bool> auto_reset(&ignore_changes_, true);
 
   ListValue* list = new ListValue();
@@ -354,11 +359,11 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
     } else {
       // This is necessary because in some previous versions of chrome, we set a
       // page index for non-app extensions. Old profiles can persist this error,
-      // and this fixes it. If we don't fix it, GetNaturalAppPageIndex() doesn't
-      // work. See http://crbug.com/98325
+      // and this fixes it. This caused GetNaturalAppPageIndex() to break
+      // (see http://crbug.com/98325) before it was an ordinal value.
       ExtensionPrefs* prefs = extension_service_->extension_prefs();
-      if (prefs->GetPageIndex((*it)->id()) != -1)
-        prefs->ClearPageIndex((*it)->id());
+      if (prefs->GetPageOrdinal((*it)->id()).IsValid())
+        prefs->ClearPageOrdinal((*it)->id());
     }
   }
 
@@ -409,18 +414,25 @@ void AppLauncherHandler::FillAppDictionary(DictionaryValue* dictionary) {
 
   if (NewTabUI::NTP4Enabled()) {
     PrefService* prefs = Profile::FromWebUI(web_ui_)->GetPrefs();
-    const ListValue* app_page_names = prefs->GetList(prefs::kNTPAppPageNames);
-    if (!app_page_names || !app_page_names->GetSize()) {
-      ListPrefUpdate update(prefs, prefs::kNTPAppPageNames);
-      ListValue* list = update.Get();
-      list->Set(0, Value::CreateStringValue(
+    const DictionaryValue* app_page_names =
+        prefs->GetDictionary(prefs::kNTPAppPageNames);
+    if (!app_page_names || !app_page_names->size()) {
+      DictionaryPrefUpdate update(prefs, prefs::kNTPAppPageNames);
+      DictionaryValue* names_dictionary = update.Get();
+      const StringOrdinal& new_page = StringOrdinal::CreateInitialOrdinal();
+      names_dictionary->Set(new_page.ToString(), Value::CreateStringValue(
           l10n_util::GetStringUTF16(IDS_APP_DEFAULT_PAGE_NAME)));
-      dictionary->Set("appPageNames",
-                      static_cast<ListValue*>(list->DeepCopy()));
-    } else {
-      dictionary->Set("appPageNames",
-                      static_cast<ListValue*>(app_page_names->DeepCopy()));
+      app_page_names = names_dictionary;
     }
+
+    // Convert the app page names from a dictionary to a list because the
+    // javascript code requires this data in a list.
+    ListValue* list = new ListValue;
+    for (DictionaryValue::Iterator it(*app_page_names);
+         it.HasNext(); it.Advance()) {
+      list->Append(it.value().DeepCopy());
+    }
+    dictionary->Set("appPageNames", list);

[csharp, 2011/11/23 15:38:07]

This seems to be the best way I could find to convert the keys of the dictionary
to a list, but please point me to a better method if one exists.

[Evan Stade, 2011/11/23 21:54:51]

this seems fine, my original comment was concerned with whether changing from a
list to a dictionary pref type would work correctly (can list json be read into
a dictionary? what would the keys be?)

[csharp, 2011/12/07 15:00:59]

I'm not exactly sure where the json and c++ code meet, so I'm not sure what the
correct answer is (I had luck figuring it out when I looked into it). I don't
think that they can be read in, but I could be wrong. In our code that isn't a
problem (at least for the appPageNames) because we are never returning a whole
list from json, it only passes back the name changes one at a time, so this
issue don't occur.

Sorry I don't have a better answer.

On 2011/11/23 21:54:51, Evan Stade wrote:

   }
 }
 
@@ -705,7 +717,8 @@ void AppLauncherHandler::HandleReorderApps(const ListValue* args) {
     auto_reset.reset(new AutoReset<bool>(&ignore_changes_, true));
 
   extension_service_->extension_prefs()->SetAppDraggedByUser(dragged_app_id);
-  extension_service_->extension_prefs()->SetAppLauncherOrder(extension_ids);
+  extension_service_->extension_prefs()->SetAppLauncherOrder(extension_ids,
+                                                             dragged_app_id);
 }
 
 void AppLauncherHandler::HandleSetPageIndex(const ListValue* args) {
@@ -713,14 +726,17 @@ void AppLauncherHandler::HandleSetPageIndex(const ListValue* args) {
   double page_index;
   CHECK(args->GetString(0, &extension_id));
   CHECK(args->GetDouble(1, &page_index));
+  const StringOrdinal& page_ordinal =
+      extension_service_->extension_prefs()->PageIntegerAsStringOrdinal(
+          static_cast<size_t>(page_index));
 
   // Don't update the page; it already knows the apps have been reordered.
   scoped_ptr<AutoReset<bool> > auto_reset;
   if (NewTabUI::NTP4Enabled())
     auto_reset.reset(new AutoReset<bool>(&ignore_changes_, true));
 
-  extension_service_->extension_prefs()->SetPageIndex(extension_id,
-      static_cast<int>(page_index));
+  extension_service_->extension_prefs()->SetPageOrdinal(extension_id,
+                                                        page_ordinal);
 }
 
 void AppLauncherHandler::HandlePromoSeen(const ListValue* args) {
@@ -735,12 +751,15 @@ void AppLauncherHandler::HandleSaveAppPageName(const ListValue* args) {
 
   double page_index;
   CHECK(args->GetDouble(1, &page_index));
+  const StringOrdinal& page_ordinal =
+      extension_service_->extension_prefs()->PageIntegerAsStringOrdinal(
+          static_cast<int>(page_index));
 
   AutoReset<bool> auto_reset(&ignore_changes_, true);
   PrefService* prefs = Profile::FromWebUI(web_ui_)->GetPrefs();
-  ListPrefUpdate update(prefs, prefs::kNTPAppPageNames);
-  ListValue* list = update.Get();
-  list->Set(static_cast<size_t>(page_index), Value::CreateStringValue(name));
+  DictionaryPrefUpdate update(prefs, prefs::kNTPAppPageNames);
+  DictionaryValue* dictionary = update.Get();
+  dictionary->Set(page_ordinal.ToString(), Value::CreateStringValue(name));
 }
 
 void AppLauncherHandler::HandleGenerateAppForLink(const ListValue* args) {
@@ -753,6 +772,9 @@ void AppLauncherHandler::HandleGenerateAppForLink(const ListValue* args) {
 
   double page_index;
   CHECK(args->GetDouble(2, &page_index));
+  const StringOrdinal& page_ordinal =
+      extension_service_->extension_prefs()->PageIntegerAsStringOrdinal(
+          static_cast<int>(page_index));
 
   Profile* profile = Profile::FromWebUI(web_ui_);
   FaviconService* favicon_service =
@@ -766,7 +788,7 @@ void AppLauncherHandler::HandleGenerateAppForLink(const ListValue* args) {
   install_info->is_bookmark_app = true;
   install_info->title = title;
   install_info->app_url = launch_url;
-  install_info->page_index = static_cast<int>(page_index);
+  install_info->page_ordinal = page_ordinal;
 
   FaviconService::Handle h = favicon_service->GetFaviconForURL(
       launch_url, history::FAVICON, &favicon_consumer_,
@@ -826,7 +848,7 @@ void AppLauncherHandler::OnFaviconForApp(FaviconService::Handle handle,
 
   scoped_refptr<CrxInstaller> installer(
       CrxInstaller::Create(extension_service_, NULL));
-  installer->set_page_index(install_info->page_index);
+  installer->set_page_ordinal(install_info->page_ordinal);
   installer->InstallWebApp(*web_app);
   attempted_bookmark_app_install_ = true;
 }
@@ -843,11 +865,11 @@ void AppLauncherHandler::SetAppToBeHighlighted() {
 // static
 void AppLauncherHandler::RegisterUserPrefs(PrefService* pref_service) {
   // TODO(csilv): We will want this to be a syncable preference instead.
-  pref_service->RegisterListPref(prefs::kNTPAppPageNames,
-                                 PrefService::UNSYNCABLE_PREF);
+  pref_service->RegisterDictionaryPref(prefs::kNTPAppPageNames,
+                                       PrefService::UNSYNCABLE_PREF);
 }
 
-// statiic
+// static
 void AppLauncherHandler::RecordWebStoreLaunch(bool promo_active) {
   UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppLaunchHistogram,
                             extension_misc::APP_LAUNCH_NTP_WEBSTORE,