Convert app_launch_index and page_index from int to StringOrdinal.

chrome/browser/extensions/extension_prefs.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/extensions/extension_prefs.h"
 
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_pref_store.h"
@@ skipping 92 matching lines @@
 
 // A preference set by the web store to indicate login information for
 // purchased apps.
 const char kWebStoreLogin[] = "extensions.webstore_login";
 
 // A preference set by the the NTP to persist the desired launch container type
 // used for apps.
 const char kPrefLaunchType[] = "launchType";
 
 // A preference determining the order of which the apps appear on the NTP.
-const char kPrefAppLaunchIndex[] = "app_launcher_index";
+const char kPrefAppLaunchIndexDeprecated[] = "app_launcher_index";
+const char kPrefAppLaunchOrdinal[] = "app_launcher_ordinal";
 
 // A preference determining the page on which an app appears in the NTP.
-const char kPrefPageIndex[] = "page_index";
+const char kPrefPageIndexDeprecated[] = "page_index";
+const char kPrefPageOrdinal[] = "page_ordinal";
 
 // A preference specifying if the user dragged the app on the NTP.
 const char kPrefUserDraggedApp[] = "user_dragged_app_ntp";
 
 // A preference for storing extra data sent in update checks for an extension.
 const char kUpdateUrlData[] = "update_url_data";
 
 // Whether the browser action is visible in the toolbar.
 const char kBrowserActionVisible[] = "browser_action_visible";
 
@@ skipping 105 matching lines @@
   const std::string extension_id_;
   const std::string incognito_or_regular_path_;
 
   DISALLOW_COPY_AND_ASSIGN(ScopedExtensionControlledPrefUpdate);
 };
 
 std::string JoinPrefs(std::string parent, const char* child) {
   return parent + "." + child;
 }
 
+// A simple structure used when sorting applications based on their app launch
+// index. This is used by the code that migrates the data from integers to
+// StringOrdinals.
+struct ApplicationNTPOldAppLaunch {
+  const std::string* application_id;
+  const int app_launch_index;
+
+  ApplicationNTPOldAppLaunch() : application_id(0),
+                                 app_launch_index(0) {}
+
+  ApplicationNTPOldAppLaunch(const std::string& app_id, int app_launch)
+      : application_id(&app_id),
+        app_launch_index(app_launch) {}
+};
+
+struct SortByNTPOldAppLaunch {
+  bool operator() (const ApplicationNTPOldAppLaunch& lhs,
+                   const ApplicationNTPOldAppLaunch& rhs) const {
+    return lhs.app_launch_index < rhs.app_launch_index;
+  }
+};
+
 }  // namespace
 
 ExtensionPrefs::ExtensionPrefs(
     PrefService* prefs,
     const FilePath& root_dir,
     ExtensionPrefValueMap* extension_pref_value_map)
     : prefs_(prefs),
       install_directory_(root_dir),
       extension_pref_value_map_(extension_pref_value_map),
       content_settings_store_(new ExtensionContentSettingsStore()) {
@@ skipping 156 matching lines @@
   const DictionaryValue* ext = GetExtensionPref(extension_id);
   ListValue* out = NULL;
   if (!ext || !ext->GetList(pref_key, &out))
     return false;
   if (out_value)
     *out_value = out;
 
   return true;
 }
 
+bool ExtensionPrefs::ReadExtensionPrefString(
+    const std::string& extension_id, const std::string& pref_key,
+    std::string* out_value) const {
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
+
+  if (!ext || !ext->GetString(pref_key, out_value))
+    return false;
+
+  return true;
+}
+
 bool ExtensionPrefs::ReadExtensionPrefURLPatternSet(
     const std::string& extension_id,
     const std::string& pref_key,
     URLPatternSet* result,
     int valid_schemes) {
   const ListValue* value = NULL;
   if (!ReadExtensionPrefList(extension_id, pref_key, &value))
     return false;
 
   result->ClearPatterns();
@@ skipping 423 matching lines @@
     if (ext->GetList(kPrefOldGrantedHosts, &hosts)) {
       UpdateExtensionPref(
           *ext_id, explicit_hosts, hosts->DeepCopy());
 
       // We can get rid of the old one by setting it to an empty list.
       UpdateExtensionPref(*ext_id, kPrefOldGrantedHosts, new ListValue());
     }
   }
 }
 
