[Sync] Generalize StringOrdinal to handle ordinal_in_parent field

chrome/browser/extensions/app_sync_data.cc

Index: chrome/browser/extensions/app_sync_data.cc
diff --git a/chrome/browser/extensions/app_sync_data.cc b/chrome/browser/extensions/app_sync_data.cc
index d4d24b5bcfdceea189127455b018c4c6f8b081df..f7ce4065ee98367579d81173355a56306a5a6476 100644
--- a/chrome/browser/extensions/app_sync_data.cc
+++ b/chrome/browser/extensions/app_sync_data.cc
@@ -30,8 +30,8 @@ AppSyncData::AppSyncData(const Extension& extension,
                          bool incognito_enabled,
                          const std::string& notifications_client_id,
                          bool notifications_disabled,
-                         const StringOrdinal& app_launch_ordinal,
-                         const StringOrdinal& page_ordinal)
+                         const syncer::StringOrdinal& app_launch_ordinal,
+                         const syncer::StringOrdinal& page_ordinal)
     : extension_sync_data_(extension, enabled, incognito_enabled),
       notifications_client_id_(notifications_client_id),
       notifications_disabled_(notifications_disabled),
@@ -65,9 +65,9 @@ void AppSyncData::PopulateAppSpecifics(sync_pb::AppSpecifics* specifics) const {
 
   // Only sync the ordinal values if they are valid.
   if (app_launch_ordinal_.IsValid())
-    specifics->set_app_launch_ordinal(app_launch_ordinal_.ToString());
+    specifics->set_app_launch_ordinal(app_launch_ordinal_.ToInternalValue());

[rlarocque, 2012/09/06 21:25:56]

Much better!

Seeing this made me want to look up whether or not app_launch_ordinal was
declared as a string, and whether or not protobuf strings can contain binary
data.

It turns out that protobufs strings expect either 7-bit ASCII or UTF-8, so
binary data is not supported.  app_launch_ordinal is declared as a string.  

Then I remembered that none of this matters because this is a StringOrdinal. 
False alarm.  

I still think it's a good thing that the name prompted me to think about these
issues, though.

[akalin, 2012/09/06 22:55:25]

Yeah.  I also added a comment to StringOrdinal explicitly saying it's safe to
store in a protobuf.

   if (page_ordinal_.IsValid())
-    specifics->set_page_ordinal(page_ordinal_.ToString());
+    specifics->set_page_ordinal(page_ordinal_.ToInternalValue());
 
   extension_sync_data_.PopulateExtensionSpecifics(
       specifics->mutable_extension());
@@ -88,8 +88,8 @@ void AppSyncData::PopulateFromAppSpecifics(
       specifics.notification_settings().has_disabled() &&
       specifics.notification_settings().disabled();
 
-  app_launch_ordinal_ = StringOrdinal(specifics.app_launch_ordinal());
-  page_ordinal_ = StringOrdinal(specifics.page_ordinal());
+  app_launch_ordinal_ = syncer::StringOrdinal(specifics.app_launch_ordinal());
+  page_ordinal_ = syncer::StringOrdinal(specifics.page_ordinal());
 }
 
 void AppSyncData::PopulateFromSyncData(const syncer::SyncData& sync_data) {