Separate the NTP app ordering from the app list app ordering

chrome/browser/ui/app_list/app_list_extension_ordering.h

Index: chrome/browser/ui/app_list/app_list_extension_ordering.h
diff --git a/chrome/browser/ui/app_list/app_list_extension_ordering.h b/chrome/browser/ui/app_list/app_list_extension_ordering.h
new file mode 100644
index 0000000000000000000000000000000000000000..86cc10bd1873cd2a5836add07977c9d1fbd51e34
--- /dev/null
+++ b/chrome/browser/ui/app_list/app_list_extension_ordering.h
@@ -0,0 +1,93 @@
+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_UI_APP_LIST_APP_LIST_EXTENSION_ORDERING_H_
+#define CHROME_BROWSER_UI_APP_LIST_APP_LIST_EXTENSION_ORDERING_H_
+
+#include <map>
+#include <set>
+#include <string>
+
+#include "chrome/common/extensions/extension.h"
+#include "sync/api/string_ordinal.h"
+
+class ExtensionScopedPrefs;
+class ExtensionServiceInterface;
+class PrefService;
+
+// The ordering for apps on the app list. This ordering is represented by a
+// single ordinal.

[koz (OOO until 15th September), 2013/06/18 07:41:36]

s/This ordering is.../This class defines the order by assigning each app a
single string ordinal/.

Perhaps expand this comment by saying that this class works on an in-memory map
of ordinals which it uses to resolve conflicts generated by sync and which it
commits to prefs for persistence.

[calamity, 2013/06/19 05:09:17]

Done.

+class AppListExtensionOrdering {
+ public:
+  explicit AppListExtensionOrdering(
+      ExtensionScopedPrefs* extension_scoped_prefs);
+  ~AppListExtensionOrdering();
+  // Set the ExtensionService to syncs order changes.
+  void SetExtensionService(ExtensionServiceInterface* extension_service);
+
+  // Initialize internal data structures that require |extension_ids|. After
+  // this is called, the prefs and our internal data structures should always be
+  // consistent.

[koz (OOO until 15th September), 2013/06/18 07:41:36]

Won't they potentially be inconsistent between calls to
SetAppListOrdinalForSync() and FixSyncCollisions()?

[calamity, 2013/06/19 05:09:17]

I don't think so. SetAppListOrdinalForSync will set the ordinal in the prefs as
well as the internal data structure.

+  void Initialize(const extensions::ExtensionIdList& extension_ids);
+
+  syncer::StringOrdinal GetAppListOrdinal(const std::string& extension_id);
+
+  // Processes extensions sync data by setting the ordinal for the given
+  // |extension_id| as |ordinal|. This can result in ordinal collisions so
+  // FixSyncCollisions() should be called after all extensions are synced.
+  // There should not be an existing ordinal for |extension_id|.
+  void SetAppListOrdinalForSync(const std::string& extension_id,

[koz (OOO until 15th September), 2013/06/18 07:41:36]

How about UpdateAppListOrdinalFromSync()?

[calamity, 2013/06/19 05:09:17]

Done.

+                                const syncer::StringOrdinal& ordinal);
+
+  // Resolves any conflicts that might be created as a result of syncing that

[koz (OOO until 15th September), 2013/06/18 07:41:36]

s/result of syncing/result of calls to SetAppListOrdinalForSync()/

[calamity, 2013/06/19 05:09:17]

Done.

+  // results in two extensions having the same position in the ordering. After
+  // this is called, it is guaranteed that there will be no collisions.
+  void FixSyncCollisions();
+
+  // Add an extension so that it precedes all other extensions in the ordering.
+  void InsertAtFront(const std::string& extension_id);
+
+  // Add an extension so that all other extensions precede it in the ordering.
+  void InsertAtBack(const std::string& extension_id);
+
+  // Add to the next available position as determined by the implementation of
+  // this ordering.
+  void InsertAtNextAvailable(const std::string& extension_id);
+
+  // Updates the moved extension in the ordering so that it is now located after
+  // the given predecessor and before the successor. Empty strings are used to
+  // indicate no successor or predecessor.
+  void OnExtensionMoved(const std::string& moved_extension_id,
+                        const std::string& predecessor_extension_id,
+                        const std::string& successor_extension_id);
+
+  // Removes an app from the ordering. Does nothing if |extension_id| is not in
+  // the ordering.
+  void Erase(const std::string& extension_id);
+
+  // Returns true if |extension1| appears before |extension2| in the ordering.
+  bool ExtensionPrecedes(const std::string& extension1,

[koz (OOO until 15th September), 2013/06/18 07:41:36]

nit: extension_id1 and extension_id2

[calamity, 2013/06/19 05:09:17]

Done.

+                         const std::string& extension2);
+  // Returns true if |extension_id| is in the ordering.

[koz (OOO until 15th September), 2013/06/18 07:41:36]

nit: newline before this line

[calamity, 2013/06/19 05:09:17]

Done.

+  bool Contains(const std::string& extension_id);
+
+ private:
+  typedef std::set<std::string> ExtensionIdSet;
+  typedef std::map<syncer::StringOrdinal, ExtensionIdSet,
+      syncer::StringOrdinal::LessThanFn> AppListOrdinalMap;
+
+  void SetAppListOrdinal(const std::string& extension_id,
+                         const syncer::StringOrdinal& ordinal);
+  void EraseAppListOrdinal(const std::string& extension_id);
+  void UpdatePrefs(const std::string& extension_id,
+                   const syncer::StringOrdinal& ordinal);
+
+  AppListOrdinalMap app_list_ordinal_map_;
+  ExtensionScopedPrefs* extension_scoped_prefs_;  // Weak, owns this instance.
+  ExtensionServiceInterface* extension_service_;  // Weak.
+
+  DISALLOW_COPY_AND_ASSIGN(AppListExtensionOrdering);
+};
+
+#endif  // CHROME_BROWSER_UI_APP_LIST_APP_LIST_EXTENSION_ORDERING_H_