Convert app_launch_index and page_index from int to StringOrdinal.

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

 // Copyright (c) 2011 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/webui/ntp/app_launcher_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/auto_reset.h"
@@ skipping 142 matching lines @@
                            ExtensionPrefs::LAUNCH_DEFAULT));
   value->SetBoolean("offline_enabled", extension->offline_enabled());
   value->SetBoolean("is_component",
       extension->location() == Extension::COMPONENT);
   value->SetBoolean("is_webstore",
       extension->id() == extension_misc::kWebStoreAppId);
 
   if (notification)
     value->Set("notification", SerializeNotification(*notification));
 
-  int app_launch_index = prefs->GetAppLaunchIndex(extension->id());
-  if (app_launch_index == -1) {
+  StringOrdinal page_index = prefs->GetPageIndex(extension->id());
+  if (!page_index.IsValid()) {
+    // Make sure every app has a page index (some predate the page index).
+    // The webstore app should be on the first page.
+    page_index = extension->id() == extension_misc::kWebStoreAppId ?
+        prefs->GetFirstAppPage() : prefs->GetNaturalAppPageIndex();
+    prefs->SetPageIndex(extension->id(), page_index);
+  }
+  // We convert the page_index 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->PageIndexAsInteger(page_index));
+
+  StringOrdinal app_launch_index = prefs->GetAppLaunchIndex(extension->id());
+  if (!app_launch_index.IsValid()) {
     // 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->CreateFirstAppLaunchIndex(page_index)
+        : prefs->GetNextAppLaunchIndex(page_index);
     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).
-    // 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);
-  }
-  value->SetInteger("page_index", page_index);
+  value->SetString("app_launch_index", app_launch_index.ToString());
 }
 
 WebUIMessageHandler* AppLauncherHandler::Attach(WebUI* web_ui) {
   registrar_.Add(this, chrome::NOTIFICATION_APP_INSTALLED_TO_NTP,
       content::Source<TabContents>(web_ui->tab_contents()));
   return WebUIMessageHandler::Attach(web_ui);
 }
 
 void AppLauncherHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback("getApps",
@@ skipping 151 matching lines @@
   AutoReset<bool> auto_reset(&ignore_changes_, true);
 
   ListValue* list = new ListValue();
   const ExtensionList* extensions = extension_service_->extensions();
   ExtensionList::const_iterator it;
   for (it = extensions->begin(); it != extensions->end(); ++it) {
     if (!IsAppExcludedFromList(*it)) {
       DictionaryValue* app_info = GetAppInfo(*it);
       list->Append(app_info);
     } else {
       // This is necessary because in some previous versions of chrome, we set a

[csharp, 2011/11/18 15:57:31]

I'd like to move this else into the migration code, but I can't seem to find
anyway in ExtensionPrefs to figure out which extension should have ordinal
values. Any advice?

[Finnur, 2011/11/21 12:16:14]

Not sure who this question is addressed to, but my answer would be that all apps
that appear on the NTP should have ordinal values. 

There are functions on the Extension object to determine whether an extension is
an app, but I don't see why you need to make a determination. If any extension
has an invalid page index, wouldn't you want to clear it? Or do we still use the
page index for non-app extensions?

On 2011/11/18 15:57:31, csharp wrote:

[csharp, 2011/11/21 15:14:32]

The problem is that some extensions (which don't appear on the NTP) have valid
indices, but they shouldn't. 
Also, I don't think any of the functions to determine if a extension is app can
be called from the migration code, which led me to this question.

On 2011/11/21 12:16:14, Finnur wrote:

[Finnur, 2011/11/21 15:23:20]

Um... that was my point. Should any extension (app or not) have an invalid
PageIndex? Why is it not ok to clear any invalid PageIndices?

In any case, I'm not sure you need to get hung up on this for this changelist,
it seems like a nice to move to a better location but not crucial and could be
done separately...

       // 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
       ExtensionPrefs* prefs = extension_service_->extension_prefs();
-      if (prefs->GetPageIndex((*it)->id()) != -1)
+      if (prefs->GetPageIndex((*it)->id()).IsValid())
         prefs->ClearPageIndex((*it)->id());
     }
   }
 
   extensions = extension_service_->disabled_extensions();
   for (it = extensions->begin(); it != extensions->end(); ++it) {
     if (!IsAppExcludedFromList(*it)) {
       DictionaryValue* app_info = new DictionaryValue();
       CreateAppInfo(*it,
                     NULL,
@@ skipping 31 matching lines @@
   dictionary->SetBoolean("disableCreateAppShortcut", true);
 #endif
 
   dictionary->SetBoolean(
       "showLauncher",
       extension_service_->apps_promo()->ShouldShowAppLauncher(
           extension_service_->GetAppIds()));
 
   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();
+      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()));
+                      static_cast<DictionaryValue*>(
+                          names_dictionary->DeepCopy()));
     } else {
       dictionary->Set("appPageNames",
-                      static_cast<ListValue*>(app_page_names->DeepCopy()));
+                      static_cast<DictionaryValue*>(
+                          app_page_names->DeepCopy()));
     }
   }
 }
 
 DictionaryValue* AppLauncherHandler::GetAppInfo(const Extension* extension) {
   AppNotificationManager* notification_manager =
       extension_service_->app_notification_manager();
   DictionaryValue* app_info = new DictionaryValue();
   // CreateAppInfo can change the extension prefs.
   AutoReset<bool> auto_reset(&ignore_changes_, true);
@@ skipping 265 matching lines @@
     if (app_order->GetString(i, &value))
       extension_ids.push_back(value);
   }
 
   // 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()->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) {
   std::string extension_id;
