Skip to content

Commit

Permalink
Add UnackedInvalidationSet class
Browse files Browse the repository at this point in the history
Add a class that holds all locally unacknowledged invalidations and
their state. It is not used anywhere outside of tests.

This class is part of an attempt to refactor the InvalidatorStorage
class. That class stores pieces of the invalidations component's state
in preferences. It currently has some simple getter and setter methods
for things like the client ID and bootstrat data, and some more
complicated methods that related to storing unacked invalidations.

The goal is to eventually move the complex logic related to
invalidations out of the InvalidationStorage class, and into the
UnackedInvalidationSet class. The UnackedInvalidationSet class can
be owned by the SyncInvalidationListener, and periodically passed back
to the InvalidationStorage class on a separate thread for serialization.

The motivation for this refactoring is not merely aesthetic. The
UnackedInvalidationSet handles certain trickles related use cases
that the current storage system can not. Extending the existing storage
system to support these features would have made the code much harder to
understand. This approach lets us add functionality and simplify the
code at the same time.

BUG=233437

Review URL: https://codereview.chromium.org/26141004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@230529 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Oct 23, 2013
1 parent 74b2df8 commit bea426f
Show file tree
Hide file tree
Showing 12 changed files with 788 additions and 27 deletions.
3 changes: 2 additions & 1 deletion sync/internal_api/public/base/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ include_rules = [
"-sync",
"+sync/base",
"+sync/internal_api/public/base",
"+sync/internal_api/public/util",
"+sync/notifier",
"+sync/protocol",
"+sync/notifier"
]
34 changes: 32 additions & 2 deletions sync/internal_api/public/base/invalidation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const char kObjectIdKey[] = "objectId";
const char kIsUnknownVersionKey[] = "isUnknownVersion";
const char kVersionKey[] = "version";
const char kPayloadKey[] = "payload";
const int64 kInvalidVersion = -1;
}