+void ExtensionPrefs::MigrateAppIndex(const ExtensionIdSet& extension_ids) {
+  if (extension_ids.empty())
+    return;
+
+  // Convert all the page index values to page ordinals. If there are any
+  // app launch values that need to be migrated, inserted them into a sorted
+  // set to be dealt with later.
+  std::multiset<ApplicationNTPOldAppLaunch, SortByNTPOldAppLaunch>

[akalin, 2011/11/29 18:39:09]

Any particular reason you're using a multiset?  If you use a multimap from int
-> const string* you can avoid the extra struct.

Also, any particular reason for the 'multi'?  Is it expected that apps would
have the same launch index?  Are you just being defensive?  If so, that's okay.

[csharp, 2011/11/29 21:21:40]

Changed to multimap (didn't think about doing this when I remove the page value
from the struct).

Multi is needed because all the pages have their apps placed in this one map and
each page can (and usually has) an app with app_launch_index 0.

On 2011/11/29 18:39:09, akalin wrote:

+      app_launches_to_convert;
+  for (ExtensionIdSet::const_iterator ext_id = extension_ids.begin();
+       ext_id != extension_ids.end(); ++ext_id) {
+    int old_page_index = 0;
+    if (ReadExtensionPrefInteger(*ext_id,
+                                 kPrefPageIndexDeprecated,
+                                 &old_page_index)) {
+      SetPageOrdinal(*ext_id, PageIntegerAsStringOrdinal(old_page_index));
+      UpdateExtensionPref(*ext_id, kPrefPageIndexDeprecated, NULL);
+    }
+
+    int old_app_launch_index = 0;
+    if (ReadExtensionPrefInteger(*ext_id,
+                                 kPrefAppLaunchIndexDeprecated,
+                                 &old_app_launch_index)) {
+      // We can't update the app launch index value yet, because we use
+      // GetNextAppLaunchOrdinal to get the new ordinal value and it requires
+      // all the ordinals with lower values to have already been migrated.
+      app_launches_to_convert.insert(ApplicationNTPOldAppLaunch(
+          *ext_id,
+          old_app_launch_index));
+      UpdateExtensionPref(*ext_id, kPrefAppLaunchIndexDeprecated, NULL);
+    }
+  }
+
+  if (app_launches_to_convert.empty())
+    return;
+
+  // Create the new app launch ordinals and remove the old preferences. Since
+  // the set is sorted, each time we migrate an apps index, we know that all of
+  // the remaining apps will appear further down the NTP than it or on a
+  // different page.
+  for (std::multiset<ApplicationNTPOldAppLaunch, SortByNTPOldAppLaunch>
+           ::const_iterator ext = app_launches_to_convert.begin();
+       ext != app_launches_to_convert.end(); ++ext) {
+    StringOrdinal page = GetPageOrdinal(*(ext->application_id));
+    if (!page.IsValid()) {
+      page = GetNaturalAppPageOrdinal();
+      SetPageOrdinal(*(ext->application_id), page);
+    }
+
+    SetAppLaunchOrdinal(*(ext->application_id),
+                          GetNextAppLaunchOrdinal(page));
+  }
+}
+
 ExtensionPermissionSet* ExtensionPrefs::GetGrantedPermissions(
     const std::string& extension_id) {
   CHECK(Extension::IdIsValid(extension_id));
   return ReadExtensionPrefPermissionSet(extension_id, kPrefGrantedPermissions);
 }
 
 void ExtensionPrefs::AddGrantedPermissions(
     const std::string& extension_id,
     const ExtensionPermissionSet* permissions) {
   CHECK(Extension::IdIsValid(extension_id));
@@ skipping 190 matching lines @@
   ListPrefUpdate update(prefs_, kExtensionToolbar);
   ListValue* toolbar_order = update.Get();
   toolbar_order->Clear();
   for (std::vector<std::string>::const_iterator iter = extension_ids.begin();
        iter != extension_ids.end(); ++iter) {
     toolbar_order->Append(new StringValue(*iter));
   }
   SavePrefs();
 }
 
