[Sync] Generalize StringOrdinal to handle ordinal_in_parent field

sync/internal_api/public/base/node_ordinal.h

Index: sync/internal_api/public/base/node_ordinal.h
diff --git a/sync/internal_api/public/base/node_ordinal.h b/sync/internal_api/public/base/node_ordinal.h
new file mode 100644
index 0000000000000000000000000000000000000000..8e5a04fd02008611d465bb1397b785867101e6b6
--- /dev/null
+++ b/sync/internal_api/public/base/node_ordinal.h
@@ -0,0 +1,55 @@
+// Copyright (c) 2012 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 SYNC_INTERNAL_API_PUBLIC_BASE_NODE_ORDINAL_H_
+#define SYNC_INTERNAL_API_PUBLIC_BASE_NODE_ORDINAL_H_
+
+#include "base/basictypes.h"
+#include "sync/internal_api/public/base/ordinal.h"
+
+namespace syncer {
+
+// A NodeOrdinal is an Ordinal whose string value comes from the
+// ordinal_in_parent field of SyncEntity (see sync.proto).  It uses
+// the entire uint8 range for backwards compatibility with the old
+// int64-based positioning.
+
+struct NodeOrdinalTraits {
+  static const uint8 kZeroDigit = 0;

[rlarocque, 2012/09/05 21:43:46]

This mean we could end up with zeroes embedded throughout the string, right? 
Will NULL characters break std::string?

[akalin, 2012/09/06 19:25:14]

Yes, but std::string can have embedded NULLs.

+  static const uint8 kMaxDigit = kuint8max;
+  static const size_t kMinLength = 8;
+};
+
+typedef Ordinal<NodeOrdinalTraits> NodeOrdinal;

[rlarocque, 2012/09/05 21:43:46]

Ordinal<T>'s ToString() methods* probably won't work very well for this new set
of traits.  Even if the NULL character issue is handled, there probably should
be some escaping or hex-encoding of the output.

It would be nice to override them, but, as you've pointed out before, it's
probably not a good idea to inherit from an instantiated template.

Maybe the right thing to do would be to wrap Ordinal<NodeOrdinalTraits> in a new
class whose methods are almost all thin wrappers around its
Ordinal<NodeOrdinalTraits> member's methods.  That would have the side-benefit
of keeping the inline template instantiation code confined to a few .cpp files. 
And it would give you a class to which you could add the {To,From}Int64 methods.
 It would be tedious, though.  

Thoughts?

* NOTE: For the context of this comment, I assumed that ToString() was intended
as "give me a string which I can print", not "give me the internal
representation of this object".  I can see in other parts of the review you rely
on it having the latter definition.  I think there's a need for functions which
implement either definition, and, once we settle on their names, the above
comment still applies.

[akalin, 2012/09/06 19:25:14]

You're right.  Split them up into ToInternalValue() and ToDebugString().  I
thought about the thin wrapper, but it seems like too much trouble.

Also changed the names used in ordinal.h to emphasize the binary nature of the
internal representation (basically use the name 'bytes' everywhere)

+
+COMPILE_ASSERT(static_cast<char>(NodeOrdinal::kZeroDigit) == '\x00',
+               NodeOrdinalHasCorrectZeroDigit);
+COMPILE_ASSERT(static_cast<char>(NodeOrdinal::kOneDigit) == '\x01',
+               NodeOrdinalHasCorrectOneDigit);
+COMPILE_ASSERT(static_cast<char>(NodeOrdinal::kMidDigit) == '\x80',
+               NodeOrdinalHasCorrectMidDigit);
+COMPILE_ASSERT(static_cast<char>(NodeOrdinal::kMaxDigit) == '\xff',
+               NodeOrdinalHasCorrectMaxDigit);
+COMPILE_ASSERT(NodeOrdinal::kMidDigitValue == 128,
+               NodeOrdinalHasCorrectMidDigitValue);
+COMPILE_ASSERT(NodeOrdinal::kMaxDigitValue == 255,
+               NodeOrdinalHasCorrectMaxDigitValue);
+COMPILE_ASSERT(NodeOrdinal::kRadix == 256,
+               NodeOrdinalHasCorrectRadix);
+
+// Converts an int64 position (usually from the position_in_parent
+// field of SyncEntity) to a NodeOrdinal.  This transformation
+// preserves the ordering relation: a < b under integer ordering if
+// and only if Int64ToNodeOrdinal(a) < Int64ToNodeOrdinal(b).
+NodeOrdinal Int64ToNodeOrdinal(int64 x);
+
+// The inverse of Int64ToNodeOrdinal.  This conversion is, in general,
+// lossy: NodeOrdinals can have arbitrary fidelity, while numeric
+// positions contain only 64 bits of information (in fact, this is the
+// reason we've moved away from them).
+int64 NodeOrdinalToInt64(const NodeOrdinal& ordinal);
+
+}  // namespace syncer
+
+#endif  // SYNC_INTERNAL_API_PUBLIC_BASE_NODE_ORDINAL_H_