Implement Invalidator::Acknowledge

sync/internal_api/public/base/invalidation_state.cc

Index: sync/internal_api/public/base/invalidation_state.cc
diff --git a/sync/internal_api/public/base/invalidation_state.cc b/sync/internal_api/public/base/invalidation_state.cc
index 6f71351ecda9da6a3c7a6e10e2901acfc7d2a985..f368641c6e561c4d706f2a4425759c7953ee3e46 100644
--- a/sync/internal_api/public/base/invalidation_state.cc
+++ b/sync/internal_api/public/base/invalidation_state.cc
@@ -4,20 +4,48 @@
 
 #include "sync/internal_api/public/base/invalidation_state.h"
 
+#include "base/rand_util.h"
 #include "base/values.h"
 
 namespace syncer {
 
+namespace {
+// Hopefully enough bytes for uniqueness.
+const size_t kBytesInHandle = 16;

[akalin, 2012/09/17 23:59:54]

include cstddef for size_t

[dcheng, 2012/09/25 22:35:06]

Done.

+// This is not a valid handle name, since all valid handles are 16 bytes long.
+const char kInvalidHandleName[] = "INVALIDHANDLENAME";
+COMPILE_ASSERT(kBytesInHandle != arraysize(kInvalidHandleName),
+               invalid_handle_name_must_differ_in_length_from_valid_handles);
+}  // namespace
+
+AckHandle AckHandle::CreateUnique() {
+  // We don't have a UUID generator, or else it'd be nice to use that instead.
+  // TODO(dcheng): Do we want to base64 encode this?

[akalin, 2012/09/17 23:59:54]

i think it would be nice to have a readable ackhandle string.  But that does
increase the probability of collision, although I think it's still negligible.

[dcheng, 2012/09/25 22:35:06]

How about formatting it as an UUID? My original rationale for 16 bytes was the
following:
- 16 bytes is the typical size of the internal buffer of a std::string, so it
avoids an extra dynamic allocation.
- 16 bytes happens to be how long UUIDs are.

However, according to 21.3.4 (disclaimer: didn't actually read, read this in a
comment in gcc's basic string implementation), the buffer always needs to be
length+1 bytes to keep a terminating NULL around, so I'm not getting the
behavior I desire already.

[akalin, 2012/10/01 23:23:08]

Yeah, the standard doesn't require keeping space for the terminating null
(data() and c_str() are allowed to differ, i.e., c_str() points to a
lazy-initialized separate buffer) but most implementations just do it anyway.

If you're concerned about dynamic alloc, why not just store a char[16]?

+  return AckHandle(base::RandBytesAsString(kBytesInHandle));
+}
+
+AckHandle AckHandle::InvalidAckHandle() {
+  return AckHandle(kInvalidHandleName);
+}
+
 bool AckHandle::Equals(const AckHandle& other) const {
-  return true;
+  return state_ == other.state_;
 }
 
 scoped_ptr<base::Value> AckHandle::ToValue() const {

[akalin, 2012/09/17 23:59:54]

return a StringValue?

[dcheng, 2012/09/25 22:35:06]

It's actually more convenient to keep using base::Value here. Unless I'm missing
something, there's no good way to convert from base::Value to base::StringValue
in InvalidationState::ResetFromValue (I could use GetAsString, but at that
point, I may as well use std::string here).

[akalin, 2012/10/01 23:23:08]

True.  Fair enough.

-  return scoped_ptr<base::Value>(new DictionaryValue());
+  return scoped_ptr<base::Value>(new StringValue(state_));
 }
 
 bool AckHandle::ResetFromValue(const base::Value& value) {

[akalin, 2012/09/17 23:59:54]

take a StringValue?

[dcheng, 2012/09/25 22:35:06]

See above.

-  return true;
+  // TODO(dcheng): Add debug mode sanity checking?
+  return value.GetAsString(&state_);
+}
+
+AckHandle::AckHandle(const std::string& state) : state_(state) {
+}
+
+InvalidationState::InvalidationState()
+    : ack_handle(AckHandle::InvalidAckHandle()) {
 }
 
 bool InvalidationState::Equals(const InvalidationState& other) const {