summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authorcsharp@chromium.org <csharp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-14 16:34:50 +0000
committercsharp@chromium.org <csharp@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-12-14 16:34:50 +0000
commit36a5c4ceb49c0bb7ed8a5242688aa285429c03de (patch)
tree28bbabf54223513cd0cf486d7f4b55b4d5996b43 /chrome
parent2f814d30264d0ebcb5f452d3b0c785b72e240ead (diff)
downloadchromium_src-36a5c4ceb49c0bb7ed8a5242688aa285429c03de.zip
chromium_src-36a5c4ceb49c0bb7ed8a5242688aa285429c03de.tar.gz
chromium_src-36a5c4ceb49c0bb7ed8a5242688aa285429c03de.tar.bz2
Convert app_launch_index and page_index from int to StringOrdinal.
This application icon index values are being changed from ints to StringOrdinals to decrease the potential number of syncs required when icons are moved, by only needing to change 1 StringOrdinal instead of all the integers. This include extra data verification that helps prevent an out of memory crash present in the earlier submission attempt. BUG=61447,107376 TEST=Open up an instance of chromium with multiple application icons and ensure that icons can still have their page and order changed. There should be no behaviour change. Review URL: http://codereview.chromium.org/8936010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114440 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r--chrome/browser/extensions/app_process_apitest.cc4
-rw-r--r--chrome/browser/extensions/crx_installer.cc3
-rw-r--r--chrome/browser/extensions/crx_installer.h9
-rw-r--r--chrome/browser/extensions/extension_browsertest.cc3
-rw-r--r--chrome/browser/extensions/extension_prefs.cc361
-rw-r--r--chrome/browser/extensions/extension_prefs.h122
-rw-r--r--chrome/browser/extensions/extension_prefs_unittest.cc271
-rw-r--r--chrome/browser/extensions/extension_service.cc6
-rw-r--r--chrome/browser/extensions/extension_service.h4
-rw-r--r--chrome/browser/extensions/test_extension_prefs.cc3
-rw-r--r--chrome/browser/extensions/unpacked_installer.cc5
-rw-r--r--chrome/browser/resources/ntp4/page_list_view.js4
-rw-r--r--chrome/browser/sync/test/integration/sync_extension_helper.cc3
-rw-r--r--chrome/browser/ui/panels/base_panel_browser_test.cc3
-rw-r--r--chrome/browser/ui/webui/ntp/app_launcher_handler.cc92
-rw-r--r--chrome/browser/ui/webui/ntp/app_launcher_handler.h6
-rw-r--r--chrome/common/string_ordinal.cc15
-rw-r--r--chrome/common/string_ordinal.h14
-rw-r--r--chrome/common/string_ordinal_unittest.cc15
19 files changed, 762 insertions, 181 deletions
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc
index 0ae9ead..8235feb 100644
--- a/chrome/browser/extensions/app_process_apitest.cc
+++ b/chrome/browser/extensions/app_process_apitest.cc
@@ -15,6 +15,7 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_file_util.h"
+#include "chrome/common/string_ordinal.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/browser/renderer_host/render_view_host.h"
#include "content/browser/tab_contents/tab_contents.h"
@@ -272,7 +273,8 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, BookmarkAppGetsNormalProcess) {
Extension::LOAD,
Extension::FROM_BOOKMARK,
&error));
- service->OnExtensionInstalled(extension, false, 0);
+ service->OnExtensionInstalled(extension, false,
+ StringOrdinal::CreateInitialOrdinal());
ASSERT_TRUE(extension.get());
ASSERT_TRUE(extension->from_bookmark());
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index add5be9..9083b65 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -133,7 +133,6 @@ CrxInstaller::CrxInstaller(base::WeakPtr<ExtensionService> frontend_weak,
extensions_enabled_(frontend_weak->extensions_enabled()),
delete_source_(false),
create_app_shortcut_(false),
- page_index_(-1),
frontend_weak_(frontend_weak),
profile_(frontend_weak->profile()),
client_(client),
@@ -588,7 +587,7 @@ void CrxInstaller::ReportSuccessFromUIThread() {
// Tell the frontend about the installation and hand off ownership of
// extension_ to it.
frontend_weak_->OnExtensionInstalled(extension_, is_gallery_install(),
- page_index_);
+ page_ordinal_);
NotifyCrxInstallComplete(extension_.get());
diff --git a/chrome/browser/extensions/crx_installer.h b/chrome/browser/extensions/crx_installer.h
index cc937ac..192ef50 100644
--- a/chrome/browser/extensions/crx_installer.h
+++ b/chrome/browser/extensions/crx_installer.h
@@ -16,6 +16,7 @@
#include "chrome/browser/extensions/extension_install_ui.h"
#include "chrome/browser/extensions/sandboxed_extension_unpacker.h"
#include "chrome/common/extensions/extension.h"
+#include "chrome/common/string_ordinal.h"
#include "chrome/common/web_apps.h"
class ExtensionService;
@@ -177,8 +178,8 @@ class CrxInstaller
install_cause_ = install_cause;
}
- void set_page_index(int page_index) {
- page_index_ = page_index;
+ void set_page_ordinal(const StringOrdinal& page_ordinal) {
+ page_ordinal_ = page_ordinal;
}
Profile* profile() { return profile_; }
@@ -271,8 +272,8 @@ class CrxInstaller
// ExtensionService on success, or delete it on failure.
scoped_refptr<const Extension> extension_;
- // The index of the NTP apps page |extension_| will be shown on.
- int page_index_;
+ // The ordinal of the NTP apps page |extension_| will be shown on.
+ StringOrdinal page_ordinal_;
// A parsed copy of the unmodified original manifest, before any
// transformations like localization have taken place.
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index 339a28c..6a0173f 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -92,7 +92,8 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithOptions(
// The call to OnExtensionInstalled ensures the other extension prefs
// are set up with the defaults.
service->extension_prefs()->OnExtensionInstalled(
- extension, Extension::ENABLED, false, 0);
+ extension, Extension::ENABLED, false,
+ StringOrdinal::CreateInitialOrdinal());
// Toggling incognito or file access will reload the extension, so wait for
// the reload and grab the new extension instance. The default state is
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 0cd7c90..4357981 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -111,10 +111,12 @@ const char kWebStoreLogin[] = "extensions.webstore_login";
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";
@@ -417,6 +419,17 @@ bool ExtensionPrefs::ReadExtensionPrefList(
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,
@@ -864,6 +877,99 @@ void ExtensionPrefs::MigratePermissions(const ExtensionIdSet& extension_ids) {
}
}
+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.
+ typedef std::map<StringOrdinal, std::map<int, const std::string*>,
+ StringOrdinalLessThan> AppPositionToIdMapping;
+ AppPositionToIdMapping 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;
+ StringOrdinal page = GetPageOrdinal(*ext_id);
+ if (ReadExtensionPrefInteger(*ext_id,
+ kPrefPageIndexDeprecated,
+ &old_page_index)) {
+ // Some extensions have invalid page index, so we don't
+ // attempt to convert them.
+ if (old_page_index < 0) {
+ DLOG(WARNING) << "Extension " << *ext_id
+ << " has an invalid page index " << old_page_index
+ << ". Aborting attempt to convert its index.";
+ break;
+ }
+
+ // Since we require all earlier StringOrdinals to already exist in order
+ // to properly convert from integers and we are iterating though them in
+ // no given order, we create earlier StringOrdinal values as required.
+ // This should be filled in by the time we are done with this loop.
+ if (page_ordinal_map_.empty())
+ page_ordinal_map_[StringOrdinal::CreateInitialOrdinal()] = 0;
+ while (page_ordinal_map_.size() <= static_cast<size_t>(old_page_index)) {
+ StringOrdinal earlier_page =
+ page_ordinal_map_.rbegin()->first.CreateAfter();
+ page_ordinal_map_[earlier_page] = 0;
+ }
+
+ page = PageIntegerAsStringOrdinal(old_page_index);
+ SetPageOrdinal(*ext_id, page);
+ 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.
+ // A valid page ordinal is also required because otherwise there is
+ // no page to add the app to.
+ if (page.IsValid())
+ app_launches_to_convert[page][old_app_launch_index] = &*ext_id;
+
+ UpdateExtensionPref(*ext_id, kPrefAppLaunchIndexDeprecated, NULL);
+ }
+ }
+
+ // Remove any empty pages that may have been added. This shouldn't occur,
+ // but double check here to prevent future problems with conversions between
+ // integers and StringOrdinals.
+ for (PageOrdinalMap::iterator it = page_ordinal_map_.begin();
+ it != page_ordinal_map_.end();) {
+ if (it->second == 0) {
+ PageOrdinalMap::iterator prev_it = it;
+ ++it;
+ page_ordinal_map_.erase(prev_it);
+ } else {
+ ++it;
+ }
+ }
+
+ 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 (AppPositionToIdMapping::const_iterator page_it =
+ app_launches_to_convert.begin();
+ page_it != app_launches_to_convert.end(); ++page_it) {
+ StringOrdinal page = page_it->first;
+ for (std::map<int, const std::string*>::const_iterator launch_it =
+ page_it->second.begin(); launch_it != page_it->second.end();
+ ++launch_it) {
+ SetAppLaunchOrdinal(*(launch_it->second),
+ CreateNextAppLaunchOrdinal(page));
+ }
+ }
+}
+
ExtensionPermissionSet* ExtensionPrefs::GetGrantedPermissions(
const std::string& extension_id) {
CHECK(Extension::IdIsValid(extension_id));
@@ -1079,7 +1185,7 @@ 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);
@@ -1111,13 +1217,12 @@ void ExtensionPrefs::OnExtensionInstalled(
}
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)));
+ StringOrdinal new_page_ordinal =
+ page_ordinal.IsValid() ? page_ordinal : GetNaturalAppPageOrdinal();
+ SetPageOrdinal(id, new_page_ordinal);
+ SetAppLaunchOrdinal(id, CreateNextAppLaunchOrdinal(new_page_ordinal));
}
+
extension_pref_value_map_->RegisterExtension(
id, install_time, initial_state == Extension::ENABLED);
content_settings_store_->RegisterExtension(
@@ -1142,6 +1247,37 @@ void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
}
}
+void ExtensionPrefs::OnExtensionMoved(
+ const std::string& moved_extension_id,
+ const std::string& predecessor_extension_id,
+ const std::string& successor_extension_id) {
+ // If there are no neighbors then no changes are required, so we can return.
+ if (predecessor_extension_id.empty() && successor_extension_id.empty())
+ return;
+
+ 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 {
+ const StringOrdinal& predecessor_ordinal =
+ GetAppLaunchOrdinal(predecessor_extension_id);
+ const StringOrdinal& successor_ordinal =
+ GetAppLaunchOrdinal(successor_extension_id);
+ SetAppLaunchOrdinal(moved_extension_id,
+ predecessor_ordinal.CreateBetween(successor_ordinal));
+ }
+
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_EXTENSION_LAUNCHER_REORDERED,
+ content::Source<ExtensionPrefs>(this),
+ content::NotificationService::NoDetails());
+}
+
void ExtensionPrefs::SetExtensionState(const std::string& extension_id,
Extension::State state) {
UpdateExtensionPref(extension_id, kPrefState,
@@ -1449,79 +1585,180 @@ void ExtensionPrefs::SetWebStoreLogin(const std::string& login) {
SavePrefs();
}
-int ExtensionPrefs::GetAppLaunchIndex(const std::string& extension_id) {
- int value;
- if (ReadExtensionPrefInteger(extension_id, kPrefAppLaunchIndex, &value))
- return value;
+StringOrdinal ExtensionPrefs::GetMinOrMaxAppLaunchOrdinalsOnPage(
+ const StringOrdinal& target_page_ordinal,
+ AppLaunchOrdinalReturn return_type) const {
+ const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+ CHECK(extensions);
+ CHECK(target_page_ordinal.IsValid());
+
+ StringOrdinal return_value;
+ for (DictionaryValue::key_iterator ext_it = extensions->begin_keys();
+ ext_it != extensions->end_keys(); ++ext_it) {
+ const StringOrdinal& page_ordinal = GetPageOrdinal(*ext_it);
+ if (page_ordinal.IsValid() && page_ordinal.Equal(target_page_ordinal)) {
+ const StringOrdinal& app_launch_ordinal = GetAppLaunchOrdinal(*ext_it);
+ if (app_launch_ordinal.IsValid()) {
+ if (!return_value.IsValid())
+ return_value = app_launch_ordinal;
+ else if (return_type == ExtensionPrefs::MIN_ORDINAL &&
+ app_launch_ordinal.LessThan(return_value))
+ return_value = app_launch_ordinal;
+ else if (return_type == ExtensionPrefs::MAX_ORDINAL &&
+ app_launch_ordinal.GreaterThan(return_value))
+ return_value = app_launch_ordinal;
+ }
+ }
+ }
+
+ return return_value;
+}
+
+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);
+}
- return -1;
+void ExtensionPrefs::SetAppLaunchOrdinal(const std::string& extension_id,
+ const StringOrdinal& ordinal) {
+ UpdateExtensionPref(extension_id, kPrefAppLaunchOrdinal,
+ Value::CreateStringValue(ordinal.ToString()));
}
-void ExtensionPrefs::SetAppLaunchIndex(const std::string& extension_id,
- int index) {
- UpdateExtensionPref(extension_id, kPrefAppLaunchIndex,
- Value::CreateIntegerValue(index));
+StringOrdinal ExtensionPrefs::CreateFirstAppLaunchOrdinal(
+ const StringOrdinal& page_ordinal) const {
+ const StringOrdinal& min_ordinal =
+ GetMinOrMaxAppLaunchOrdinalsOnPage(page_ordinal,
+ ExtensionPrefs::MIN_ORDINAL);
+
+ if (min_ordinal.IsValid())
+ return min_ordinal.CreateBefore();
+ else
+ return StringOrdinal::CreateInitialOrdinal();
+}
+
+StringOrdinal ExtensionPrefs::CreateNextAppLaunchOrdinal(
+ const StringOrdinal& page_ordinal) const {
+ const StringOrdinal& max_ordinal =
+ GetMinOrMaxAppLaunchOrdinalsOnPage(page_ordinal,
+ ExtensionPrefs::MAX_ORDINAL);
+
+ if (max_ordinal.IsValid())
+ return max_ordinal.CreateAfter();
+ else
+ return StringOrdinal::CreateInitialOrdinal();
}
-int ExtensionPrefs::GetNextAppLaunchIndex(int on_page) {
+StringOrdinal ExtensionPrefs::CreateFirstAppPageOrdinal() const {
const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
- if (!extensions)
- return 0;
+ CHECK(extensions);
- 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 (page_ordinal_map_.empty())
+ return StringOrdinal::CreateInitialOrdinal();
+
+ return page_ordinal_map_.begin()->first;
}
-int ExtensionPrefs::GetNaturalAppPageIndex() {
+StringOrdinal ExtensionPrefs::GetNaturalAppPageOrdinal() const {
const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
- if (!extensions)
- return 0;
+ CHECK(extensions);
- 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;
+ if (page_ordinal_map_.empty())
+ return StringOrdinal::CreateInitialOrdinal();
+
+ for (PageOrdinalMap::const_iterator it = page_ordinal_map_.begin();
+ 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::SetAppLauncherOrder(
- const std::vector<std::string>& extension_ids) {
- for (size_t i = 0; i < extension_ids.size(); ++i)
- SetAppLaunchIndex(extension_ids.at(i), i);
+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);
+}
- content::NotificationService::current()->Notify(
- chrome::NOTIFICATION_EXTENSION_LAUNCHER_REORDERED,
- content::Source<ExtensionPrefs>(this),
- content::NotificationService::NoDetails());
+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()));
}
-int ExtensionPrefs::GetPageIndex(const std::string& extension_id) {
- int value = -1;
- ReadExtensionPrefInteger(extension_id, kPrefPageIndex, &value);
- return value;
+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)
+ page_ordinal_map_.erase(old_value);
+ }
}
-void ExtensionPrefs::SetPageIndex(const std::string& extension_id, int index) {
- UpdateExtensionPref(extension_id, kPrefPageIndex,
- Value::CreateIntegerValue(index));
+int ExtensionPrefs::PageStringOrdinalAsInteger(
+ const StringOrdinal& page_ordinal) const {
+ if (!page_ordinal.IsValid())
+ return -1;
+
+ PageOrdinalMap::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;
+ }
}
-void ExtensionPrefs::ClearPageIndex(const std::string& extension_id) {
- UpdateExtensionPref(extension_id, kPrefPageIndex, NULL);
+StringOrdinal ExtensionPrefs::PageIntegerAsStringOrdinal(size_t page_index)
+ const {
+ // We shouldn't have a page_index that is more than 1 position away from the
+ // current end as that would imply we have empty pages which is not allowed.
+ CHECK_LE(page_index, page_ordinal_map_.size());
+
+ const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+ if (!extensions)
+ return StringOrdinal();
+
+ if (page_index < page_ordinal_map_.size()) {
+ PageOrdinalMap::const_iterator it = page_ordinal_map_.begin();
+ std::advance(it, page_index);
+
+ return it->first;
+
+ } else {
+ if (page_ordinal_map_.empty())
+ return StringOrdinal::CreateInitialOrdinal();
+ else
+ return page_ordinal_map_.rbegin()->first.CreateAfter();
+ }
}
bool ExtensionPrefs::WasAppDraggedByUser(const std::string& extension_id) {
@@ -1668,7 +1905,9 @@ void ExtensionPrefs::InitPrefStore(bool extensions_disabled) {
}
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
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 75d7068..2bc1282 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,17 +100,26 @@ 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_ordinal| is an invalid ordinal, then a page will be found
+ // for the App.
void OnExtensionInstalled(const Extension* extension,
Extension::State initial_state,
bool from_webstore,
- int page_index);
+ const StringOrdinal& page_ordinal);
// Called when an extension is uninstalled, so that prefs get cleaned up.
void OnExtensionUninstalled(const std::string& extension_id,
const Extension::Location& location,
bool external_uninstall);
+ // Updates the app launcher value for the moved extension 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);
+
// Called to change the extension's state when it is enabled/disabled.
void SetExtensionState(const std::string& extension_id, Extension::State);
@@ -304,36 +314,55 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
bool GetWebStoreLogin(std::string* result);
void SetWebStoreLogin(const std::string& login);
- // 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);
+ // Get the application launch ordinal for an app with |extension_id|. This
+ // determines the order in which the app appears on the page it's on in the
+ // New Tab Page (Note that you can compare app launch ordinals only if the
+ // apps are on the same page). A string value close to |a*| generally
+ // indicates top left. If the extension has no launch ordinal, an invalid
+ // StringOrdinal is returned.
+ StringOrdinal GetAppLaunchOrdinal(const std::string& extension_id) const;
+
+ // Sets a specific launch ordinal for an app with |extension_id|.
+ void SetAppLaunchOrdinal(const std::string& extension_id,
+ const StringOrdinal& ordinal);
+
+ // Returns a StringOrdinal that is lower than any app launch ordinal for the
+ // given page.
+ StringOrdinal CreateFirstAppLaunchOrdinal(const StringOrdinal& page_ordinal)
+ const;
- // Sets a specific launch index for an extension with |extension_id|.
- void SetAppLaunchIndex(const std::string& extension_id, int index);
+ // Returns a StringOrdinal that is higher than any app launch ordinal for the
+ // given page.
+ StringOrdinal CreateNextAppLaunchOrdinal(const StringOrdinal& page_ordinal)
+ const;
- // Gets the next available application launch index. This is 1 higher than the
- // highest current application launch index found for the page |on_page|.
- int GetNextAppLaunchIndex(int on_page);
+ // Returns a StringOrdinal that is lower than any existing page ordinal.
+ StringOrdinal CreateFirstAppPageOrdinal() const;
- // 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, which is the earliest non-full
+ // page. The returned ordinal may correspond to a page that doesn't yet exist
+ // if all pages are full.
+ StringOrdinal GetNaturalAppPageOrdinal() const;
- // Sets the order the apps should be displayed in the app launcher.
- void SetAppLauncherOrder(const std::vector<std::string>& extension_ids);
+ // Get the page ordinal for an app with |extension_id|. This determines
+ // which page an app will appear on in page-based NTPs. If the app has no
+ // page specified, an invalid StringOrdinal is returned.
+ StringOrdinal GetPageOrdinal(const std::string& extension_id) const;
- // 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);
+ // Sets a specific page ordinal for an app with |extension_id|.
+ void SetPageOrdinal(const std::string& extension_id,
+ const StringOrdinal& ordinal);
- // Sets a specific page index for an extension with |extension_id|.
- void SetPageIndex(const std::string& extension_id, int index);
+ // Removes the page ordinal for an app.
+ void ClearPageOrdinal(const std::string& extension_id);
- // Removes the page index for an extension.
- void ClearPageIndex(const std::string& extension_id);
+ // Convert the page StringOrdinal value to its integer equivalent. This takes
+ // O(# of apps) worst-case.
+ int PageStringOrdinalAsInteger(const StringOrdinal& page_ordinal) const;
+
+ // Converts the page index integer to its StringOrdinal equivalent. This takes
+ // O(# of apps) worst-case.
+ StringOrdinal PageIntegerAsStringOrdinal(size_t page_index) const;
// Returns true if the user repositioned the app on the app launcher via drag
// and drop.
@@ -409,6 +438,9 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer {
virtual base::Time GetCurrentTime() const;
private:
+ friend class ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage; // Unit test.
+ friend class ExtensionPrefsMigrateAppIndex; // Unit test.
+ friend class ExtensionPrefsMigrateAppIndexInvalid; // Unit test.
friend class ExtensionPrefsUninstallExtension; // Unit test.
// ExtensionContentSettingsStore::Observer methods:
@@ -457,6 +489,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|.
+ 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 +551,52 @@ 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;
+ // An enum used by GetMinOrMaxAppLaunchOrdinalsOnPage to specify which
+ // value should be returned.
+ enum AppLaunchOrdinalReturn {MIN_ORDINAL, MAX_ORDINAL};
+
+ // This function returns the lowest ordinal on |page_ordinal| if
+ // |return_value| == AppLaunchOrdinalReturn::MIN_ORDINAL, otherwise it returns
+ // the largest ordinal on |page_ordinal|. If there are no apps on the page
+ // then an invalid StringOrdinal is returned. It is an error to call this
+ // function with an invalid |page_ordinal|.
+ StringOrdinal GetMinOrMaxAppLaunchOrdinalsOnPage(
+ const StringOrdinal& page_ordinal,
+ AppLaunchOrdinalReturn return_type) const;
+
+ // Initialize the |page_ordinal_map_| with the page ordinals used by the
+ // given extensions.
+ void InitializePageOrdinalMap(const ExtensionIdSet& extension_ids);
+
+ // Called when an application changes the value of its page ordinal so that
+ // |page_ordinal_map_| is aware that |old_value| page ordinal has been
+ // replace by the |new_value| page ordinal and adjusts its mapping
+ // accordingly. This works with valid and invalid StringOrdinals.
+ void UpdatePageOrdinalMap(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 indices mapping to how often they are
+ // used, this is used for mapping the StringOrdinals to their integer
+ // equivalent as well as quick lookup of the sorted StringOrdinals.
+ // TODO(csharp) Convert this to a two-layer map to allow page ordinal lookup
+ // by id and vice versa.
+ typedef std::map<StringOrdinal, int, StringOrdinalLessThan> PageOrdinalMap;
+ PageOrdinalMap page_ordinal_map_;
+
// Weak pointer, owned by Profile.
ExtensionPrefValueMap* extension_pref_value_map_;
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index 3d937d8..66dfbfd 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -10,11 +10,13 @@
#include "base/stringprintf.h"
#include "chrome/browser/extensions/extension_prefs.h"
#include "chrome/browser/extensions/test_extension_prefs.h"
+#include "chrome/browser/extensions/extension_pref_value_map.h"
#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/browser/prefs/scoped_user_pref_update.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension_constants.h"
#include "chrome/common/extensions/extension_permission_set.h"
+#include "chrome/common/string_ordinal.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_source.h"
#include "content/test/notification_observer_mock.h"
@@ -654,7 +656,8 @@ class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest {
extension_ = prefs_.AddExtension("on_extension_installed");
EXPECT_FALSE(prefs()->IsExtensionDisabled(extension_->id()));
prefs()->OnExtensionInstalled(
- extension_.get(), Extension::DISABLED, false, -1);
+ extension_.get(), Extension::DISABLED, false,
+ StringOrdinal());
}
virtual void Verify() {
@@ -667,85 +670,109 @@ class ExtensionPrefsOnExtensionInstalled : public ExtensionPrefsTest {
TEST_F(ExtensionPrefsOnExtensionInstalled,
ExtensionPrefsOnExtensionInstalled) {}
-class ExtensionPrefsAppLaunchIndex : public ExtensionPrefsTest {
+class ExtensionPrefsAppLaunchOrdinal : public ExtensionPrefsTest {
public:
virtual void Initialize() {
// No extensions yet.
- EXPECT_EQ(0, prefs()->GetNextAppLaunchIndex(0));
+ StringOrdinal page = StringOrdinal::CreateInitialOrdinal();
+ EXPECT_TRUE(StringOrdinal::CreateInitialOrdinal().Equal(
+ prefs()->CreateNextAppLaunchOrdinal(page)));
extension_ = prefs_.AddApp("on_extension_installed");
EXPECT_FALSE(prefs()->IsExtensionDisabled(extension_->id()));
prefs()->OnExtensionInstalled(extension_.get(), Extension::ENABLED,
- false, -1);
+ false, StringOrdinal());
}
virtual void Verify() {
- int launch_index = prefs()->GetAppLaunchIndex(extension_->id());
- // Extension should have been assigned a launch index > 0.
- EXPECT_GT(launch_index, 0);
- EXPECT_EQ(launch_index + 1, prefs()->GetNextAppLaunchIndex(0));
- // Set a new launch index of one higher and verify.
- prefs()->SetAppLaunchIndex(extension_->id(),
- prefs()->GetNextAppLaunchIndex(0));
- int new_launch_index = prefs()->GetAppLaunchIndex(extension_->id());
- EXPECT_EQ(launch_index + 1, new_launch_index);
-
- // This extension doesn't exist, so it should return -1.
- EXPECT_EQ(-1, prefs()->GetAppLaunchIndex("foo"));
-
- // The second page doesn't have any apps so its next launch index should
- // still be 0.
- EXPECT_EQ(prefs()->GetNextAppLaunchIndex(1), 0);
+ StringOrdinal launch_ordinal =
+ prefs()->GetAppLaunchOrdinal(extension_->id());
+ StringOrdinal page_ordinal = StringOrdinal::CreateInitialOrdinal();
+
+ // Extension should have been assigned a valid StringOrdinal.
+ EXPECT_TRUE(launch_ordinal.IsValid());
+ EXPECT_TRUE(launch_ordinal.LessThan(
+ prefs()->CreateNextAppLaunchOrdinal(page_ordinal)));
+ // Set a new launch ordinal of and verify it comes after.
+ prefs()->SetAppLaunchOrdinal(
+ extension_->id(),
+ prefs()->CreateNextAppLaunchOrdinal(page_ordinal));
+ StringOrdinal new_launch_ordinal =
+ prefs()->GetAppLaunchOrdinal(extension_->id());
+ EXPECT_TRUE(launch_ordinal.LessThan(new_launch_ordinal));
+
+ // This extension doesn't exist, so it should return an invalid
+ // StringOrdinal.
+ StringOrdinal invalid_app_launch_ordinal =
+ prefs()->GetAppLaunchOrdinal("foo");
+ EXPECT_FALSE(invalid_app_launch_ordinal.IsValid());
+ EXPECT_EQ(-1, prefs()->PageStringOrdinalAsInteger(
+ invalid_app_launch_ordinal));
+
+ // The second page doesn't have any apps so its next launch ordinal should
+ // be the first launch ordinal.
+ StringOrdinal next_page = page_ordinal.CreateAfter();
+ StringOrdinal next_page_app_launch_ordinal =
+ prefs()->CreateNextAppLaunchOrdinal(next_page);
+ EXPECT_TRUE(next_page_app_launch_ordinal.Equal(
+ prefs()->CreateFirstAppLaunchOrdinal(next_page)));
}
private:
scoped_refptr<Extension> extension_;
};
-TEST_F(ExtensionPrefsAppLaunchIndex, ExtensionPrefsAppLaunchIndex) {}
+TEST_F(ExtensionPrefsAppLaunchOrdinal, ExtensionPrefsAppLaunchOrdinal) {}
-class ExtensionPrefsPageIndex : public ExtensionPrefsTest {
+class ExtensionPrefsPageOrdinal : public ExtensionPrefsTest {
public:
virtual void Initialize() {
- extension_ = prefs_.AddApp("page_index");
- // Install to page 3 (index 2).
+ extension_ = prefs_.AddApp("page_ordinal");
+ // Install with a page preference.
+ StringOrdinal page = StringOrdinal::CreateInitialOrdinal();
prefs()->OnExtensionInstalled(extension_.get(), Extension::ENABLED,
- false, 2);
- EXPECT_EQ(2, prefs()->GetPageIndex(extension_->id()));
+ false, page);
+ EXPECT_TRUE(page.Equal(prefs()->GetPageOrdinal(extension_->id())));
+ EXPECT_EQ(0, prefs()->PageStringOrdinalAsInteger(page));
- scoped_refptr<Extension> extension2 = prefs_.AddApp("page_index_2");
+ scoped_refptr<Extension> extension2 = prefs_.AddApp("page_ordinal_2");
// Install without any page preference.
prefs()->OnExtensionInstalled(extension_.get(), Extension::ENABLED,
- false, -1);
- EXPECT_EQ(0, prefs()->GetPageIndex(extension_->id()));
+ false, StringOrdinal());
+ EXPECT_TRUE(prefs()->GetPageOrdinal(extension_->id()).IsValid());
}
virtual void Verify() {
- // Set the page index.
- prefs()->SetPageIndex(extension_->id(), 1);
- // Verify the page index.
- EXPECT_EQ(1, prefs()->GetPageIndex(extension_->id()));
+ StringOrdinal old_page = prefs()->GetPageOrdinal(extension_->id());
+ StringOrdinal new_page = old_page.CreateAfter();
+
+ // Set the page ordinal.
+ prefs()->SetPageOrdinal(extension_->id(), new_page);
+ // Verify the page ordinal.
+ EXPECT_TRUE(new_page.Equal(prefs()->GetPageOrdinal(extension_->id())));
+ EXPECT_EQ(1, prefs()->PageStringOrdinalAsInteger(new_page));
- // This extension doesn't exist, so it should return -1.
- EXPECT_EQ(-1, prefs()->GetPageIndex("foo"));
+ // This extension doesn't exist, so it should return an invalid
+ // StringOrdinal.
+ EXPECT_FALSE(prefs()->GetPageOrdinal("foo").IsValid());
}
private:
scoped_refptr<Extension> extension_;
};
-TEST_F(ExtensionPrefsPageIndex, ExtensionPrefsPageIndex) {}
+TEST_F(ExtensionPrefsPageOrdinal, ExtensionPrefsPageOrdinal) {}
class ExtensionPrefsAppLocation : public ExtensionPrefsTest {
public:
virtual void Initialize() {
extension_ = prefs_.AddExtension("not_an_app");
- // Non-apps should not have any app launch index or page index.
+ // Non-apps should not have any app launch ordinal or page ordinal.
prefs()->OnExtensionInstalled(extension_.get(), Extension::ENABLED,
- false, 0);
+ false, StringOrdinal());
}
virtual void Verify() {
- EXPECT_EQ(-1, prefs()->GetAppLaunchIndex(extension_->id()));
- EXPECT_EQ(-1, prefs()->GetPageIndex(extension_->id()));
+ EXPECT_FALSE(prefs()->GetAppLaunchOrdinal(extension_->id()).IsValid());
+ EXPECT_FALSE(prefs()->GetPageOrdinal(extension_->id()).IsValid());
}
private:
@@ -759,7 +786,7 @@ class ExtensionPrefsAppDraggedByUser : public ExtensionPrefsTest {
extension_ = prefs_.AddExtension("on_extension_installed");
EXPECT_FALSE(prefs()->WasAppDraggedByUser(extension_->id()));
prefs()->OnExtensionInstalled(extension_.get(), Extension::ENABLED,
- false, -1);
+ false, StringOrdinal());
}
virtual void Verify() {
@@ -909,7 +936,8 @@ class ExtensionPrefsPreferencesBase : public ExtensionPrefsTest {
Extension* extensions[] = {ext1_, ext2_, ext3_};
for (int i = 0; i < 3; ++i) {
if (ext == extensions[i] && !installed[i]) {
- prefs()->OnExtensionInstalled(ext, Extension::ENABLED, false, -1);
+ prefs()->OnExtensionInstalled(ext, Extension::ENABLED,
+ false, StringOrdinal());
installed[i] = true;
break;
}
@@ -1216,3 +1244,162 @@ class ExtensionPrefsDisableExtensions
int iteration_;
};
TEST_F(ExtensionPrefsDisableExtensions, ExtensionPrefsDisableExtensions) {}
+
+// Tests the application index to ordinal migration code. This should be removed
+// when the migrate code is taken out.
+class ExtensionPrefsMigrateAppIndex : public ExtensionPrefsPreferencesBase {
+ public:
+ ExtensionPrefsMigrateAppIndex() {}
+ virtual ~ExtensionPrefsMigrateAppIndex() {}
+ virtual void Initialize() {
+ // A preference determining the order of which the apps appear on the NTP.
+ const char kPrefAppLaunchIndexDeprecated[] = "app_launcher_index";
+ // A preference determining the page on which an app appears in the NTP.
+ const char kPrefPageIndexDeprecated[] = "page_index";
+
+ // Setup the deprecated preferences.
+ prefs()->UpdateExtensionPref(ext1_->id(),
+ kPrefAppLaunchIndexDeprecated,
+ Value::CreateIntegerValue(0));
+ prefs()->UpdateExtensionPref(ext1_->id(),
+ kPrefPageIndexDeprecated,
+ Value::CreateIntegerValue(0));
+
+ prefs()->UpdateExtensionPref(ext2_->id(),
+ kPrefAppLaunchIndexDeprecated,
+ Value::CreateIntegerValue(1));
+ prefs()->UpdateExtensionPref(ext2_->id(),
+ kPrefPageIndexDeprecated,
+ Value::CreateIntegerValue(0));
+
+ prefs()->UpdateExtensionPref(ext3_->id(),
+ kPrefAppLaunchIndexDeprecated,
+ Value::CreateIntegerValue(0));
+ prefs()->UpdateExtensionPref(ext3_->id(),
+ kPrefPageIndexDeprecated,
+ Value::CreateIntegerValue(1));
+
+ // We insert the ids in reserve order so that we have to deal with the
+ // element on the 2nd page before the 1st page is seen.
+ ExtensionPrefs::ExtensionIdSet ids;
+ ids.push_back(ext3_->id());
+ ids.push_back(ext2_->id());
+ ids.push_back(ext1_->id());
+
+ prefs_.prefs()->MigrateAppIndex(ids);
+ }
+ virtual void Verify() {
+ StringOrdinal first_ordinal = StringOrdinal::CreateInitialOrdinal();
+
+ EXPECT_TRUE(first_ordinal.Equal(prefs()->GetAppLaunchOrdinal(ext1_->id())));
+ EXPECT_TRUE(first_ordinal.LessThan(
+ prefs()->GetAppLaunchOrdinal(ext2_->id())));
+ EXPECT_TRUE(first_ordinal.Equal(prefs()->GetAppLaunchOrdinal(ext3_->id())));
+
+ EXPECT_TRUE(first_ordinal.Equal(prefs()->GetPageOrdinal(ext1_->id())));
+ EXPECT_TRUE(first_ordinal.Equal(prefs()->GetPageOrdinal(ext2_->id())));
+ EXPECT_TRUE(first_ordinal.LessThan(prefs()->GetPageOrdinal(ext3_->id())));
+ }
+};
+TEST_F(ExtensionPrefsMigrateAppIndex, ExtensionPrefsMigrateAppIndex) {}
+
+// Tests the application index to ordinal migration code for values that
+// shouldn't be converted. This should be removed when the migrate code
+// is taken out.
+// http://crbug.com/107376
+class ExtensionPrefsMigrateAppIndexInvalid
+ : public ExtensionPrefsPreferencesBase {
+ public:
+ ExtensionPrefsMigrateAppIndexInvalid() {}
+ virtual ~ExtensionPrefsMigrateAppIndexInvalid() {}
+ virtual void Initialize() {
+ // A preference determining the order of which the apps appear on the NTP.
+ const char kPrefAppLaunchIndexDeprecated[] = "app_launcher_index";
+ // A preference determining the page on which an app appears in the NTP.
+ const char kPrefPageIndexDeprecated[] = "page_index";
+
+ // Setup the deprecated preference.
+ prefs()->UpdateExtensionPref(ext1_->id(),
+ kPrefAppLaunchIndexDeprecated,
+ Value::CreateIntegerValue(0));
+ prefs()->UpdateExtensionPref(ext1_->id(),
+ kPrefPageIndexDeprecated,
+ Value::CreateIntegerValue(-1));
+
+ ExtensionPrefs::ExtensionIdSet ids;
+ ids.push_back(ext1_->id());
+
+ prefs_.prefs()->MigrateAppIndex(ids);
+ }
+ virtual void Verify() {
+ // Make sure that the invalid page_index wasn't converted over.
+ EXPECT_FALSE(prefs()->GetAppLaunchOrdinal(ext1_->id()).IsValid());
+ }
+};
+TEST_F(ExtensionPrefsMigrateAppIndexInvalid,
+ ExtensionPrefsMigrateAppIndexInvalid) {}
+
+class ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage :
+ public ExtensionPrefsPreferencesBase {
+ public:
+ ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage() {}
+ virtual ~ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage() {}
+ virtual void Initialize() {
+ DictionaryValue simple_dict;
+ simple_dict.SetString(keys::kVersion, "1.0.0.0");
+ simple_dict.SetString(keys::kName, "unused");
+ simple_dict.SetString(keys::kApp, "true");
+ simple_dict.SetString(keys::kLaunchLocalPath, "fake.html");
+
+ std::string error;
+ app1_scoped_ = Extension::Create(
+ prefs_.temp_dir().AppendASCII("app1_"), Extension::EXTERNAL_PREF,
+ simple_dict, Extension::STRICT_ERROR_CHECKS, &error);
+ prefs()->OnExtensionInstalled(app1_scoped_.get(),
+ Extension::ENABLED,
+ false,
+ StringOrdinal());
+
+ app2_scoped_ = Extension::Create(
+ prefs_.temp_dir().AppendASCII("app2_"), Extension::EXTERNAL_PREF,
+ simple_dict, Extension::STRICT_ERROR_CHECKS, &error);
+ prefs()->OnExtensionInstalled(app2_scoped_.get(),
+ Extension::ENABLED,
+ false,
+ StringOrdinal());
+ }
+ virtual void Verify() {
+ StringOrdinal page = StringOrdinal::CreateInitialOrdinal();
+
+ StringOrdinal min = prefs()->GetMinOrMaxAppLaunchOrdinalsOnPage(
+ page,
+ ExtensionPrefs::MIN_ORDINAL);
+ StringOrdinal max = prefs()->GetMinOrMaxAppLaunchOrdinalsOnPage(
+ page,
+ ExtensionPrefs::MAX_ORDINAL);
+ EXPECT_TRUE(min.IsValid());
+ EXPECT_TRUE(max.IsValid());
+ EXPECT_TRUE(min.LessThan(max));
+
+ // Ensure that the min and max values aren't set for empty pages.
+ min = StringOrdinal();
+ max = StringOrdinal();
+ StringOrdinal empty_page = page.CreateAfter();
+ EXPECT_FALSE(min.IsValid());
+ EXPECT_FALSE(max.IsValid());
+ min = prefs()->GetMinOrMaxAppLaunchOrdinalsOnPage(
+ empty_page,
+ ExtensionPrefs::MIN_ORDINAL);
+ max = prefs()->GetMinOrMaxAppLaunchOrdinalsOnPage(
+ empty_page,
+ ExtensionPrefs::MAX_ORDINAL);
+ EXPECT_FALSE(min.IsValid());
+ EXPECT_FALSE(max.IsValid());
+ }
+
+ private:
+ scoped_refptr<Extension> app1_scoped_;
+ scoped_refptr<Extension> app2_scoped_;
+};
+TEST_F(ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage,
+ ExtensionPrefsGetMinOrMaxAppLaunchOrdinalsOnPage) {}
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 40fb7c7..c5fd28c 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2043,7 +2043,9 @@ void ExtensionService::UpdateActiveExtensionsInCrashReporter() {
}
void ExtensionService::OnExtensionInstalled(
- const Extension* extension, bool from_webstore, int page_index) {
+ const Extension* extension,
+ bool from_webstore,
+ const StringOrdinal& page_ordinal) {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
// Ensure extension is deleted unless we transfer ownership.
@@ -2100,7 +2102,7 @@ void ExtensionService::OnExtensionInstalled(
extension,
initial_enable ? Extension::ENABLED : Extension::DISABLED,
from_webstore,
- page_index);
+ page_ordinal);
// Unpacked extensions default to allowing file access, but if that has been
// overridden, don't reset the value.
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 5ab869c..00fff2e 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -379,7 +379,9 @@ class ExtensionService
// Called by the backend when an extension has been installed.
void OnExtensionInstalled(
- const Extension* extension, bool from_webstore, int page_index);
+ const Extension* extension,
+ bool from_webstore,
+ const StringOrdinal& page_ordinal);
// Initializes the |extension|'s active permission set and disables the
// extension if the privilege level has increased (e.g., due to an upgrade).
diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc
index cccbdec..aa19c4e 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -143,7 +143,8 @@ scoped_refptr<Extension> TestExtensionPrefs::AddExtensionWithManifestAndFlags(
EXPECT_TRUE(Extension::IdIsValid(extension->id()));
prefs_->OnExtensionInstalled(extension, Extension::ENABLED,
- extra_flags & Extension::FROM_WEBSTORE, 0);
+ extra_flags & Extension::FROM_WEBSTORE,
+ StringOrdinal::CreateInitialOrdinal());
return extension;
}
diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc
index 9bbea9a..eabbfe6 100644
--- a/chrome/browser/extensions/unpacked_installer.cc
+++ b/chrome/browser/extensions/unpacked_installer.cc
@@ -12,6 +12,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_file_util.h"
+#include "chrome/common/string_ordinal.h"
using content::BrowserThread;
@@ -56,7 +57,7 @@ void SimpleExtensionLoadPrompt::ShowPrompt() {
void SimpleExtensionLoadPrompt::InstallUIProceed() {
if (service_weak_.get())
service_weak_->OnExtensionInstalled(
- extension_, false, -1); // Not from web store.
+ extension_, false, StringOrdinal()); // Not from web store.
delete this;
}
@@ -209,7 +210,7 @@ void UnpackedInstaller::OnLoaded(
}
service_weak_->OnExtensionInstalled(extension,
false, // Not from web store.
- -1);
+ StringOrdinal());
}
} // namespace extensions
diff --git a/chrome/browser/resources/ntp4/page_list_view.js b/chrome/browser/resources/ntp4/page_list_view.js
index a5065b1..682e8fa 100644
--- a/chrome/browser/resources/ntp4/page_list_view.js
+++ b/chrome/browser/resources/ntp4/page_list_view.js
@@ -279,9 +279,9 @@ cr.define('ntp4', function() {
return true;
}
- // Sort by launch index
+ // Sort by launch ordinal
apps.sort(function(a, b) {
- return a.app_launch_index - b.app_launch_index;
+ return a.app_launch_ordinal > b.app_launch_ordinal;
});
// An app to animate (in case it was just installed).
diff --git a/chrome/browser/sync/test/integration/sync_extension_helper.cc b/chrome/browser/sync/test/integration/sync_extension_helper.cc
index 08d1e8b..f9c7875 100644
--- a/chrome/browser/sync/test/integration/sync_extension_helper.cc
+++ b/chrome/browser/sync/test/integration/sync_extension_helper.cc
@@ -13,6 +13,7 @@
#include "chrome/browser/extensions/pending_extension_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/string_ordinal.h"
#include "chrome/browser/sync/test/integration/sync_test.h"
#include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -64,7 +65,7 @@ void SyncExtensionHelper::InstallExtension(
ASSERT_TRUE(extension.get()) << "Could not get extension " << name
<< " (profile = " << profile << ")";
profile->GetExtensionService()->OnExtensionInstalled(
- extension, extension->UpdatesFromGallery(), 0);
+ extension, extension->UpdatesFromGallery(), StringOrdinal());
}
void SyncExtensionHelper::UninstallExtension(
diff --git a/chrome/browser/ui/panels/base_panel_browser_test.cc b/chrome/browser/ui/panels/base_panel_browser_test.cc
index 2afe673..0b66219 100644
--- a/chrome/browser/ui/panels/base_panel_browser_test.cc
+++ b/chrome/browser/ui/panels/base_panel_browser_test.cc
@@ -19,6 +19,7 @@
#include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
+#include "chrome/common/string_ordinal.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/browser/tab_contents/test_tab_contents.h"
#include "content/public/browser/notification_service.h"
@@ -379,6 +380,6 @@ scoped_refptr<Extension> BasePanelBrowserTest::CreateExtension(
EXPECT_TRUE(extension.get());
EXPECT_STREQ("", error.c_str());
browser()->GetProfile()->GetExtensionService()->
- OnExtensionInstalled(extension.get(), false, -1);
+ OnExtensionInstalled(extension.get(), false, StringOrdinal());
return extension;
}
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
index f94da20..6d1b8e0 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.cc
@@ -67,6 +67,10 @@ extension_misc::AppLaunchBucket ParseLaunchSource(
} // namespace
+AppLauncherHandler::AppInstallInfo::AppInstallInfo() {}
+
+AppLauncherHandler::AppInstallInfo::~AppInstallInfo() {}
+
AppLauncherHandler::AppLauncherHandler(ExtensionService* extension_service)
: extension_service_(extension_service),
ignore_changes_(false),
@@ -158,27 +162,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->CreateFirstAppPageOrdinal() : prefs->GetNaturalAppPageOrdinal();
+ prefs->SetPageOrdinal(extension->id(), page_ordinal);
}
- value->SetInteger("page_index", page_index);
+ // We convert the page_ordinal to an integer because 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->CreateNextAppLaunchOrdinal(page_ordinal);
+ prefs->SetAppLaunchOrdinal(extension->id(), app_launch_ordinal);
+ }
+ value->SetString("app_launch_ordinal", app_launch_ordinal.ToString());
}
WebUIMessageHandler* AppLauncherHandler::Attach(WebUI* web_ui) {
@@ -327,7 +336,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();
@@ -341,11 +350,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(extension->id()) != -1)
- prefs->ClearPageIndex(extension->id());
+ if (prefs->GetPageOrdinal(extension->id()).IsValid())
+ prefs->ClearPageOrdinal(extension->id());
}
}
@@ -643,17 +652,26 @@ void AppLauncherHandler::HandleReorderApps(const ListValue* args) {
CHECK(args->GetString(0, &dragged_app_id));
CHECK(args->GetList(1, &app_order));
- std::vector<std::string> extension_ids;
+ std::string predecessor_to_moved_ext;
+ std::string successor_to_moved_ext;
for (size_t i = 0; i < app_order->GetSize(); ++i) {
std::string value;
- if (app_order->GetString(i, &value))
- extension_ids.push_back(value);
+ if (app_order->GetString(i, &value) && value == dragged_app_id) {
+ if (i > 0)
+ CHECK(app_order->GetString(i - 1, &predecessor_to_moved_ext));
+ if (i + 1 < app_order->GetSize())
+ CHECK(app_order->GetString(i + 1, &successor_to_moved_ext));
+ break;
+ }
}
// Don't update the page; it already knows the apps have been reordered.
AutoReset<bool> auto_reset(&ignore_changes_, true);
extension_service_->extension_prefs()->SetAppDraggedByUser(dragged_app_id);
- extension_service_->extension_prefs()->SetAppLauncherOrder(extension_ids);
+ extension_service_->extension_prefs()->OnExtensionMoved(
+ dragged_app_id,
+ predecessor_to_moved_ext,
+ successor_to_moved_ext);
}
void AppLauncherHandler::HandleSetPageIndex(const ListValue* args) {
@@ -661,11 +679,14 @@ 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.
AutoReset<bool> auto_reset(&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) {
@@ -698,6 +719,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<size_t>(page_index));
Profile* profile = Profile::FromWebUI(web_ui_);
FaviconService* favicon_service =
@@ -711,7 +735,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_,
@@ -787,7 +811,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;
}
@@ -803,12 +827,12 @@ void AppLauncherHandler::SetAppToBeHighlighted() {
// static
void AppLauncherHandler::RegisterUserPrefs(PrefService* pref_service) {
- // TODO(csilv): We will want this to be a syncable preference instead.
+ // TODO(csharp): We will want this to be a syncable preference instead.
pref_service->RegisterListPref(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,
diff --git a/chrome/browser/ui/webui/ntp/app_launcher_handler.h b/chrome/browser/ui/webui/ntp/app_launcher_handler.h
index 35079ce..4006d3b 100644
--- a/chrome/browser/ui/webui/ntp/app_launcher_handler.h
+++ b/chrome/browser/ui/webui/ntp/app_launcher_handler.h
@@ -15,6 +15,7 @@
#include "chrome/browser/prefs/pref_change_registrar.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/string_ordinal.h"
#include "content/browser/cancelable_request.h"
#include "content/browser/webui/web_ui.h"
#include "content/public/browser/notification_observer.h"
@@ -114,10 +115,13 @@ class AppLauncherHandler : public WebUIMessageHandler,
private:
struct AppInstallInfo {
+ AppInstallInfo();
+ ~AppInstallInfo();
+
bool is_bookmark_app;
string16 title;
GURL app_url;
- int page_index;
+ StringOrdinal page_ordinal;
};
// Records a web store launch in the appropriate histograms. |promo_active|
diff --git a/chrome/common/string_ordinal.cc b/chrome/common/string_ordinal.cc
index f070026..069afd2 100644
--- a/chrome/common/string_ordinal.cc
+++ b/chrome/common/string_ordinal.cc
@@ -174,6 +174,10 @@ StringOrdinal::StringOrdinal() : string_ordinal_(""),
is_valid_(false) {
}
+StringOrdinal StringOrdinal::CreateInitialOrdinal() {
+ return StringOrdinal(std::string(1, kMidDigit));
+}
+
bool StringOrdinal::IsValid() const {
return is_valid_;
}
@@ -184,6 +188,12 @@ bool StringOrdinal::LessThan(const StringOrdinal& other) const {
return string_ordinal_ < other.string_ordinal_;
}
+bool StringOrdinal::GreaterThan(const StringOrdinal& other) const {
+ CHECK(IsValid());
+ CHECK(other.IsValid());
+ return string_ordinal_ > other.string_ordinal_;
+}
+
bool StringOrdinal::Equal(const StringOrdinal& other) const {
CHECK(IsValid());
CHECK(other.IsValid());
@@ -238,3 +248,8 @@ std::string StringOrdinal::ToString() const {
CHECK(IsValid());
return string_ordinal_;
}
+
+bool StringOrdinalLessThan::operator() (const StringOrdinal& lhs,
+ const StringOrdinal& rhs) const {
+ return lhs.LessThan(rhs);
+}
diff --git a/chrome/common/string_ordinal.h b/chrome/common/string_ordinal.h
index abe0fe4..f97bdf8 100644
--- a/chrome/common/string_ordinal.h
+++ b/chrome/common/string_ordinal.h
@@ -29,6 +29,11 @@ class StringOrdinal {
// Creates an invalid StringOrdinal.
StringOrdinal();
+ // Creates a valid initial StringOrdinal, this is called to create the first
+ // element of StringOrdinal list (i.e. before we have any other values we can
+ // generate from).
+ static StringOrdinal CreateInitialOrdinal();
+
bool IsValid() const;
// All remaining functions can only be called if IsValid() holds.
@@ -39,6 +44,9 @@ class StringOrdinal {
// Returns true iff |*this| < |other|.
bool LessThan(const StringOrdinal& other) const;
+ // Returns true iff |*this| > |other|.
+ bool GreaterThan(const StringOrdinal& other) const;
+
// Returns true iff |*this| == |other| (i.e. |*this| < |other| and
// |other| < |*this| are both false).
bool Equal(const StringOrdinal& other) const;
@@ -69,4 +77,10 @@ class StringOrdinal {
bool is_valid_;
};
+// A helper class that can be used by STL containers that require sorting.
+class StringOrdinalLessThan {
+ public:
+ bool operator() (const StringOrdinal& lhs, const StringOrdinal& rhs) const;
+};
+
#endif // CHROME_COMMON_STRING_ORDINAL_H_
diff --git a/chrome/common/string_ordinal_unittest.cc b/chrome/common/string_ordinal_unittest.cc
index 8b95eab..f3bc922 100644
--- a/chrome/common/string_ordinal_unittest.cc
+++ b/chrome/common/string_ordinal_unittest.cc
@@ -29,6 +29,21 @@ TEST(StringOrdinalTest, LessThan) {
EXPECT_FALSE(middle_value.LessThan(small_value));
}
+TEST(StringOrdinalTest, GreaterThan) {
+ StringOrdinal small_value("b");
+ StringOrdinal middle_value("n");
+ StringOrdinal big_value("z");
+
+ EXPECT_TRUE(big_value.GreaterThan(small_value));
+ EXPECT_TRUE(big_value.GreaterThan(middle_value));
+ EXPECT_TRUE(middle_value.GreaterThan(small_value));
+
+
+ EXPECT_FALSE(small_value.GreaterThan(middle_value));
+ EXPECT_FALSE(small_value.GreaterThan(big_value));
+ EXPECT_FALSE(middle_value.GreaterThan(big_value));
+}
+
// Tests the CreateBetween StringOrdinal function by calling
// on the small_value with the large_value as the parameter and
// vice-versa.