+void ExtensionPrefs::SetAppOrdinalValues(DictionaryValue* extension_dict,

[akalin, 2011/11/29 18:39:09]

this is called in essentially one place, right?  I think it's better to inline
it, since the name is kind of generic

[csharp, 2011/11/29 21:21:40]

Done.

+                                         const StringOrdinal& page_ordinal) {
+  extension_dict->Set(kPrefPageOrdinal,

[akalin, 2011/11/29 18:39:09]

doesn't this overlap with SetPageOrdinal?

[csharp, 2011/11/29 21:21:40]

It does, changing to just call those functions.

On 2011/11/29 18:39:09, akalin wrote:

+                      Value::CreateStringValue(page_ordinal.ToString()));
+  UpdatePageOrdinalMap(StringOrdinal(), page_ordinal);
+
+  extension_dict->Set(kPrefAppLaunchOrdinal,
+                      Value::CreateStringValue(
+                          GetNextAppLaunchOrdinal(page_ordinal).ToString()));
+}
+
 void ExtensionPrefs::OnExtensionInstalled(
     const Extension* extension,
     Extension::State initial_state,
     bool from_webstore,
-    int page_index) {
+    const StringOrdinal& page_ordinal) {
   const std::string& id = extension->id();
   CHECK(Extension::IdIsValid(id));
   ScopedExtensionPrefUpdate update(prefs_, id);
   DictionaryValue* extension_dict = update.Get();
   const base::Time install_time = GetCurrentTime();
   extension_dict->Set(kPrefState, Value::CreateIntegerValue(initial_state));
   extension_dict->Set(kPrefLocation,
                       Value::CreateIntegerValue(extension->location()));
   extension_dict->Set(kPrefFromWebStore,
                       Value::CreateBooleanValue(from_webstore));
@@ skipping 11 matching lines @@
       extension->path());
   extension_dict->Set(kPrefPath, Value::CreateStringValue(path));
   // We store prefs about LOAD extensions, but don't cache their manifest
   // since it may change on disk.
   if (extension->location() != Extension::LOAD) {
     extension_dict->Set(kPrefManifest,
                         extension->manifest_value()->DeepCopy());
   }
 
   if (extension->is_app()) {
-    if (page_index == -1)
-      page_index = GetNaturalAppPageIndex();
-    extension_dict->Set(kPrefPageIndex,
-                        Value::CreateIntegerValue(page_index));
-    extension_dict->Set(kPrefAppLaunchIndex,
-        Value::CreateIntegerValue(GetNextAppLaunchIndex(page_index)));
+    if (page_ordinal.IsValid())
+      SetAppOrdinalValues(extension_dict, page_ordinal);
+    else
+      SetAppOrdinalValues(extension_dict, GetNaturalAppPageOrdinal());
   }
+
   extension_pref_value_map_->RegisterExtension(
       id, install_time, initial_state == Extension::ENABLED);
   content_settings_store_->RegisterExtension(
       id, install_time, initial_state == Extension::ENABLED);
 }
 
 void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
                                             const Extension::Location& location,
                                             bool external_uninstall) {
   // For external extensions, we save a preference reminding ourself not to try
@@ skipping 311 matching lines @@
     return true;
   }
   return false;
 }
 
 void ExtensionPrefs::SetWebStoreLogin(const std::string& login) {
   prefs_->SetString(kWebStoreLogin, login);
   SavePrefs();
 }
 
