Convert app_launch_index and page_index from int to StringOrdinal.

chrome/browser/extensions/extension_prefs.h

Index: chrome/browser/extensions/extension_prefs.h
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index d7760a76d98a7210d7a0aae4bbb75cdd10689c7c..465fb13d1891347e4c80f096abd6f62ef5fde517 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -16,6 +16,7 @@
 #include "chrome/browser/extensions/extension_prefs_scope.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/string_ordinal.h"
 #include "googleurl/src/gurl.h"
 
 class ExtensionPrefValueMap;
@@ -99,11 +100,12 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
   void SetToolbarOrder(const std::vector<std::string>& extension_ids);
 
   // Called when an extension is installed, so that prefs get created.
-  // If |page_index| is -1, and the then a page will be found for the App.
+  // If |page_index| is kUnsetIndex, and the then a page will be found

[akalin, 2011/11/17 03:06:32]

kUnsetIndex, and the then -> an invalid ordinal, then

[csharp, 2011/11/17 19:51:58]

Done.

+  //  for the App.
   void OnExtensionInstalled(const Extension* extension,
                             Extension::State initial_state,
                             bool from_webstore,
-                            int page_index);
+                            StringOrdinal page_index);
 
   // Called when an extension is uninstalled, so that prefs get cleaned up.
   void OnExtensionUninstalled(const std::string& extension_id,
@@ -304,37 +306,53 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
   bool GetWebStoreLogin(std::string* result);
   void SetWebStoreLogin(const std::string& login);
 
+  // Get the first page index StringOrdinal value.
+  StringOrdinal GetFirstAppPage() const;
+
+  // Create a StringOrdinal which will be the first element for the given page.
+  StringOrdinal CreateFirstAppLaunchIndex(const StringOrdinal& page_index)
+      const;
+
   // Get the application launch index for an extension with |extension_id|. This
   // determines the order of which the applications appear on the New Tab Page.
-  // A value of 0 generally indicates top left. If the extension has no launch
-  // index a -1 value is returned.
-  int GetAppLaunchIndex(const std::string& extension_id);
+  // A value close to |A*| generally indicates top left. If the extension has no
+  // launch index a invalid StringOrdinal is returned.

[Finnur, 2011/11/17 14:59:25]

nit: a invalid -> an invalid.

[csharp, 2011/11/17 19:51:58]

Done.

+  StringOrdinal GetAppLaunchIndex(const std::string& extension_id) const;
 
   // Sets a specific launch index for an extension with |extension_id|.
-  void SetAppLaunchIndex(const std::string& extension_id, int index);
+  void SetAppLaunchIndex(const std::string& extension_id,
+                         const StringOrdinal& index);
 
-  // Gets the next available application launch index. This is 1 higher than the
+  // Gets the next available application launch index. This is higher than the
   // highest current application launch index found for the page |on_page|.
-  int GetNextAppLaunchIndex(int on_page);
+  StringOrdinal GetNextAppLaunchIndex(StringOrdinal on_page) const;

[Finnur, 2011/11/17 14:59:25]

const ref?

[csharp, 2011/11/17 19:51:58]

Done.

 
-  // Gets the page a new app should install to. Starts on page 0, and if there
-  // are N or more apps on it, tries to install on the next page.
-  int GetNaturalAppPageIndex();
+  // Gets the page a new app should install to. Starts the first page, and if

[Finnur, 2011/11/17 14:59:25]

Starts the first page? You mean Starts on (or with) the first page? (or am I
misreading this?)

[csharp, 2011/11/17 19:51:58]

Done.

+  // there are N or more apps on it, tries to install on the next page.
+  StringOrdinal GetNaturalAppPageIndex() const;
 
   // Sets the order the apps should be displayed in the app launcher.
-  void SetAppLauncherOrder(const std::vector<std::string>& extension_ids);
+  void SetAppLauncherOrder(const std::vector<std::string>& extension_ids,
+                           const std::string& moved_extension_id);
 
   // Get the application page index for an extension with |extension_id|.  This
   // determines which page an app will appear on in page-based NTPs.  If
-  // the app has no page specified, -1 is returned.
-  int GetPageIndex(const std::string& extension_id);
+  // the app has no page specified, kUnsetIndex is returned.
+  StringOrdinal GetPageIndex(const std::string& extension_id) const;
 
   // Sets a specific page index for an extension with |extension_id|.
-  void SetPageIndex(const std::string& extension_id, int index);
+  void SetPageIndex(const std::string& extension_id,
+                    const StringOrdinal& index);
 
   // Removes the page index for an extension.
   void ClearPageIndex(const std::string& extension_id);
 
+  // Convert the page index StringOrdinal value to its integer equivalent.
+  int PageIndexAsInteger(const StringOrdinal& page_index) const;
+
+  // Converts the page index integer to its StringOrdinal equivalent.
+  StringOrdinal PageIndexAsStringOrdinal(size_t page_index);
+
   // Returns true if the user repositioned the app on the app launcher via drag
   // and drop.
   bool WasAppDraggedByUser(const std::string& extension_id);
@@ -457,6 +475,11 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
                              const std::string& pref_key,
                              const base::ListValue** out_value);
 