-  double page_index;
+  double page_index_raw;
   CHECK(args->GetString(0, &extension_id));
-  CHECK(args->GetDouble(1, &page_index));
+  CHECK(args->GetDouble(1, &page_index_raw));
+  StringOrdinal page_index =
+      extension_service_->extension_prefs()->PageIndexAsStringOrdinal(
+          static_cast<size_t>(page_index_raw));
 
   // 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));
+                                                      page_index);
 }
 
 void AppLauncherHandler::HandlePromoSeen(const ListValue* args) {
   UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppsPromoHistogram,
                             extension_misc::PROMO_SEEN,
                             extension_misc::PROMO_BUCKET_BOUNDARY);
 }
 
 void AppLauncherHandler::HandleSaveAppPageName(const ListValue* args) {
   string16 name;
   CHECK(args->GetString(0, &name));
 
   double page_index;
   CHECK(args->GetDouble(1, &page_index));
+  std::string page_index_str = base::IntToString(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_index_str, Value::CreateStringValue(name));
 }
 
 void AppLauncherHandler::HandleGenerateAppForLink(const ListValue* args) {
   std::string url;
   CHECK(args->GetString(0, &url));
   GURL launch_url(url);
 
   string16 title;
   CHECK(args->GetString(1, &title));
 
   double page_index;
   CHECK(args->GetDouble(2, &page_index));
+  StringOrdinal page_index_str =
+      extension_service_->extension_prefs()->PageIndexAsStringOrdinal(
+          static_cast<int>(page_index));
 
   Profile* profile = Profile::FromWebUI(web_ui_);
   FaviconService* favicon_service =
       profile->GetFaviconService(Profile::EXPLICIT_ACCESS);
   if (!favicon_service) {
     LOG(ERROR) << "No favicon service";
     return;
   }
 
   scoped_ptr<AppInstallInfo> install_info(new AppInstallInfo());
   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_index = page_index_str;
 
   FaviconService::Handle h = favicon_service->GetFaviconForURL(
       launch_url, history::FAVICON, &favicon_consumer_,
       base::Bind(&AppLauncherHandler::OnFaviconForApp, base::Unretained(this)));
   favicon_consumer_.SetClientData(favicon_service, h, install_info.release());
 }
 
 void AppLauncherHandler::HandleRecordAppLaunchByURL(
     const base::ListValue* args) {
   std::string url;
@@ skipping 56 matching lines @@
     return;
 
   StringValue app_id(highlight_app_id_);
   web_ui_->CallJavascriptFunction("ntp4.setAppToBeHighlighted", app_id);
   highlight_app_id_.clear();
 }
 
 // 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,
                             extension_misc::APP_LAUNCH_BUCKET_BOUNDARY);
 
   if (!promo_active) return;
 
   UMA_HISTOGRAM_ENUMERATION(extension_misc::kAppsPromoHistogram,
                             extension_misc::PROMO_LAUNCH_WEB_STORE,
                             extension_misc::PROMO_BUCKET_BOUNDARY);
@@ skipping 134 matching lines @@
 
 void AppLauncherHandler::UninstallDefaultApps() {
   AppsPromo* apps_promo = extension_service_->apps_promo();
   const ExtensionIdSet& app_ids = apps_promo->old_default_apps();
   for (ExtensionIdSet::const_iterator iter = app_ids.begin();
        iter != app_ids.end(); ++iter) {
     if (extension_service_->GetExtensionById(*iter, true))
       extension_service_->UninstallExtension(*iter, false, NULL);
   }
 }