Invalidation Invalidation::Init(
Expand All @@ -30,7 +31,14 @@ Invalidation Invalidation::Init(

Invalidation Invalidation::InitUnknownVersion(
const invalidation::ObjectId& id) {
return Invalidation(id, true, -1, std::string(), AckHandle::CreateUnique());
return Invalidation(id, true, kInvalidVersion,
std::string(), AckHandle::CreateUnique());
}

Invalidation Invalidation::InitFromDroppedInvalidation(
const Invalidation& dropped) {
return Invalidation(dropped.id_, true, kInvalidVersion,
std::string(), dropped.ack_handle_);
}

scoped_ptr<Invalidation> Invalidation::InitFromValue(
Expand All @@ -52,7 +60,7 @@ scoped_ptr<Invalidation> Invalidation::InitFromValue(
return scoped_ptr<Invalidation>(new Invalidation(
id,
true,
-1,
kInvalidVersion,
std::string(),
AckHandle::CreateUnique()));
} else {
Expand Down Expand Up @@ -105,6 +113,28 @@ void Invalidation::set_ack_handle(const AckHandle& ack_handle) {
ack_handle_ = ack_handle;
}

void Invalidation::set_ack_handler(syncer::WeakHandle<AckHandler> handler) {
ack_handler_ = handler;
}

bool Invalidation::SupportsAcknowledgement() const {
return ack_handler_.IsInitialized();
}

// void Invalidation::Acknowledge() const {
// if (SupportsAcknowledgement()) {
// ack_handler_.Call(FROM_HERE,
// &AckHandler::Acknowledge,
// id_,
// ack_handle_);
// }
// }

// void Invalidation::Drop(DroppedInvalidationTracker* tracker) const {
// DCHECK(tracker->object_id() == object_id());
// tracker->Drop(ack_handler_, ack_handle_);
// }

bool Invalidation::Equals(const Invalidation& other) const {
return id_ == other.id_
&& is_unknown_version_ == other.is_unknown_version_
Expand Down
14 changes: 14 additions & 0 deletions sync/internal_api/public/base/invalidation.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "google/cacheinvalidation/include/types.h"
#include "sync/base/sync_export.h"
#include "sync/internal_api/public/base/ack_handle.h"
#include "sync/internal_api/public/util/weak_handle.h"

namespace syncer {

Expand All @@ -30,6 +31,7 @@ class SYNC_EXPORT Invalidation {
int64 version,
const std::string& payload);
static Invalidation InitUnknownVersion(const invalidation::ObjectId& id);
static Invalidation InitFromDroppedInvalidation(const Invalidation& dropped);
static scoped_ptr<Invalidation> InitFromValue(
const base::DictionaryValue& value);

Expand All @@ -48,8 +50,19 @@ class SYNC_EXPORT Invalidation {
const std::string& payload() const;

const AckHandle& ack_handle() const;

// TODO(rlarocque): Remove this method and use AckHandlers instead.
void set_ack_handle(const AckHandle& ack_handle);

void set_ack_handler(syncer::WeakHandle<AckHandler> ack_handler);

// True if this class has a valid AckHandler.
bool SupportsAcknowledgement() const;

// TODO(rlarocque): Re-enable these when we switch to AckHandlers.
// void Acknowledge() const;
// void Drop(DroppedInvalidationTracker* tracker) const;

scoped_ptr<base::DictionaryValue> ToValue() const;
std::string ToString() const;

Expand All @@ -76,6 +89,7 @@ class SYNC_EXPORT Invalidation {

// A locally generated unique ID used to manage local acknowledgements.
AckHandle ack_handle_;
syncer::WeakHandle<AckHandler> ack_handler_;
};

} // namespace syncer
Expand Down
20 changes: 20 additions & 0 deletions sync/notifier/invalidation_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/values.h"
#include "google/cacheinvalidation/include/types.h"
#include "google/cacheinvalidation/types.pb.h"
#include "sync/internal_api/public/base/invalidation.h"

namespace invalidation {
void PrintTo(const invalidation::ObjectId& id, std::ostream* os) {
Expand All @@ -27,6 +28,25 @@ bool ObjectIdLessThan::operator()(const invalidation::ObjectId& lhs,
(lhs.source() == rhs.source() && lhs.name() < rhs.name());
}

bool InvalidationVersionLessThan::operator()(
const Invalidation& a,
const Invalidation& b) const {
DCHECK(a.object_id() == b.object_id())
<< "a: " << ObjectIdToString(a.object_id()) << ", "
<< "b: " << ObjectIdToString(a.object_id());

if (a.is_unknown_version() && !b.is_unknown_version())
return true;

if (!a.is_unknown_version() && b.is_unknown_version())
return false;

if (a.is_unknown_version() && b.is_unknown_version())
return false;

return a.version() < b.version();
}

bool RealModelTypeToObjectId(ModelType model_type,
invalidation::ObjectId* object_id) {
std::string notification_type;
Expand Down
7 changes: 7 additions & 0 deletions sync/notifier/invalidation_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,18 @@ SYNC_EXPORT_PRIVATE void PrintTo(const invalidation::ObjectId& id,

namespace syncer {

class Invalidation;

struct SYNC_EXPORT ObjectIdLessThan {
bool operator()(const invalidation::ObjectId& lhs,
const invalidation::ObjectId& rhs) const;
};

struct InvalidationVersionLessThan {
bool operator()(const syncer::Invalidation& a,
const syncer::Invalidation& b) const;
};

typedef std::set<invalidation::ObjectId, ObjectIdLessThan> ObjectIdSet;

SYNC_EXPORT bool RealModelTypeToObjectId(ModelType model_type,
Expand Down
19 changes: 0 additions & 19 deletions sync/notifier/single_object_invalidation_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,6 @@

namespace syncer {

bool InvalidationVersionLessThan::operator()(
const Invalidation& a,
const Invalidation& b) {
DCHECK(a.object_id() == b.object_id())
<< "a: " << ObjectIdToString(a.object_id()) << ", "
<< "b: " << ObjectIdToString(a.object_id());

if (a.is_unknown_version() && !b.is_unknown_version())
return true;

if (!a.is_unknown_version() && b.is_unknown_version())
return false;

if (a.is_unknown_version() && b.is_unknown_version())
return false;

return a.version() < b.version();
}

SingleObjectInvalidationSet::SingleObjectInvalidationSet() {}

SingleObjectInvalidationSet::~SingleObjectInvalidationSet() {}
Expand Down
5 changes: 1 addition & 4 deletions sync/notifier/single_object_invalidation_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@
#include "base/memory/scoped_ptr.h"
#include "sync/base/sync_export.h"
#include "sync/internal_api/public/base/invalidation.h"
#include "sync/notifier/invalidation_util.h"

namespace base {
class ListValue;
} // namespace base

namespace syncer {

struct InvalidationVersionLessThan {
bool operator()(const Invalidation& a, const Invalidation& b);
};

// Holds a list of invalidations that all share the same Object ID.
//
// The list is kept sorted by version to make it easier to perform common
Expand Down
Loading

0 comments on commit bea426f

Please sign in to comment.