Implement Invalidator::Acknowledge

sync/internal_api/public/base/invalidation_state.cc

 // 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.
 
 #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 {
   return (payload == other.payload) && ack_handle.Equals(other.ack_handle);
 }
 
 scoped_ptr<base::DictionaryValue> InvalidationState::ToValue() const {
   scoped_ptr<DictionaryValue> value(new DictionaryValue());
   value->SetString("payload", payload);
   value->Set("ackHandle", ack_handle.ToValue().release());
   return value.Pass();
 }
 
 bool InvalidationState::ResetFromValue(const base::DictionaryValue& value) {
   const base::Value* ack_handle_value = NULL;
   return
       value.GetString("payload", &payload) &&
       value.Get("ackHandle", &ack_handle_value) &&
       ack_handle.ResetFromValue(*ack_handle_value);
 }
 
 }  // namespace syncer