-int ExtensionPrefs::GetAppLaunchIndex(const std::string& extension_id) {
-  int value;
-  if (ReadExtensionPrefInteger(extension_id, kPrefAppLaunchIndex, &value))
-    return value;
+bool ExtensionPrefs::GetMaxAndMinAppLaunchOrdinalsOnPage(
+    const StringOrdinal& target_page_ordinal,
+    StringOrdinal* min_app_launch_value,
+    StringOrdinal* max_app_launch_value) const {
+  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  CHECK(extensions);
+  CHECK(target_page_ordinal.IsValid());
+  CHECK(min_app_launch_value);
+  CHECK(max_app_launch_value);
 
-  return -1;
+  StringOrdinal* min_value = NULL;
+  StringOrdinal* max_value = NULL;
+  for (DictionaryValue::key_iterator ext_it = extensions->begin_keys();
+       ext_it != extensions->end_keys(); ++ext_it) {
+    StringOrdinal page_ordinal = GetPageOrdinal(*ext_it);
+    if (page_ordinal.IsValid() && page_ordinal.Equal(target_page_ordinal)) {
+      StringOrdinal app_launch_ordinal = GetAppLaunchOrdinal(*ext_it);
+      if (app_launch_ordinal.IsValid()) {
+        if (min_value == NULL || !min_value->IsValid() ||
+            app_launch_ordinal.LessThan(*min_value))
+          min_value = &app_launch_ordinal;

[akalin, 2011/11/29 18:39:09]

uh, how does this work?  you're setting the address to a stack variable scoped
to the for loop, and then accessing it outside the for loop.  You probably just
want to mutate {min,max}_app_launch_value directly.

[csharp, 2011/11/29 21:21:40]

Done.

+
+        if (max_value == NULL || !max_value->IsValid() ||
+            max_value->LessThan(app_launch_ordinal))
+          max_value = &app_launch_ordinal;
+      }
+    }
+  }
+
+  if (min_value != NULL)
+    *min_app_launch_value = *min_value;
+  if (max_value != NULL)
+    *max_app_launch_value = *max_value;
+
+  // If |min_app_launch_value| is valid then we had at least 1 app on the page,
+  // so both positions are valid.
+  return min_app_launch_value->IsValid();

[akalin, 2011/11/29 18:39:09]

it looks like the callers can just check IsValid() on either min or max, so we
may not need the retval after all

[csharp, 2011/11/29 21:21:40]

Done.

 }
 
-void ExtensionPrefs::SetAppLaunchIndex(const std::string& extension_id,
-                                       int index) {
-  UpdateExtensionPref(extension_id, kPrefAppLaunchIndex,
-                      Value::CreateIntegerValue(index));
+StringOrdinal ExtensionPrefs::GetFirstAppPage() const {
+  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  CHECK(extensions);
+
+  if (page_ordinal_map_.empty())
+    return StringOrdinal::CreateInitialOrdinal();
+
+  return page_ordinal_map_.begin()->first;
 }
 
-int ExtensionPrefs::GetNextAppLaunchIndex(int on_page) {
-  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
-  if (!extensions)
-    return 0;
+StringOrdinal ExtensionPrefs::CreateFirstAppLaunchOrdinal(
+    const StringOrdinal& page_ordinal) const {
+  StringOrdinal max_ordinal;
+  StringOrdinal min_ordinal;
+  GetMaxAndMinAppLaunchOrdinalsOnPage(page_ordinal, &min_ordinal, &max_ordinal);
 
-  int max_value = -1;
-  for (DictionaryValue::key_iterator extension_id = extensions->begin_keys();
-       extension_id != extensions->end_keys(); ++extension_id) {
-    int value = GetAppLaunchIndex(*extension_id);
-    int page = GetPageIndex(*extension_id);
-    if (page == on_page && value > max_value)
-      max_value = value;
-  }
-  return max_value + 1;
+  if (min_ordinal.IsValid())
+    return min_ordinal.CreateBefore();
+  else
+    return StringOrdinal::CreateInitialOrdinal();
 }
 
-int ExtensionPrefs::GetNaturalAppPageIndex() {
-  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
-  if (!extensions)
-    return 0;
-
-  std::map<int, int> page_counts;
-  for (DictionaryValue::key_iterator extension_id = extensions->begin_keys();
-       extension_id != extensions->end_keys(); ++extension_id) {
-    int page_index = GetPageIndex(*extension_id);
-    if (page_index >= 0)
-      page_counts[page_index] = page_counts[page_index] + 1;
-  }
-  for (int i = 0; ; i++) {
-    std::map<int, int>::const_iterator it = page_counts.find(i);
-    if (it == page_counts.end() || it->second < kNaturalAppPageSize)
-      return i;
-  }
+StringOrdinal ExtensionPrefs::GetAppLaunchOrdinal(
+    const std::string& extension_id) const {
+  std::string raw_value;
+  // If the preference read fails then raw_value will still be unset and we
+  // will return an invalid StringOrdinal to signal that no app launch ordinal
+  // was found.
+  ReadExtensionPrefString(extension_id, kPrefAppLaunchOrdinal, &raw_value);
+  return StringOrdinal(raw_value);
 }
 
-void ExtensionPrefs::SetAppLauncherOrder(
-    const std::vector<std::string>& extension_ids) {
-  for (size_t i = 0; i < extension_ids.size(); ++i)
-    SetAppLaunchIndex(extension_ids.at(i), i);
+void ExtensionPrefs::SetAppLaunchOrdinal(const std::string& extension_id,
+                                         const StringOrdinal& ordinal) {
+  UpdateExtensionPref(extension_id, kPrefAppLaunchOrdinal,
+                      Value::CreateStringValue(ordinal.ToString()));
+}
+
+StringOrdinal ExtensionPrefs::GetNextAppLaunchOrdinal(
+    const StringOrdinal& page_ordinal) const {
+  StringOrdinal max_ordinal;
+  StringOrdinal min_ordinal;
+  GetMaxAndMinAppLaunchOrdinalsOnPage(page_ordinal, &min_ordinal, &max_ordinal);
+
+  if (max_ordinal.IsValid())
+    return max_ordinal.CreateAfter();
+  else
+    return StringOrdinal::CreateInitialOrdinal();
+}
+
+StringOrdinal ExtensionPrefs::GetNaturalAppPageOrdinal() const {
+  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  CHECK(extensions);
+
+  if (page_ordinal_map_.empty())
+    return StringOrdinal::CreateInitialOrdinal();
+
+  std::map<StringOrdinal, int>::const_iterator it = page_ordinal_map_.begin();

[akalin, 2011/11/29 18:39:09]

scope this to the for loop (and indent as needed)

[csharp, 2011/11/29 21:21:40]

Done.

+  for (; it != page_ordinal_map_.end(); ++it) {
+    if (it->second < kNaturalAppPageSize)
+      return it->first;
+  }
+
+  // Add a new page as all existing pages are full.
+  StringOrdinal last_element = page_ordinal_map_.rbegin()->first;
+  return last_element.CreateAfter();
+}
+
+void ExtensionPrefs::OnExtensionMoved(
+    const std::string& moved_extension_id,
+    const std::string& predecessor_extension_id,
+    const std::string& successor_extension_id) {
+  // We only need to update the ordinal if the app has at least 1 neighbor.
+  if (!predecessor_extension_id.empty() || !successor_extension_id.empty()) {

[akalin, 2011/11/29 18:39:09]

do an early exit if there are no neighbors to avoid posting a notification?

[csharp, 2011/11/29 21:21:40]

Done.

+    if (predecessor_extension_id.empty()) {
+      SetAppLaunchOrdinal(
+          moved_extension_id,
+          GetAppLaunchOrdinal(successor_extension_id).CreateBefore());
+    } else if (successor_extension_id.empty()) {
+      SetAppLaunchOrdinal(
+          moved_extension_id,
+          GetAppLaunchOrdinal(predecessor_extension_id).CreateAfter());
+    } else {
+      StringOrdinal predecessor_ordinal =
+          GetAppLaunchOrdinal(predecessor_extension_id);
+      StringOrdinal sucessor_ordinal =

[akalin, 2011/11/29 18:39:09]

sucessor -> successor

[csharp, 2011/11/29 21:21:40]

Done.

+          GetAppLaunchOrdinal(successor_extension_id);
+      SetAppLaunchOrdinal(moved_extension_id,
+                          predecessor_ordinal.CreateBetween(sucessor_ordinal));
+    }
+  }
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_LAUNCHER_REORDERED,
       content::Source<ExtensionPrefs>(this),
       content::NotificationService::NoDetails());
 }
 
-int ExtensionPrefs::GetPageIndex(const std::string& extension_id) {
-  int value = -1;
-  ReadExtensionPrefInteger(extension_id, kPrefPageIndex, &value);
-  return value;
+StringOrdinal ExtensionPrefs::GetPageOrdinal(const std::string& extension_id)
+    const {
+  std::string raw_data;
+  // If the preference read fails then raw_data will still be unset and we will
+  // return an invalid StringOrdinal to signal that no page ordinal was found.
+  ReadExtensionPrefString(extension_id, kPrefPageOrdinal, &raw_data);
+  return StringOrdinal(raw_data);
 }
 
-void ExtensionPrefs::SetPageIndex(const std::string& extension_id, int index) {
-  UpdateExtensionPref(extension_id, kPrefPageIndex,
-                      Value::CreateIntegerValue(index));
+void ExtensionPrefs::SetPageOrdinal(const std::string& extension_id,
+                                    const StringOrdinal& page_ordinal) {
+  UpdatePageOrdinalMap(GetPageOrdinal(extension_id), page_ordinal);
+  UpdateExtensionPref(extension_id, kPrefPageOrdinal,
+                      Value::CreateStringValue(page_ordinal.ToString()));
 }
 
-void ExtensionPrefs::ClearPageIndex(const std::string& extension_id) {
-  UpdateExtensionPref(extension_id, kPrefPageIndex, NULL);
+void ExtensionPrefs::ClearPageOrdinal(const std::string& extension_id) {
+  UpdatePageOrdinalMap(GetPageOrdinal(extension_id), StringOrdinal());
+  UpdateExtensionPref(extension_id, kPrefPageOrdinal, NULL);
+}
+
+void ExtensionPrefs::InitializePageOrdinalMap(
+    const ExtensionIdSet& extension_ids) {
+  for (ExtensionIdSet::const_iterator ext_it = extension_ids.begin();
+       ext_it != extension_ids.end(); ++ext_it) {
+    UpdatePageOrdinalMap(StringOrdinal(), GetPageOrdinal(*ext_it));
+  }
+}
+
+void ExtensionPrefs::UpdatePageOrdinalMap(const StringOrdinal& old_value,
+                                          const StringOrdinal& new_value) {
+  if (new_value.IsValid())
+    ++page_ordinal_map_[new_value];
+
+  if (old_value.IsValid()) {
+    --page_ordinal_map_[old_value];
+
+    if (page_ordinal_map_[old_value] == 0)

[akalin, 2011/11/29 18:39:09]

is this correct?  this changes all the indices of the following pages, right?

also, you add in 0-valued pages in PageIntegerAsStringOrdinal.  How is this
supposed to work?

[csharp, 2011/11/29 21:21:40]

I'm pretty sure this is correct. When a page no longer has any icons on it, it
disappears from the NTP, so we would want to remove it and decrease the index of
all the pages after it.

The zero-value pages should only come into affect for the migration code,
because we could end up placing the apps on page 5 before the apps on page 2
(based on their app launch ordinals), so we need to temporarily add the 0-valued
pages.

I've updated the code so only the migration code can create 0-valued pages,
which also lets PageIntegerAsStringOrdinal become a const function.

On 2011/11/29 18:39:09, akalin wrote:

+      page_ordinal_map_.erase(old_value);
+  }
+}
+
+int ExtensionPrefs::PageStringOrdinalAsInteger(
+    const StringOrdinal& page_ordinal)
+    const {

[akalin, 2011/11/29 18:39:09]

move to prev line

[csharp, 2011/11/29 21:21:40]

Done.

+  if (!page_ordinal.IsValid())
+    return -1;
+
+  std::map<StringOrdinal, int>::const_iterator it =
+      page_ordinal_map_.find(page_ordinal);
+  if (it != page_ordinal_map_.end()) {
+    return std::distance(page_ordinal_map_.begin(), it);
+  } else {
+    return -1;
+  }
+}
+
+StringOrdinal ExtensionPrefs::PageIntegerAsStringOrdinal(size_t page_index) {
+  const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  if (!extensions)
+    return StringOrdinal();
+
+  if (page_index < page_ordinal_map_.size()) {
+    std::map<StringOrdinal, int, StringOrdinalLessThan>::const_iterator it =
+        page_ordinal_map_.begin();
+    advance(it, page_index);

[akalin, 2011/11/29 18:39:09]

std::advance?

[csharp, 2011/11/29 21:21:40]

Done.

+
+    return it->first;
+  } else {
+    if (page_ordinal_map_.empty())
+      page_ordinal_map_[StringOrdinal::CreateInitialOrdinal()] = 0;

[akalin, 2011/11/29 18:39:09]

ah, i see why this isn't const.

Perhaps split off the resizing into its own method, say 'RegisterPageInteger'?

[csharp, 2011/11/29 21:21:40]

Remove the need for adding extra values to the map, so there isn't much code
here now.

On 2011/11/29 18:39:09, akalin wrote:

+
+    // Create new pages until we reach the desired index.
+    while (page_index >= page_ordinal_map_.size()) {
+      StringOrdinal new_page = page_ordinal_map_.rbegin()->first.CreateAfter();
+      page_ordinal_map_[new_page] = 0;
+    }
+    return page_ordinal_map_.rbegin()->first;
+  }
 }
 
 bool ExtensionPrefs::WasAppDraggedByUser(const std::string& extension_id) {
   return ReadExtensionPrefBoolean(extension_id, kPrefUserDraggedApp);
 }
 
 void ExtensionPrefs::SetAppDraggedByUser(const std::string& extension_id) {
   UpdateExtensionPref(extension_id, kPrefUserDraggedApp,
                       Value::CreateBooleanValue(true));
 }
@@ skipping 126 matching lines @@
   // Create empty preferences dictionary for each extension (these dictionaries
   // are pruned when persisting the preferences to disk).
   for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
        ext_id != extension_ids.end(); ++ext_id) {
     ScopedExtensionPrefUpdate update(prefs_, *ext_id);
     // This creates an empty dictionary if none is stored.
     update.Get();
   }
 
   FixMissingPrefs(extension_ids);
+  InitializePageOrdinalMap(extension_ids);
   MigratePermissions(extension_ids);
+  MigrateAppIndex(extension_ids);
 
   // Store extension controlled preference values in the
   // |extension_pref_value_map_|, which then informs the subscribers
   // (ExtensionPrefStores) about the winning values.
   for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
        ext_id != extension_ids.end(); ++ext_id) {
     extension_pref_value_map_->RegisterExtension(
         *ext_id,
         GetInstallTime(*ext_id),
         !IsExtensionDisabled(*ext_id));
@@ skipping 147 matching lines @@
   prefs->RegisterListPref(prefs::kExtensionInstallAllowList,
                           PrefService::UNSYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kExtensionInstallDenyList,
                           PrefService::UNSYNCABLE_PREF);
   prefs->RegisterListPref(prefs::kExtensionInstallForceList,
                           PrefService::UNSYNCABLE_PREF);
   prefs->RegisterStringPref(kWebStoreLogin,
                             std::string() /* default_value */,
                             PrefService::UNSYNCABLE_PREF);
 }