+  // Reads a string pref |pref_key| from extension with id |extension_id|

[Finnur, 2011/11/17 14:59:25]

nit: end comments with punctuation marks.

[csharp, 2011/11/17 19:51:58]

Done.

+  bool ReadExtensionPrefString(const std::string& extension_id,
+                               const std::string& pref_key,
+                               std::string* out_value) const;
+
   // Interprets the list pref, |pref_key| in |extension_id|'s preferences, as a
   // URLPatternSet. The |valid_schemes| specify how to parse the URLPatterns.
   bool ReadExtensionPrefURLPatternSet(const std::string& extension_id,
@@ -514,17 +537,39 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
   // Migrates the permissions data in the pref store.
   void MigratePermissions(const ExtensionIdSet& extension_ids);
 
+  // Migrates the app launcher and page index values.
+  void MigrateAppIndex(const ExtensionIdSet& extension_ids);
+
   // Checks whether there is a state pref for the extension and if so, whether
   // it matches |check_state|.
   bool DoesExtensionHaveState(const std::string& id,
                               Extension::State check_state) const;
 
+  // Searches though all loaded app and returns all the apps that appear on
+  // the page |page_index|.
+  void GetAllAppsOnPageSorted(
+      const StringOrdinal& page_index,
+      std::set<StringOrdinal, StringOrdinal::Comparsion>* indexs_on_page) const;

[Finnur, 2011/11/17 14:59:25]

indexs? Is that a typo?
Do you mean indices?

[csharp, 2011/11/17 19:51:58]

Fixed.

+
+  // Initialize the page_index_map_ with the page indexes used by the
+  // given extensions.
+  void InitializePageIndexMap(const ExtensionIdSet& extension_ids);
+
+  // Update a value in the page_index_map_.

[Finnur, 2011/11/17 14:59:25]

This needs more documenting... What does update mean in this context? What do
old and new values mean?

[csharp, 2011/11/17 19:51:58]

Comment updated to provide more details, let me know if this is still a bit
unclear.

On 2011/11/17 14:59:25, Finnur wrote:
Done.

+  void UpdatePageIndexMap(const StringOrdinal& old_value,
+                          const StringOrdinal& new_value);
+
   // The pref service specific to this set of extension prefs. Owned by profile.
   PrefService* prefs_;
 
   // Base extensions install directory.
   FilePath install_directory_;
 
+  // A map of all the StringOrdinal page indexs mapping to how often they are

[Finnur, 2011/11/17 14:59:25]

Here it is again... indexs... ?

[csharp, 2011/11/17 19:51:58]

Fixed.

+  // used, this is used for mapping the StringOrdinals to their integer
+  // equivalent as well as quick lookup of the sorted StringOrdinals.
+  std::map<StringOrdinal, int, StringOrdinal::Comparsion> page_index_map_;
+
   // Weak pointer, owned by Profile.
   ExtensionPrefValueMap* extension_pref_value_map_;