diff options
78 files changed, 1224 insertions, 2622 deletions
diff --git a/chrome/browser/drive/drive_notification_manager.cc b/chrome/browser/drive/drive_notification_manager.cc index d4141b0..a3dcf34 100644 --- a/chrome/browser/drive/drive_notification_manager.cc +++ b/chrome/browser/drive/drive_notification_manager.cc @@ -9,6 +9,7 @@ #include "chrome/browser/invalidation/invalidation_service.h" #include "chrome/browser/invalidation/invalidation_service_factory.h" #include "google/cacheinvalidation/types.pb.h" +#include "sync/notifier/object_id_invalidation_map.h" namespace drive { @@ -75,12 +76,10 @@ void DriveNotificationManager::OnIncomingInvalidation( kDriveInvalidationObjectId); DCHECK_EQ(1U, ids.count(object_id)); - // TODO(dcheng): Only acknowledge the invalidation once the fetch has - // completed. http://crbug.com/156843 - DCHECK(invalidation_service_); - syncer::Invalidation inv = invalidation_map.ForObject(object_id).back(); - invalidation_service_->AcknowledgeInvalidation(object_id, inv.ack_handle()); - + // This effectively disables 'local acks'. It tells the invalidations system + // to not bother saving invalidations across restarts for us. + // See crbug.com/320878. + invalidation_map.AcknowledgeAll(); NotifyObserversToUpdate(NOTIFICATION_XMPP); } diff --git a/chrome/browser/extensions/api/push_messaging/DEPS b/chrome/browser/extensions/api/push_messaging/DEPS index ba24abe..8a4c453 100644 --- a/chrome/browser/extensions/api/push_messaging/DEPS +++ b/chrome/browser/extensions/api/push_messaging/DEPS @@ -1,5 +1,5 @@ include_rules = [ "+google/cacheinvalidation/types.pb.h", - "+sync/internal_api/public/base/invalidation_test_util.h", + "+sync/internal_api/public/base/invalidation.h", "+sync/notifier/sync_notifier_observer.h", ] diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_apitest.cc b/chrome/browser/extensions/api/push_messaging/push_messaging_apitest.cc index 8cc3000..01cc1a4 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_apitest.cc +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_apitest.cc @@ -18,6 +18,7 @@ #include "chrome/common/chrome_switches.h" #include "chrome/test/base/ui_test_utils.h" #include "google/cacheinvalidation/types.pb.h" +#include "sync/internal_api/public/base/invalidation.h" #include "sync/notifier/fake_invalidator.h" #include "testing/gmock/include/gmock/gmock.h" diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc index b8b81e0..1e52211 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.cc @@ -125,13 +125,14 @@ void PushMessagingInvalidationHandler::OnInvalidatorStateChange( void PushMessagingInvalidationHandler::OnIncomingInvalidation( const syncer::ObjectIdInvalidationMap& invalidation_map) { DCHECK(thread_checker_.CalledOnValidThread()); + invalidation_map.AcknowledgeAll(); + syncer::ObjectIdSet ids = invalidation_map.GetObjectIds(); for (syncer::ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { const syncer::SingleObjectInvalidationSet& list = invalidation_map.ForObject(*it); const syncer::Invalidation& invalidation = list.back(); - service_->AcknowledgeInvalidation(*it, invalidation.ack_handle()); std::string payload; if (invalidation.is_unknown_version()) { @@ -140,8 +141,7 @@ void PushMessagingInvalidationHandler::OnIncomingInvalidation( payload = list.back().payload(); } - syncer::ObjectIdSet::iterator suppressed_id = - suppressed_ids_.find(*it); + syncer::ObjectIdSet::iterator suppressed_id = suppressed_ids_.find(*it); if (suppressed_id != suppressed_ids_.end()) { suppressed_ids_.erase(suppressed_id); continue; @@ -153,10 +153,29 @@ void PushMessagingInvalidationHandler::OnIncomingInvalidation( std::string extension_id; int subchannel; if (ObjectIdToExtensionAndSubchannel(*it, &extension_id, &subchannel)) { - DVLOG(2) << "Sending push message to reciever, extension is " - << extension_id << ", subchannel is " << subchannel - << ", and payload is " << payload; - delegate_->OnMessage(extension_id, subchannel, payload); + const syncer::SingleObjectInvalidationSet& invalidation_list = + invalidation_map.ForObject(*it); + + // We always forward unknown version invalidation when we receive one. + if (invalidation_list.StartsWithUnknownVersion()) { + DVLOG(2) << "Sending push message to reciever, extension is " + << extension_id << ", subchannel is " << subchannel + << "and payload was lost"; + delegate_->OnMessage(extension_id, subchannel, std::string()); + } + + // If we receive a new max version for this object, forward its payload. + const syncer::Invalidation& max_invalidation = invalidation_list.back(); + if (!max_invalidation.is_unknown_version() && + max_invalidation.version() > max_object_version_map_[*it]) { + max_object_version_map_[*it] = max_invalidation.version(); + DVLOG(2) << "Sending push message to reciever, extension is " + << extension_id << ", subchannel is " << subchannel + << ", and payload is " << max_invalidation.payload(); + delegate_->OnMessage(extension_id, + subchannel, + max_invalidation.payload()); + } } } } diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h index 4f7d9e3..dfb89e6 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler.h @@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_EXTENSIONS_API_PUSH_MESSAGING_PUSH_MESSAGING_INVALIDATION_HANDLER_H_ #define CHROME_BROWSER_EXTENSIONS_API_PUSH_MESSAGING_PUSH_MESSAGING_INVALIDATION_HANDLER_H_ +#include <map> #include <set> #include <string> @@ -13,6 +14,7 @@ #include "base/threading/thread_checker.h" #include "chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_mapper.h" #include "sync/notifier/invalidation_handler.h" +#include "sync/notifier/invalidation_util.h" namespace invalidation { class InvalidationService; @@ -60,6 +62,9 @@ class PushMessagingInvalidationHandler : public PushMessagingInvalidationMapper, std::set<std::string> registered_extensions_; syncer::ObjectIdSet suppressed_ids_; PushMessagingInvalidationHandlerDelegate* const delegate_; + std::map<invalidation::ObjectId, + int64, + syncer::ObjectIdLessThan> max_object_version_map_; DISALLOW_COPY_AND_ASSIGN(PushMessagingInvalidationHandler); }; diff --git a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc index 5c29b50..f868f9c 100644 --- a/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc +++ b/chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_unittest.cc @@ -9,7 +9,6 @@ #include "chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_delegate.h" #include "chrome/browser/invalidation/invalidation_service.h" #include "google/cacheinvalidation/types.pb.h" -#include "sync/internal_api/public/base/invalidation_test_util.h" #include "sync/notifier/object_id_invalidation_map.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,7 +17,6 @@ using ::testing::_; using ::testing::NotNull; using ::testing::SaveArg; using ::testing::StrictMock; -using syncer::AckHandle; namespace extensions { @@ -34,8 +32,6 @@ class MockInvalidationService : public invalidation::InvalidationService { void(syncer::InvalidationHandler*, const syncer::ObjectIdSet&)); MOCK_METHOD1(UnregisterInvalidationHandler, void(syncer::InvalidationHandler*)); - MOCK_METHOD2(AcknowledgeInvalidation, void(const invalidation::ObjectId&, - const syncer::AckHandle&)); MOCK_CONST_METHOD0(GetInvalidatorState, syncer::InvalidatorState()); MOCK_CONST_METHOD0(GetInvalidatorClientId, std::string()); @@ -126,14 +122,6 @@ TEST_F(PushMessagingInvalidationHandlerTest, Dispatch) { OnMessage("dddddddddddddddddddddddddddddddd", 0, "payload")); EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 3, "")); - - syncer::ObjectIdSet ids = invalidation_map.GetObjectIds(); - for (syncer::ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); - ++it) { - const syncer::Invalidation& inv = invalidation_map.ForObject(*it).back(); - const syncer::AckHandle& ack_handle = inv.ack_handle(); - EXPECT_CALL(service_, AcknowledgeInvalidation(*it, ack_handle)); - } handler_->OnIncomingInvalidation(invalidation_map); } @@ -170,15 +158,50 @@ TEST_F(PushMessagingInvalidationHandlerTest, DispatchInvalidObjectIds) { invalidation::ObjectId( ipc::invalidation::ObjectSource::CHROME_PUSH_MESSAGING, "U/dddddddddddddddddddddddddddddddd/4"))); - // Invalid object IDs should still be acknowledged. - syncer::ObjectIdSet ids = invalidation_map.GetObjectIds(); - for (syncer::ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); - ++it) { - const syncer::Invalidation& inv = invalidation_map.ForObject(*it).back(); - const syncer::AckHandle& ack_handle = inv.ack_handle(); - EXPECT_CALL(service_, AcknowledgeInvalidation(*it, ack_handle)); - } handler_->OnIncomingInvalidation(invalidation_map); } +// Test version filtering of incoming invalidations. +TEST_F(PushMessagingInvalidationHandlerTest, InvalidationVersionsOutOfOrder) { + const invalidation::ObjectId id0( + ipc::invalidation::ObjectSource::CHROME_PUSH_MESSAGING, + "U/dddddddddddddddddddddddddddddddd/0"); + const invalidation::ObjectId id3( + ipc::invalidation::ObjectSource::CHROME_PUSH_MESSAGING, + "U/dddddddddddddddddddddddddddddddd/3"); + + // The first received invalidation should get through. + syncer::ObjectIdInvalidationMap map1; + map1.Insert(syncer::Invalidation::Init(id0, 5, "5")); + EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 0, "5")); + handler_->OnIncomingInvalidation(map1); + testing::Mock::VerifyAndClearExpectations(&delegate_); + + // Invalid versions are always allowed through. + syncer::ObjectIdInvalidationMap map2; + map2.Insert(syncer::Invalidation::InitUnknownVersion(id0)); + EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 0, "")); + handler_->OnIncomingInvalidation(map2); + testing::Mock::VerifyAndClearExpectations(&delegate_); + + // An older version should not make it through. + syncer::ObjectIdInvalidationMap map3; + map3.Insert(syncer::Invalidation::Init(id0, 4, "4")); + handler_->OnIncomingInvalidation(map3); + + // A newer version will make it through. + syncer::ObjectIdInvalidationMap map4; + map4.Insert(syncer::Invalidation::Init(id0, 6, "6")); + EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 0, "6")); + handler_->OnIncomingInvalidation(map4); + testing::Mock::VerifyAndClearExpectations(&delegate_); + + // An unrelated object should be unaffected by all the above. + syncer::ObjectIdInvalidationMap map5; + map5.Insert(syncer::Invalidation::Init(id3, 1, "1")); + EXPECT_CALL(delegate_, OnMessage("dddddddddddddddddddddddddddddddd", 3, "1")); + handler_->OnIncomingInvalidation(map5); + testing::Mock::VerifyAndClearExpectations(&delegate_); +} + } // namespace extensions diff --git a/chrome/browser/invalidation/fake_invalidation_service.cc b/chrome/browser/invalidation/fake_invalidation_service.cc index f789ac8..96f4b55 100644 --- a/chrome/browser/invalidation/fake_invalidation_service.cc +++ b/chrome/browser/invalidation/fake_invalidation_service.cc @@ -5,12 +5,12 @@ #include "chrome/browser/invalidation/fake_invalidation_service.h" #include "chrome/browser/invalidation/invalidation_service_util.h" +#include "sync/notifier/object_id_invalidation_map.h" namespace invalidation { FakeInvalidationService::FakeInvalidationService() - : client_id_(GenerateInvalidatorClientId()), - received_invalid_acknowledgement_(false) { + : client_id_(GenerateInvalidatorClientId()) { invalidator_registrar_.UpdateInvalidatorState(syncer::INVALIDATIONS_ENABLED); } @@ -33,25 +33,6 @@ void FakeInvalidationService::UnregisterInvalidationHandler( invalidator_registrar_.UnregisterHandler(handler); } -void FakeInvalidationService::AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) { - // Try to find the given handle and object id in the unacknowledged list. - AckHandleList::iterator handle; - AckHandleList::iterator begin = unacknowledged_handles_.begin(); - AckHandleList::iterator end = unacknowledged_handles_.end(); - for (handle = begin; handle != end; ++handle) - if (handle->first.Equals(ack_handle) && handle->second == id) - break; - if (handle == end) - received_invalid_acknowledgement_ = false; - else - unacknowledged_handles_.erase(handle); - - // Add to the acknowledged list. - acknowledged_handles_.push_back(AckHandleList::value_type(ack_handle, id)); -} - syncer::InvalidatorState FakeInvalidationService::GetInvalidatorState() const { return invalidator_registrar_.GetInvalidatorState(); } @@ -67,26 +48,28 @@ void FakeInvalidationService::SetInvalidatorState( void FakeInvalidationService::EmitInvalidationForTest( const syncer::Invalidation& invalidation) { + // This function might need to modify the invalidator, so we start by making + // an identical copy of it. + syncer::Invalidation invalidation_copy(invalidation); + + // If no one is listening to this invalidation, do not send it out. + syncer::ObjectIdSet registered_ids = + invalidator_registrar_.GetAllRegisteredIds(); + if (registered_ids.find(invalidation.object_id()) == registered_ids.end()) { + mock_ack_handler_.RegisterUnsentInvalidation(&invalidation_copy); + return; + } + + // Otherwise, register the invalidation with the mock_ack_handler_ and deliver + // it to the appropriate consumer. + mock_ack_handler_.RegisterInvalidation(&invalidation_copy); syncer::ObjectIdInvalidationMap invalidation_map; - - invalidation_map.Insert(invalidation); - unacknowledged_handles_.push_back( - AckHandleList::value_type( - invalidation.ack_handle(), - invalidation.object_id())); - + invalidation_map.Insert(invalidation_copy); invalidator_registrar_.DispatchInvalidationsToHandlers(invalidation_map); } -bool FakeInvalidationService::IsInvalidationAcknowledged( - const syncer::AckHandle& ack_handle) const { - // Try to find the given handle in the acknowledged list. - AckHandleList::const_iterator begin = acknowledged_handles_.begin(); - AckHandleList::const_iterator end = acknowledged_handles_.end(); - for (AckHandleList::const_iterator handle = begin; handle != end; ++handle) - if (handle->first.Equals(ack_handle)) - return true; - return false; +syncer::MockAckHandler* FakeInvalidationService::GetMockAckHandler() { + return &mock_ack_handler_; } } // namespace invalidation diff --git a/chrome/browser/invalidation/fake_invalidation_service.h b/chrome/browser/invalidation/fake_invalidation_service.h index 5887912..e176889 100644 --- a/chrome/browser/invalidation/fake_invalidation_service.h +++ b/chrome/browser/invalidation/fake_invalidation_service.h @@ -11,6 +11,11 @@ #include "base/basictypes.h" #include "chrome/browser/invalidation/invalidation_service.h" #include "sync/notifier/invalidator_registrar.h" +#include "sync/notifier/mock_ack_handler.h" + +namespace syncer { +class Invalidation; +} namespace invalidation { @@ -29,10 +34,6 @@ class FakeInvalidationService : public InvalidationService { virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; - virtual void AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) OVERRIDE; - virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; @@ -41,25 +42,17 @@ class FakeInvalidationService : public InvalidationService { const syncer::InvalidatorRegistrar& invalidator_registrar() const { return invalidator_registrar_; } - void EmitInvalidationForTest(const syncer::Invalidation& invalidation); - // Determines if the given AckHandle has been acknowledged. - bool IsInvalidationAcknowledged(const syncer::AckHandle& ack_handle) const; + void EmitInvalidationForTest(const syncer::Invalidation& invalidation); - // Determines if AcknowledgeInvalidation was ever called with an invalid - // ObjectId/AckHandle pair. - bool ReceivedInvalidAcknowledgement() { - return received_invalid_acknowledgement_; - } + // Emitted invalidations will be hooked up to this AckHandler. Clients can + // query it to assert the invalidaitons are being acked properly. + syncer::MockAckHandler* GetMockAckHandler(); private: std::string client_id_; syncer::InvalidatorRegistrar invalidator_registrar_; - typedef std::list<std::pair<syncer::AckHandle, invalidation::ObjectId> > - AckHandleList; - AckHandleList unacknowledged_handles_; - AckHandleList acknowledged_handles_; - bool received_invalid_acknowledgement_; + syncer::MockAckHandler mock_ack_handler_; DISALLOW_COPY_AND_ASSIGN(FakeInvalidationService); }; diff --git a/chrome/browser/invalidation/invalidation_service.h b/chrome/browser/invalidation/invalidation_service.h index 7ee0b73..33b09f3 100644 --- a/chrome/browser/invalidation/invalidation_service.h +++ b/chrome/browser/invalidation/invalidation_service.h @@ -11,7 +11,6 @@ namespace syncer { class InvalidationHandler; -class AckHandle; } // namespace syncer namespace invalidation { @@ -91,11 +90,6 @@ class InvalidationService : public BrowserContextKeyedService { virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) = 0; - // Sends an acknowledgement that an invalidation for |id| was successfully - // handled. - virtual void AcknowledgeInvalidation(const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) = 0; - // Returns the current invalidator state. When called from within // InvalidationHandler::OnInvalidatorStateChange(), this must return // the updated state. diff --git a/chrome/browser/invalidation/invalidation_service_android.cc b/chrome/browser/invalidation/invalidation_service_android.cc index 4bada4b..ad30aaa 100644 --- a/chrome/browser/invalidation/invalidation_service_android.cc +++ b/chrome/browser/invalidation/invalidation_service_android.cc @@ -7,6 +7,7 @@ #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/invalidation/invalidation_controller_android.h" #include "content/public/browser/notification_service.h" +#include "sync/notifier/object_id_invalidation_map.h" namespace invalidation { @@ -44,13 +45,6 @@ void InvalidationServiceAndroid::UnregisterInvalidationHandler( invalidator_registrar_.UnregisterHandler(handler); } -void InvalidationServiceAndroid::AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) { - DCHECK(CalledOnValidThread()); - // Do nothing. The Android invalidator does not support ack tracking. -} - syncer::InvalidatorState InvalidationServiceAndroid::GetInvalidatorState() const { DCHECK(CalledOnValidThread()); diff --git a/chrome/browser/invalidation/invalidation_service_android.h b/chrome/browser/invalidation/invalidation_service_android.h index e932e2c..36b3315 100644 --- a/chrome/browser/invalidation/invalidation_service_android.h +++ b/chrome/browser/invalidation/invalidation_service_android.h @@ -48,9 +48,6 @@ class InvalidationServiceAndroid const syncer::ObjectIdSet& ids) OVERRIDE; virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; - virtual void AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) OVERRIDE; virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; diff --git a/chrome/browser/invalidation/invalidator_storage.cc b/chrome/browser/invalidation/invalidator_storage.cc index 042f942..1d2fc86 100644 --- a/chrome/browser/invalidation/invalidator_storage.cc +++ b/chrome/browser/invalidation/invalidator_storage.cc @@ -16,78 +16,37 @@ #include "base/values.h" #include "chrome/common/pref_names.h" #include "components/user_prefs/pref_registry_syncable.h" -#include "sync/internal_api/public/base/model_type.h" - -using syncer::InvalidationStateMap; namespace { -const char kSourceKey[] = "source"; -const char kNameKey[] = "name"; -const char kMaxVersionKey[] = "max-version"; -const char kPayloadKey[] = "payload"; -const char kCurrentAckHandleKey[] = "current-ack"; -const char kExpectedAckHandleKey[] = "expected-ack"; +const char kInvalidatorMaxInvalidationVersions[] = + "invalidator.max_invalidation_versions"; -bool ValueToObjectIdAndState(const DictionaryValue& value, - invalidation::ObjectId* id, - syncer::InvalidationState* state) { - std::string source_str; - if (!value.GetString(kSourceKey, &source_str)) { - DLOG(WARNING) << "Unable to deserialize source"; - return false; - } - int source = 0; - if (!base::StringToInt(source_str, &source)) { - DLOG(WARNING) << "Invalid source: " << source_str; - return false; - } - std::string name; - if (!value.GetString(kNameKey, &name)) { - DLOG(WARNING) << "Unable to deserialize name"; - return false; - } - *id = invalidation::ObjectId(source, name); - std::string max_version_str; - if (!value.GetString(kMaxVersionKey, &max_version_str)) { - DLOG(WARNING) << "Unable to deserialize max version"; - return false; - } - if (!base::StringToInt64(max_version_str, &state->version)) { - DLOG(WARNING) << "Invalid max invalidation version: " << max_version_str; - return false; - } - value.GetString(kPayloadKey, &state->payload); - // The ack handle fields won't be set if upgrading from previous versions of - // Chrome. - const base::DictionaryValue* current_ack_handle_value = NULL; - if (value.GetDictionary(kCurrentAckHandleKey, ¤t_ack_handle_value)) { - state->current.ResetFromValue(*current_ack_handle_value); - } - const base::DictionaryValue* expected_ack_handle_value = NULL; - if (value.GetDictionary(kExpectedAckHandleKey, &expected_ack_handle_value)) { - state->expected.ResetFromValue(*expected_ack_handle_value); - } else { - // In this case, we should never have a valid current value set. - DCHECK(!state->current.IsValid()); - state->current = syncer::AckHandle::InvalidAckHandle(); + +bool ValueToUnackedInvalidationStorageMap( + const ListValue& value, + syncer::UnackedInvalidationsMap* map) { + for (size_t i = 0; i != value.GetSize(); ++i) { + invalidation::ObjectId invalid_id; + syncer::UnackedInvalidationSet storage(invalid_id); + const base::DictionaryValue* dict; + if (!value.GetDictionary(i, &dict) || !storage.ResetFromValue(*dict)) { + DLOG(WARNING) << "Failed to parse ObjectState at position " << i; + return false; + } + map->insert(std::make_pair(storage.object_id(), storage)); } return true; } -// The caller owns the returned value. -DictionaryValue* ObjectIdAndStateToValue( - const invalidation::ObjectId& id, const syncer::InvalidationState& state) { - DictionaryValue* value = new DictionaryValue; - value->SetString(kSourceKey, base::IntToString(id.source())); - value->SetString(kNameKey, id.name()); - value->SetString(kMaxVersionKey, base::Int64ToString(state.version)); - value->SetString(kPayloadKey, state.payload); - if (state.current.IsValid()) - value->Set(kCurrentAckHandleKey, state.current.ToValue().release()); - if (state.expected.IsValid()) - value->Set(kExpectedAckHandleKey, state.expected.ToValue().release()); - return value; +scoped_ptr<base::ListValue> UnackedInvalidationStorageMapToValue( + const syncer::UnackedInvalidationsMap& map) { + scoped_ptr<base::ListValue> value(new base::ListValue); + for (syncer::UnackedInvalidationsMap::const_iterator it = map.begin(); + it != map.end(); ++it) { + value->Append(it->second.ToValue().release()); + } + return value.Pass(); } } // namespace @@ -97,7 +56,7 @@ namespace invalidation { // static void InvalidatorStorage::RegisterProfilePrefs( user_prefs::PrefRegistrySyncable* registry) { - registry->RegisterListPref(prefs::kInvalidatorMaxInvalidationVersions, + registry->RegisterListPref(prefs::kInvalidatorSavedInvalidations, user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); registry->RegisterStringPref( prefs::kInvalidatorInvalidationState, @@ -107,156 +66,22 @@ void InvalidatorStorage::RegisterProfilePrefs( prefs::kInvalidatorClientId, std::string(), user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); - registry->RegisterDictionaryPref( - prefs::kSyncMaxInvalidationVersions, - user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); + + // This pref is obsolete. We register it so we can clear it. + // At some point in the future, it will be safe to remove this. + registry->RegisterListPref(kInvalidatorMaxInvalidationVersions, + user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF); } InvalidatorStorage::InvalidatorStorage(PrefService* pref_service) : pref_service_(pref_service) { DCHECK(pref_service); - MigrateMaxInvalidationVersionsPref(); + pref_service_->ClearPref(kInvalidatorMaxInvalidationVersions); } InvalidatorStorage::~InvalidatorStorage() { } -InvalidationStateMap InvalidatorStorage::GetAllInvalidationStates() const { - DCHECK(thread_checker_.CalledOnValidThread()); - InvalidationStateMap state_map; - const base::ListValue* state_map_list = - pref_service_->GetList(prefs::kInvalidatorMaxInvalidationVersions); - CHECK(state_map_list); - DeserializeFromList(*state_map_list, &state_map); - return state_map; -} - -void InvalidatorStorage::SetMaxVersionAndPayload( - const invalidation::ObjectId& id, - int64 max_version, - const std::string& payload) { - DCHECK(thread_checker_.CalledOnValidThread()); - InvalidationStateMap state_map = GetAllInvalidationStates(); - InvalidationStateMap::iterator it = state_map.find(id); - if ((it != state_map.end()) && (max_version <= it->second.version)) { - NOTREACHED(); - return; - } - state_map[id].version = max_version; - state_map[id].payload = payload; - - base::ListValue state_map_list; - SerializeToList(state_map, &state_map_list); - pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions, - state_map_list); -} - -void InvalidatorStorage::Forget(const syncer::ObjectIdSet& ids) { - DCHECK(thread_checker_.CalledOnValidThread()); - InvalidationStateMap state_map = GetAllInvalidationStates(); - for (syncer::ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); - ++it) { - state_map.erase(*it); - } - - base::ListValue state_map_list; - SerializeToList(state_map, &state_map_list); - pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions, - state_map_list); -} - -// static -void InvalidatorStorage::DeserializeFromList( - const base::ListValue& state_map_list, - InvalidationStateMap* state_map) { - state_map->clear(); - for (size_t i = 0; i < state_map_list.GetSize(); ++i) { - const DictionaryValue* value = NULL; - if (!state_map_list.GetDictionary(i, &value)) { - DLOG(WARNING) << "Unable to deserialize entry " << i; - continue; - } - invalidation::ObjectId id; - syncer::InvalidationState state; - if (!ValueToObjectIdAndState(*value, &id, &state)) { - DLOG(WARNING) << "Error while deserializing entry " << i; - continue; - } - (*state_map)[id] = state; - } -} - -// static -void InvalidatorStorage::SerializeToList( - const InvalidationStateMap& state_map, - base::ListValue* state_map_list) { - for (InvalidationStateMap::const_iterator it = state_map.begin(); - it != state_map.end(); ++it) { - state_map_list->Append(ObjectIdAndStateToValue(it->first, it->second)); - } -} - -// Legacy migration code. -void InvalidatorStorage::MigrateMaxInvalidationVersionsPref() { - const base::DictionaryValue* max_versions_dict = - pref_service_->GetDictionary(prefs::kSyncMaxInvalidationVersions); - CHECK(max_versions_dict); - if (!max_versions_dict->empty()) { - InvalidationStateMap state_map; - DeserializeMap(max_versions_dict, &state_map); - base::ListValue state_map_list; - SerializeToList(state_map, &state_map_list); - pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions, - state_map_list); - UMA_HISTOGRAM_BOOLEAN("InvalidatorStorage.MigrateInvalidationVersionsPref", - true); - } else { - UMA_HISTOGRAM_BOOLEAN("InvalidatorStorage.MigrateInvalidationVersionsPref", - false); - } - pref_service_->ClearPref(prefs::kSyncMaxInvalidationVersions); -} - -// Legacy migration code. -// static -void InvalidatorStorage::DeserializeMap( - const base::DictionaryValue* max_versions_dict, - InvalidationStateMap* map) { - map->clear(); - // Convert from a string -> string DictionaryValue to a - // ModelType -> int64 map. - for (base::DictionaryValue::Iterator it(*max_versions_dict); !it.IsAtEnd(); - it.Advance()) { - int model_type_int = 0; - if (!base::StringToInt(it.key(), &model_type_int)) { - LOG(WARNING) << "Invalid model type key: " << it.key(); - continue; - } - if ((model_type_int < syncer::FIRST_REAL_MODEL_TYPE) || - (model_type_int >= syncer::MODEL_TYPE_COUNT)) { - LOG(WARNING) << "Out-of-range model type key: " << model_type_int; - continue; - } - const syncer::ModelType model_type = - syncer::ModelTypeFromInt(model_type_int); - std::string max_version_str; - CHECK(it.value().GetAsString(&max_version_str)); - int64 max_version = 0; - if (!base::StringToInt64(max_version_str, &max_version)) { - LOG(WARNING) << "Invalid max invalidation version for " - << syncer::ModelTypeToString(model_type) << ": " - << max_version_str; - continue; - } - invalidation::ObjectId id; - if (!syncer::RealModelTypeToObjectId(model_type, &id)) { - DLOG(WARNING) << "Invalid model type: " << model_type; - continue; - } - (*map)[id].version = max_version; - } -} - void InvalidatorStorage::SetInvalidatorClientId(const std::string& client_id) { DCHECK(thread_checker_.CalledOnValidThread()); Clear(); // We can't reuse our old invalidation state if the ID changes. @@ -283,52 +108,29 @@ std::string InvalidatorStorage::GetBootstrapData() const { return data; } -void InvalidatorStorage::Clear() { - DCHECK(thread_checker_.CalledOnValidThread()); - pref_service_->ClearPref(prefs::kInvalidatorMaxInvalidationVersions); - pref_service_->ClearPref(prefs::kInvalidatorClientId); - pref_service_->ClearPref(prefs::kInvalidatorInvalidationState); +void InvalidatorStorage::SetSavedInvalidations( + const syncer::UnackedInvalidationsMap& map) { + scoped_ptr<base::ListValue> value(UnackedInvalidationStorageMapToValue(map)); + pref_service_->Set(prefs::kInvalidatorSavedInvalidations, *value.get()); } -void InvalidatorStorage::GenerateAckHandles( - const syncer::ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - const base::Callback<void(const syncer::AckHandleMap&)> callback) { - DCHECK(thread_checker_.CalledOnValidThread()); - InvalidationStateMap state_map = GetAllInvalidationStates(); - - syncer::AckHandleMap ack_handles; - for (syncer::ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); - ++it) { - state_map[*it].expected = syncer::AckHandle::CreateUnique(); - ack_handles.insert(std::make_pair(*it, state_map[*it].expected)); +syncer::UnackedInvalidationsMap +InvalidatorStorage::GetSavedInvalidations() const { + syncer::UnackedInvalidationsMap map; + const base::ListValue* value = + pref_service_->GetList(prefs::kInvalidatorSavedInvalidations); + if (!ValueToUnackedInvalidationStorageMap(*value, &map)) { + return syncer::UnackedInvalidationsMap(); + } else { + return map; } - - base::ListValue state_map_list; - SerializeToList(state_map, &state_map_list); - pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions, - state_map_list); - - ignore_result(task_runner->PostTask(FROM_HERE, - base::Bind(callback, ack_handles))); } -void InvalidatorStorage::Acknowledge(const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) { +void InvalidatorStorage::Clear() { DCHECK(thread_checker_.CalledOnValidThread()); - InvalidationStateMap state_map = GetAllInvalidationStates(); - - InvalidationStateMap::iterator it = state_map.find(id); - // This could happen if the acknowledgement is delayed and Forget() has - // already been called. - if (it == state_map.end()) - return; - it->second.current = ack_handle; - - base::ListValue state_map_list; - SerializeToList(state_map, &state_map_list); - pref_service_->Set(prefs::kInvalidatorMaxInvalidationVersions, - state_map_list); + pref_service_->ClearPref(prefs::kInvalidatorSavedInvalidations); + pref_service_->ClearPref(prefs::kInvalidatorClientId); + pref_service_->ClearPref(prefs::kInvalidatorInvalidationState); } } // namespace invalidation diff --git a/chrome/browser/invalidation/invalidator_storage.h b/chrome/browser/invalidation/invalidator_storage.h index 4a2fdd5..75c3f90 100644 --- a/chrome/browser/invalidation/invalidator_storage.h +++ b/chrome/browser/invalidation/invalidator_storage.h @@ -14,6 +14,7 @@ #include "base/memory/weak_ptr.h" #include "base/threading/thread_checker.h" #include "sync/notifier/invalidation_state_tracker.h" +#include "sync/notifier/unacked_invalidation_set.h" class PrefService; @@ -38,23 +39,15 @@ class InvalidatorStorage : public base::SupportsWeakPtr<InvalidatorStorage>, virtual ~InvalidatorStorage(); // InvalidationStateTracker implementation. - virtual syncer::InvalidationStateMap GetAllInvalidationStates() const - OVERRIDE; - virtual void SetMaxVersionAndPayload(const invalidation::ObjectId& id, - int64 max_version, - const std::string& payload) OVERRIDE; - virtual void Forget(const syncer::ObjectIdSet& ids) OVERRIDE; virtual void SetInvalidatorClientId(const std::string& client_id) OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; virtual void SetBootstrapData(const std::string& data) OVERRIDE; virtual std::string GetBootstrapData() const OVERRIDE; + virtual void SetSavedInvalidations( + const syncer::UnackedInvalidationsMap& map) OVERRIDE; + virtual syncer::UnackedInvalidationsMap GetSavedInvalidations() + const OVERRIDE; virtual void Clear() OVERRIDE; - virtual void GenerateAckHandles( - const syncer::ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const syncer::AckHandleMap&)> callback) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) OVERRIDE; private: FRIEND_TEST_ALL_PREFIXES(InvalidatorStorageTest, SerializeEmptyMap); @@ -78,20 +71,6 @@ class InvalidatorStorage : public base::SupportsWeakPtr<InvalidatorStorage>, base::ThreadChecker thread_checker_; - // Helpers to convert between InvalidationStateMap <--> ListValue. - static void DeserializeFromList( - const base::ListValue& state_map_list, - syncer::InvalidationStateMap* state_map); - static void SerializeToList( - const syncer::InvalidationStateMap& state_map, - base::ListValue* state_map_list); - - // Code for migrating from old MaxInvalidationVersions pref, which was a map - // from sync types to max invalidation versions. - void MigrateMaxInvalidationVersionsPref(); - static void DeserializeMap(const base::DictionaryValue* max_versions_dict, - syncer::InvalidationStateMap* map); - PrefService* const pref_service_; DISALLOW_COPY_AND_ASSIGN(InvalidatorStorage); diff --git a/chrome/browser/invalidation/invalidator_storage_unittest.cc b/chrome/browser/invalidation/invalidator_storage_unittest.cc index 224ed0d..b09d8d5 100644 --- a/chrome/browser/invalidation/invalidator_storage_unittest.cc +++ b/chrome/browser/invalidation/invalidator_storage_unittest.cc @@ -4,47 +4,18 @@ #include "chrome/browser/invalidation/invalidator_storage.h" -#include "base/bind.h" -#include "base/message_loop/message_loop.h" -#include "base/message_loop/message_loop_proxy.h" #include "base/prefs/pref_service.h" -#include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "chrome/common/pref_names.h" #include "chrome/test/base/testing_pref_service_syncable.h" -#include "sync/internal_api/public/base/invalidation_test_util.h" -#include "testing/gmock/include/gmock/gmock.h" +#include "sync/notifier/unacked_invalidation_set_test_util.h" #include "testing/gtest/include/gtest/gtest.h" -using syncer::InvalidationStateMap; - -namespace { - -const char kSourceKey[] = "source"; -const char kNameKey[] = "name"; -const char kMaxVersionKey[] = "max-version"; -const char kPayloadKey[] = "payload"; -const char kCurrentAckHandleKey[] = "current-ack"; -const char kExpectedAckHandleKey[] = "expected-ack"; - -const int kChromeSyncSourceId = 1004; - -void GenerateAckHandlesTestHelper(syncer::AckHandleMap* output, - const syncer::AckHandleMap& input) { - *output = input; -} - -} // namespace - namespace invalidation { class InvalidatorStorageTest : public testing::Test { public: - InvalidatorStorageTest() - : kBookmarksId_(kChromeSyncSourceId, "BOOKMARK"), - kPreferencesId_(kChromeSyncSourceId, "PREFERENCE"), - kAppNotificationsId_(kChromeSyncSourceId, "APP_NOTIFICATION"), - kAutofillId_(kChromeSyncSourceId, "AUTOFILL") {} + InvalidatorStorageTest() {} virtual void SetUp() { InvalidatorStorage::RegisterProfilePrefs(pref_service_.registry()); @@ -52,69 +23,12 @@ class InvalidatorStorageTest : public testing::Test { protected: TestingPrefServiceSyncable pref_service_; - - const invalidation::ObjectId kBookmarksId_; - const invalidation::ObjectId kPreferencesId_; - const invalidation::ObjectId kAppNotificationsId_; - const invalidation::ObjectId kAutofillId_; - - base::MessageLoop loop_; }; -// Set invalidation states for various keys and verify that they are written and -// read back correctly. -TEST_F(InvalidatorStorageTest, SetMaxVersionAndPayload) { - InvalidatorStorage storage(&pref_service_); - - InvalidationStateMap expected_states; - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - - expected_states[kBookmarksId_].version = 2; - expected_states[kBookmarksId_].payload = "hello"; - storage.SetMaxVersionAndPayload(kBookmarksId_, 2, "hello"); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - - expected_states[kPreferencesId_].version = 5; - storage.SetMaxVersionAndPayload(kPreferencesId_, 5, std::string()); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - - expected_states[kAppNotificationsId_].version = 3; - expected_states[kAppNotificationsId_].payload = "world"; - storage.SetMaxVersionAndPayload(kAppNotificationsId_, 3, "world"); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - - expected_states[kAppNotificationsId_].version = 4; - expected_states[kAppNotificationsId_].payload = "again"; - storage.SetMaxVersionAndPayload(kAppNotificationsId_, 4, "again"); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); -} - -// Forgetting an entry should cause that entry to be deleted. -TEST_F(InvalidatorStorageTest, Forget) { - InvalidatorStorage storage(&pref_service_); - EXPECT_TRUE(storage.GetAllInvalidationStates().empty()); - - InvalidationStateMap expected_states; - expected_states[kBookmarksId_].version = 2; - expected_states[kBookmarksId_].payload = "a"; - expected_states[kPreferencesId_].version = 5; - expected_states[kPreferencesId_].payload = "b"; - storage.SetMaxVersionAndPayload(kBookmarksId_, 2, "a"); - storage.SetMaxVersionAndPayload(kPreferencesId_, 5, "b"); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - - expected_states.erase(kPreferencesId_); - syncer::ObjectIdSet to_forget; - to_forget.insert(kPreferencesId_); - storage.Forget(to_forget); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); -} - // Clearing the storage should erase all version map entries, bootstrap data, // and the client ID. TEST_F(InvalidatorStorageTest, Clear) { InvalidatorStorage storage(&pref_service_); - EXPECT_TRUE(storage.GetAllInvalidationStates().empty()); EXPECT_TRUE(storage.GetBootstrapData().empty()); EXPECT_TRUE(storage.GetInvalidatorClientId().empty()); @@ -124,292 +38,12 @@ TEST_F(InvalidatorStorageTest, Clear) { storage.SetBootstrapData("test"); EXPECT_EQ("test", storage.GetBootstrapData()); - { - InvalidationStateMap expected_states; - expected_states[kAppNotificationsId_].version = 3; - storage.SetMaxVersionAndPayload(kAppNotificationsId_, 3, std::string()); - EXPECT_EQ(expected_states, storage.GetAllInvalidationStates()); - } - storage.Clear(); - EXPECT_TRUE(storage.GetAllInvalidationStates().empty()); EXPECT_TRUE(storage.GetBootstrapData().empty()); EXPECT_TRUE(storage.GetInvalidatorClientId().empty()); } -TEST_F(InvalidatorStorageTest, SerializeEmptyMap) { - InvalidationStateMap empty_map; - base::ListValue list; - InvalidatorStorage::SerializeToList(empty_map, &list); - EXPECT_TRUE(list.empty()); -} - -// Make sure we don't choke on a variety of malformed input. -TEST_F(InvalidatorStorageTest, DeserializeFromListInvalidFormat) { - InvalidationStateMap map; - base::ListValue list_with_invalid_format; - DictionaryValue* value; - - // The various cases below use distinct values to make it easier to track down - // failures. - value = new DictionaryValue(); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString("completely", "invalid"); - list_with_invalid_format.Append(value); - - // Missing two required fields - value = new DictionaryValue(); - value->SetString(kSourceKey, "10"); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString(kNameKey, "missing source and version"); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString(kMaxVersionKey, "3"); - list_with_invalid_format.Append(value); - - // Missing one required field - value = new DictionaryValue(); - value->SetString(kSourceKey, "14"); - value->SetString(kNameKey, "missing version"); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString(kSourceKey, "233"); - value->SetString(kMaxVersionKey, "5"); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString(kNameKey, "missing source"); - value->SetString(kMaxVersionKey, "25"); - list_with_invalid_format.Append(value); - - // Invalid values in fields - value = new DictionaryValue(); - value->SetString(kSourceKey, "a"); - value->SetString(kNameKey, "bad source"); - value->SetString(kMaxVersionKey, "12"); - list_with_invalid_format.Append(value); - - value = new DictionaryValue(); - value->SetString(kSourceKey, "1"); - value->SetString(kNameKey, "bad max version"); - value->SetString(kMaxVersionKey, "a"); - list_with_invalid_format.Append(value); - - // And finally something that should work. - invalidation::ObjectId valid_id(42, "this should work"); - value = new DictionaryValue(); - value->SetString(kSourceKey, "42"); - value->SetString(kNameKey, valid_id.name()); - value->SetString(kMaxVersionKey, "20"); - list_with_invalid_format.Append(value); - - InvalidatorStorage::DeserializeFromList(list_with_invalid_format, &map); - - EXPECT_EQ(1U, map.size()); - EXPECT_EQ(20, map[valid_id].version); -} - -// Tests behavior when there are duplicate entries for a single key. The value -// of the last entry with that key should be used in the version map. -TEST_F(InvalidatorStorageTest, DeserializeFromListWithDuplicates) { - InvalidationStateMap map; - base::ListValue list; - DictionaryValue* value; - - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source())); - value->SetString(kNameKey, kBookmarksId_.name()); - value->SetString(kMaxVersionKey, "20"); - list.Append(value); - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kAutofillId_.source())); - value->SetString(kNameKey, kAutofillId_.name()); - value->SetString(kMaxVersionKey, "10"); - list.Append(value); - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source())); - value->SetString(kNameKey, kBookmarksId_.name()); - value->SetString(kMaxVersionKey, "15"); - list.Append(value); - - InvalidatorStorage::DeserializeFromList(list, &map); - EXPECT_EQ(2U, map.size()); - EXPECT_EQ(10, map[kAutofillId_].version); - EXPECT_EQ(15, map[kBookmarksId_].version); -} - -TEST_F(InvalidatorStorageTest, DeserializeFromEmptyList) { - InvalidationStateMap map; - base::ListValue list; - InvalidatorStorage::DeserializeFromList(list, &map); - EXPECT_TRUE(map.empty()); -} - -// Tests that deserializing a well-formed value results in the expected state -// map. -TEST_F(InvalidatorStorageTest, DeserializeFromListBasic) { - InvalidationStateMap map; - base::ListValue list; - DictionaryValue* value; - syncer::AckHandle ack_handle_1 = syncer::AckHandle::CreateUnique(); - syncer::AckHandle ack_handle_2 = syncer::AckHandle::CreateUnique(); - - value = new DictionaryValue(); - value->SetString(kSourceKey, - base::IntToString(kAppNotificationsId_.source())); - value->SetString(kNameKey, kAppNotificationsId_.name()); - value->SetString(kMaxVersionKey, "20"); - value->SetString(kPayloadKey, "testing"); - value->Set(kCurrentAckHandleKey, ack_handle_1.ToValue().release()); - value->Set(kExpectedAckHandleKey, ack_handle_2.ToValue().release()); - list.Append(value); - - InvalidatorStorage::DeserializeFromList(list, &map); - EXPECT_EQ(1U, map.size()); - EXPECT_EQ(20, map[kAppNotificationsId_].version); - EXPECT_EQ("testing", map[kAppNotificationsId_].payload); - EXPECT_THAT(map[kAppNotificationsId_].current, syncer::Eq(ack_handle_1)); - EXPECT_THAT(map[kAppNotificationsId_].expected, syncer::Eq(ack_handle_2)); -} - -// Tests that deserializing well-formed values when optional parameters are -// omitted works. -TEST_F(InvalidatorStorageTest, DeserializeFromListMissingOptionalValues) { - InvalidationStateMap map; - base::ListValue list; - DictionaryValue* value; - syncer::AckHandle ack_handle = syncer::AckHandle::CreateUnique(); - - // Payload missing because of an upgrade from a previous browser version that - // didn't set the field. - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kAutofillId_.source())); - value->SetString(kNameKey, kAutofillId_.name()); - value->SetString(kMaxVersionKey, "10"); - list.Append(value); - // A crash between SetMaxVersion() and a callback from GenerateAckHandles() - // could result in this state. - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kBookmarksId_.source())); - value->SetString(kNameKey, kBookmarksId_.name()); - value->SetString(kMaxVersionKey, "15"); - value->SetString(kPayloadKey, "hello"); - list.Append(value); - // Never acknowledged, so current ack handle is unset. - value = new DictionaryValue(); - value->SetString(kSourceKey, base::IntToString(kPreferencesId_.source())); - value->SetString(kNameKey, kPreferencesId_.name()); - value->SetString(kMaxVersionKey, "20"); - value->SetString(kPayloadKey, "world"); - value->Set(kExpectedAckHandleKey, ack_handle.ToValue().release()); - list.Append(value); - - InvalidatorStorage::DeserializeFromList(list, &map); - EXPECT_EQ(3U, map.size()); - - EXPECT_EQ(10, map[kAutofillId_].version); - EXPECT_EQ("", map[kAutofillId_].payload); - EXPECT_FALSE(map[kAutofillId_].current.IsValid()); - EXPECT_FALSE(map[kAutofillId_].expected.IsValid()); - - EXPECT_EQ(15, map[kBookmarksId_].version); - EXPECT_EQ("hello", map[kBookmarksId_].payload); - EXPECT_FALSE(map[kBookmarksId_].current.IsValid()); - EXPECT_FALSE(map[kBookmarksId_].expected.IsValid()); - - EXPECT_EQ(20, map[kPreferencesId_].version); - EXPECT_EQ("world", map[kPreferencesId_].payload); - EXPECT_FALSE(map[kPreferencesId_].current.IsValid()); - EXPECT_THAT(map[kPreferencesId_].expected, Eq(ack_handle)); -} - -// Tests for legacy deserialization code. -TEST_F(InvalidatorStorageTest, DeserializeMapOutOfRange) { - InvalidationStateMap map; - base::DictionaryValue dict_with_out_of_range_type; - - dict_with_out_of_range_type.SetString( - base::IntToString(syncer::TOP_LEVEL_FOLDER), "100"); - dict_with_out_of_range_type.SetString( - base::IntToString(syncer::BOOKMARKS), "5"); - - InvalidatorStorage::DeserializeMap(&dict_with_out_of_range_type, &map); - - EXPECT_EQ(1U, map.size()); - EXPECT_EQ(5, map[kBookmarksId_].version); -} - -TEST_F(InvalidatorStorageTest, DeserializeMapInvalidFormat) { - InvalidationStateMap map; - base::DictionaryValue dict_with_invalid_format; - - dict_with_invalid_format.SetString("whoops", "5"); - dict_with_invalid_format.SetString("ohnoes", "whoops"); - dict_with_invalid_format.SetString( - base::IntToString(syncer::BOOKMARKS), "ohnoes"); - dict_with_invalid_format.SetString( - base::IntToString(syncer::AUTOFILL), "10"); - - InvalidatorStorage::DeserializeMap(&dict_with_invalid_format, &map); - - EXPECT_EQ(1U, map.size()); - EXPECT_EQ(10, map[kAutofillId_].version); -} - -TEST_F(InvalidatorStorageTest, DeserializeMapEmptyDictionary) { - InvalidationStateMap map; - base::DictionaryValue dict; - InvalidatorStorage::DeserializeMap(&dict, &map); - EXPECT_TRUE(map.empty()); -} - -TEST_F(InvalidatorStorageTest, DeserializeMapBasic) { - InvalidationStateMap map; - base::DictionaryValue dict; - - dict.SetString(base::IntToString(syncer::AUTOFILL), "10"); - dict.SetString(base::IntToString(syncer::BOOKMARKS), "15"); - - InvalidatorStorage::DeserializeMap(&dict, &map); - EXPECT_EQ(2U, map.size()); - EXPECT_EQ(10, map[kAutofillId_].version); - EXPECT_EQ(15, map[kBookmarksId_].version); -} - -// Test that the migration code for the legacy preference works as expected. -// Migration should happen on construction of InvalidatorStorage. -TEST_F(InvalidatorStorageTest, MigrateLegacyPreferences) { - base::DictionaryValue* legacy_dict = new DictionaryValue; - legacy_dict->SetString(base::IntToString(syncer::AUTOFILL), "10"); - legacy_dict->SetString(base::IntToString(syncer::BOOKMARKS), "32"); - legacy_dict->SetString(base::IntToString(syncer::PREFERENCES), "54"); - pref_service_.SetUserPref(prefs::kSyncMaxInvalidationVersions, legacy_dict); - InvalidatorStorage storage(&pref_service_); - - // Legacy pref should be cleared. - const base::DictionaryValue* dict = - pref_service_.GetDictionary(prefs::kSyncMaxInvalidationVersions); - EXPECT_TRUE(dict->empty()); - - // Validate the new pref is set correctly. - InvalidationStateMap map; - const base::ListValue* list = - pref_service_.GetList(prefs::kInvalidatorMaxInvalidationVersions); - InvalidatorStorage::DeserializeFromList(*list, &map); - - EXPECT_EQ(3U, map.size()); - EXPECT_EQ(10, map[kAutofillId_].version); - EXPECT_EQ(32, map[kBookmarksId_].version); - EXPECT_EQ(54, map[kPreferencesId_].version); -} - TEST_F(InvalidatorStorageTest, SetGetNotifierClientId) { InvalidatorStorage storage(&pref_service_); const std::string client_id("fK6eDzAIuKqx9A4+93bljg=="); @@ -427,80 +61,44 @@ TEST_F(InvalidatorStorageTest, SetGetBootstrapData) { EXPECT_EQ(mess, storage.GetBootstrapData()); } -// Test that we correctly generate ack handles, acknowledge them, and persist -// them. -TEST_F(InvalidatorStorageTest, GenerateAckHandlesAndAcknowledge) { +TEST_F(InvalidatorStorageTest, SaveGetInvalidations_Empty) { InvalidatorStorage storage(&pref_service_); - syncer::ObjectIdSet ids; - InvalidationStateMap state_map; - syncer::AckHandleMap ack_handle_map; - syncer::AckHandleMap::const_iterator it; - - // Test that it works as expected if the key doesn't already exist in the map, - // e.g. the first invalidation received for the object ID was not for a - // specific version. - ids.insert(kAutofillId_); - storage.GenerateAckHandles( - ids, base::MessageLoopProxy::current(), - base::Bind(&GenerateAckHandlesTestHelper, &ack_handle_map)); - loop_.RunUntilIdle(); - EXPECT_EQ(1U, ack_handle_map.size()); - it = ack_handle_map.find(kAutofillId_); - // Android STL appears to be buggy and causes gtest's IsContainerTest<> to - // treat an iterator as a STL container so we use != instead of ASSERT_NE. - ASSERT_TRUE(ack_handle_map.end() != it); - EXPECT_TRUE(it->second.IsValid()); - state_map[kAutofillId_].expected = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); + syncer::UnackedInvalidationsMap empty_map; + ASSERT_TRUE(empty_map.empty()); - storage.Acknowledge(kAutofillId_, it->second); - state_map[kAutofillId_].current = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); - - ids.clear(); - - // Test that it works as expected if the key already exists. - state_map[kBookmarksId_].version = 11; - state_map[kBookmarksId_].payload = "hello"; - storage.SetMaxVersionAndPayload(kBookmarksId_, 11, "hello"); - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); - ids.insert(kBookmarksId_); - storage.GenerateAckHandles( - ids, base::MessageLoopProxy::current(), - base::Bind(&GenerateAckHandlesTestHelper, &ack_handle_map)); - loop_.RunUntilIdle(); - EXPECT_EQ(1U, ack_handle_map.size()); - it = ack_handle_map.find(kBookmarksId_); - ASSERT_TRUE(ack_handle_map.end() != it); - EXPECT_TRUE(it->second.IsValid()); - state_map[kBookmarksId_].expected = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); - - storage.Acknowledge(kBookmarksId_, it->second); - state_map[kBookmarksId_].current = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); + storage.SetSavedInvalidations(empty_map); + syncer::UnackedInvalidationsMap restored_map = + storage.GetSavedInvalidations(); + EXPECT_TRUE(restored_map.empty()); +} - // Finally, test that the ack handles are updated if we're asked to generate - // another ack handle for the same object ID. - state_map[kBookmarksId_].version = 12; - state_map[kBookmarksId_].payload = "world"; - storage.SetMaxVersionAndPayload(kBookmarksId_, 12, "world"); - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); - ids.insert(kBookmarksId_); - storage.GenerateAckHandles( - ids, base::MessageLoopProxy::current(), - base::Bind(&GenerateAckHandlesTestHelper, &ack_handle_map)); - loop_.RunUntilIdle(); - EXPECT_EQ(1U, ack_handle_map.size()); - it = ack_handle_map.find(kBookmarksId_); - ASSERT_TRUE(ack_handle_map.end() != it); - EXPECT_TRUE(it->second.IsValid()); - state_map[kBookmarksId_].expected = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); +TEST_F(InvalidatorStorageTest, SaveGetInvalidations) { + InvalidatorStorage storage(&pref_service_); - storage.Acknowledge(kBookmarksId_, it->second); - state_map[kBookmarksId_].current = it->second; - EXPECT_EQ(state_map, storage.GetAllInvalidationStates()); + ObjectId id1(10, "object1"); + syncer::UnackedInvalidationSet storage1(id1); + syncer::Invalidation unknown_version_inv = + syncer::Invalidation::InitUnknownVersion(id1); + syncer::Invalidation known_version_inv = + syncer::Invalidation::Init(id1, 314, "payload"); + storage1.Add(unknown_version_inv); + storage1.Add(known_version_inv); + + ObjectId id2(10, "object2"); + syncer::UnackedInvalidationSet storage2(id2); + syncer::Invalidation obj2_inv = + syncer::Invalidation::Init(id2, 1234, "payl\0ad\xff"); + storage2.Add(obj2_inv); + + syncer::UnackedInvalidationsMap map; + map.insert(std::make_pair(storage1.object_id(), storage1)); + map.insert(std::make_pair(storage2.object_id(), storage2)); + + storage.SetSavedInvalidations(map); + syncer::UnackedInvalidationsMap restored_map = + storage.GetSavedInvalidations(); + + EXPECT_THAT(map, syncer::test_util::Eq(restored_map)); } } // namespace invalidation diff --git a/chrome/browser/invalidation/p2p_invalidation_service.cc b/chrome/browser/invalidation/p2p_invalidation_service.cc index c802136..1eda06e 100644 --- a/chrome/browser/invalidation/p2p_invalidation_service.cc +++ b/chrome/browser/invalidation/p2p_invalidation_service.cc @@ -57,12 +57,6 @@ void P2PInvalidationService::UnregisterInvalidationHandler( invalidator_->UnregisterHandler(handler); } -void P2PInvalidationService::AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) { - invalidator_->Acknowledge(id, ack_handle); -} - void P2PInvalidationService::SendInvalidation( const syncer::ObjectIdSet& ids) { invalidator_->SendInvalidation(ids); diff --git a/chrome/browser/invalidation/p2p_invalidation_service.h b/chrome/browser/invalidation/p2p_invalidation_service.h index 8a6e3c0..9e6c023 100644 --- a/chrome/browser/invalidation/p2p_invalidation_service.h +++ b/chrome/browser/invalidation/p2p_invalidation_service.h @@ -5,7 +5,6 @@ #include "base/threading/non_thread_safe.h" #include "chrome/browser/invalidation/invalidation_service.h" #include "components/browser_context_keyed_service/browser_context_keyed_service.h" -#include "sync/notifier/object_id_invalidation_map.h" #ifndef CHROME_BROWSER_INVALIDATION_P2P_INVALIDATION_SERVICE_H_ #define CHROME_BROWSER_INVALIDATION_P2P_INVALIDATION_SERVICE_H_ @@ -40,9 +39,6 @@ class P2PInvalidationService const syncer::ObjectIdSet& ids) OVERRIDE; virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; - virtual void AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) OVERRIDE; virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; diff --git a/chrome/browser/invalidation/ticl_invalidation_service.cc b/chrome/browser/invalidation/ticl_invalidation_service.cc index f4cae8b..f5b053f 100644 --- a/chrome/browser/invalidation/ticl_invalidation_service.cc +++ b/chrome/browser/invalidation/ticl_invalidation_service.cc @@ -133,15 +133,6 @@ void TiclInvalidationService::UnregisterInvalidationHandler( } } -void TiclInvalidationService::AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) { - DCHECK(CalledOnValidThread()); - if (invalidator_) { - invalidator_->Acknowledge(id, ack_handle); - } -} - syncer::InvalidatorState TiclInvalidationService::GetInvalidatorState() const { DCHECK(CalledOnValidThread()); if (invalidator_) { @@ -360,7 +351,7 @@ void TiclInvalidationService::StartInvalidator() { invalidator_.reset(new syncer::NonBlockingInvalidator( options, invalidator_storage_->GetInvalidatorClientId(), - invalidator_storage_->GetAllInvalidationStates(), + invalidator_storage_->GetSavedInvalidations(), invalidator_storage_->GetBootstrapData(), syncer::WeakHandle<syncer::InvalidationStateTracker>( invalidator_storage_->AsWeakPtr()), diff --git a/chrome/browser/invalidation/ticl_invalidation_service.h b/chrome/browser/invalidation/ticl_invalidation_service.h index 26c5039..6231d71 100644 --- a/chrome/browser/invalidation/ticl_invalidation_service.h +++ b/chrome/browser/invalidation/ticl_invalidation_service.h @@ -54,9 +54,6 @@ class TiclInvalidationService const syncer::ObjectIdSet& ids) OVERRIDE; virtual void UnregisterInvalidationHandler( syncer::InvalidationHandler* handler) OVERRIDE; - virtual void AcknowledgeInvalidation( - const invalidation::ObjectId& id, - const syncer::AckHandle& ack_handle) OVERRIDE; virtual syncer::InvalidatorState GetInvalidatorState() const OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; diff --git a/chrome/browser/policy/cloud/DEPS b/chrome/browser/policy/cloud/DEPS index 01fe65b..788d27a 100644 --- a/chrome/browser/policy/cloud/DEPS +++ b/chrome/browser/policy/cloud/DEPS @@ -37,6 +37,10 @@ specific_include_rules = { "+sync/internal_api/public/base/invalidation.h", ], + r"cloud_policy_invalidator.h": [ + "+sync/internal_api/public/base/invalidation.h", + ], + # TODO(joaodasilva): remove these exceptions. r"cloud_policy_invalidator\.cc": [ "+chrome/browser/invalidation/invalidation_service.h", diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator.cc b/chrome/browser/policy/cloud/cloud_policy_invalidator.cc index b71b934..733b157 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator.cc +++ b/chrome/browser/policy/cloud/cloud_policy_invalidator.cc @@ -42,7 +42,6 @@ CloudPolicyInvalidator::CloudPolicyInvalidator( invalid_(false), invalidation_version_(0), unknown_version_invalidation_count_(0), - ack_handle_(syncer::AckHandle::InvalidAckHandle()), weak_factory_(this), max_fetch_delay_(kMaxFetchDelayDefault), policy_hash_value_(0) { @@ -98,6 +97,16 @@ void CloudPolicyInvalidator::OnIncomingInvalidation( NOTREACHED(); return; } + + // Acknowledge all except the invalidation with the highest version. + syncer::SingleObjectInvalidationSet::const_reverse_iterator it = + list.rbegin(); + ++it; + for ( ; it != list.rend(); ++it) { + it->Acknowledge(); + } + + // Handle the highest version invalidation. HandleInvalidation(list.back()); } @@ -154,10 +163,12 @@ void CloudPolicyInvalidator::OnStoreError(CloudPolicyStore* store) {} void CloudPolicyInvalidator::HandleInvalidation( const syncer::Invalidation& invalidation) { - // The invalidation service may send an invalidation more than once if there - // is a delay in acknowledging it. Duplicate invalidations are ignored. - if (invalid_ && ack_handle_.Equals(invalidation.ack_handle())) + // Ignore old invalidations. + if (invalid_ && + !invalidation.is_unknown_version() && + invalidation.version() <= invalidation_version_) { return; + } // If there is still a pending invalidation, acknowledge it, since we only // care about the latest invalidation. @@ -166,7 +177,7 @@ void CloudPolicyInvalidator::HandleInvalidation( // Update invalidation state. invalid_ = true; - ack_handle_ = invalidation.ack_handle(); + invalidation_.reset(new syncer::Invalidation(invalidation)); // When an invalidation with unknown version is received, use negative // numbers based on the number of such invalidations received. This @@ -322,7 +333,8 @@ void CloudPolicyInvalidator::AcknowledgeInvalidation() { DCHECK(invalid_); invalid_ = false; core_->client()->SetInvalidationInfo(0, std::string()); - invalidation_service_->AcknowledgeInvalidation(object_id_, ack_handle_); + invalidation_->Acknowledge(); + invalidation_.reset(); // Cancel any scheduled policy refreshes. weak_factory_.InvalidateWeakPtrs(); } diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator.h b/chrome/browser/policy/cloud/cloud_policy_invalidator.h index 5ad66db..a007a2d 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator.h +++ b/chrome/browser/policy/cloud/cloud_policy_invalidator.h @@ -10,10 +10,13 @@ #include "base/basictypes.h" #include "base/callback.h" #include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/threading/thread_checker.h" #include "chrome/browser/policy/cloud/cloud_policy_core.h" #include "chrome/browser/policy/cloud/cloud_policy_store.h" +#include "google/cacheinvalidation/include/types.h" +#include "sync/internal_api/public/base/invalidation.h" #include "sync/notifier/invalidation_handler.h" namespace base { @@ -165,8 +168,8 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler, // invalidation_version_ when such invalidations occur. int unknown_version_invalidation_count_; - // The acknowledgment handle for the current invalidation. - syncer::AckHandle ack_handle_; + // The most up to date invalidation. + scoped_ptr<syncer::Invalidation> invalidation_; // WeakPtrFactory used to create callbacks to this object. base::WeakPtrFactory<CloudPolicyInvalidator> weak_factory_; diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc index 662d847..adccdb1 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc @@ -107,17 +107,15 @@ class CloudPolicyInvalidatorTest : public testing::Test { // Enables the invalidation service. It is enabled by default. void EnableInvalidationService(); - // Causes the invalidation service to fire an invalidation. Returns an ack - // handle which be used to verify that the invalidation was acknowledged. - syncer::AckHandle FireInvalidation( + // Causes the invalidation service to fire an invalidation. + syncer::Invalidation FireInvalidation( PolicyObject object, int64 version, const std::string& payload); // Causes the invalidation service to fire an invalidation with unknown - // version. Returns an ack handle which be used to verify that the - // invalidation was acknowledged. - syncer::AckHandle FireUnknownVersionInvalidation(PolicyObject object); + // version. + syncer::Invalidation FireUnknownVersionInvalidation(PolicyObject object); // Checks the expected value of the currently set invalidation info. bool CheckInvalidationInfo(int64 version, const std::string& payload); @@ -131,13 +129,15 @@ class CloudPolicyInvalidatorTest : public testing::Test { bool CheckPolicyRefreshed(); bool CheckPolicyRefreshedWithUnknownVersion(); + bool IsUnsent(const syncer::Invalidation& invalidation); + // Returns the invalidations enabled state set by the invalidator on the // refresh scheduler. bool InvalidationsEnabled(); // Determines if the invalidation with the given ack handle has been // acknowledged. - bool IsInvalidationAcknowledged(const syncer::AckHandle& ack_handle); + bool IsInvalidationAcknowledged(const syncer::Invalidation& invalidation); // Determines if the invalidator has registered for an object with the // invalidation service. @@ -217,7 +217,6 @@ void CloudPolicyInvalidatorTest::SetUp() { } void CloudPolicyInvalidatorTest::TearDown() { - EXPECT_FALSE(invalidation_service_.ReceivedInvalidAcknowledgement()); if (invalidator_) invalidator_->Shutdown(); core_.Disconnect(); @@ -301,7 +300,7 @@ void CloudPolicyInvalidatorTest::EnableInvalidationService() { invalidation_service_.SetInvalidatorState(syncer::INVALIDATIONS_ENABLED); } -syncer::AckHandle CloudPolicyInvalidatorTest::FireInvalidation( +syncer::Invalidation CloudPolicyInvalidatorTest::FireInvalidation( PolicyObject object, int64 version, const std::string& payload) { @@ -310,15 +309,15 @@ syncer::AckHandle CloudPolicyInvalidatorTest::FireInvalidation( version, payload); invalidation_service_.EmitInvalidationForTest(invalidation); - return invalidation.ack_handle(); + return invalidation; } -syncer::AckHandle CloudPolicyInvalidatorTest::FireUnknownVersionInvalidation( +syncer::Invalidation CloudPolicyInvalidatorTest::FireUnknownVersionInvalidation( PolicyObject object) { - syncer::Invalidation invalidation = - syncer::Invalidation::InitUnknownVersion(GetPolicyObjectId(object)); + syncer::Invalidation invalidation = syncer::Invalidation::InitUnknownVersion( + GetPolicyObjectId(object)); invalidation_service_.EmitInvalidationForTest(invalidation); - return invalidation.ack_handle(); + return invalidation; } bool CloudPolicyInvalidatorTest::CheckInvalidationInfo( @@ -338,6 +337,11 @@ bool CloudPolicyInvalidatorTest::CheckPolicyRefreshed() { return CheckPolicyRefreshed(base::TimeDelta()); } +bool CloudPolicyInvalidatorTest::IsUnsent( + const syncer::Invalidation& invalidation) { + return invalidation_service_.GetMockAckHandler()->IsUnsent(invalidation); +} + bool CloudPolicyInvalidatorTest::CheckPolicyRefreshedWithUnknownVersion() { return CheckPolicyRefreshed(base::TimeDelta::FromMinutes( CloudPolicyInvalidator::kMissingPayloadDelay)); @@ -348,8 +352,14 @@ bool CloudPolicyInvalidatorTest::InvalidationsEnabled() { } bool CloudPolicyInvalidatorTest::IsInvalidationAcknowledged( - const syncer::AckHandle& ack_handle) { - return invalidation_service_.IsInvalidationAcknowledged(ack_handle); + const syncer::Invalidation& invalidation) { + // The acknowledgement task is run through a WeakHandle that posts back to our + // own thread. We need to run any posted tasks before we can check + // acknowledgement status. + loop_.RunUntilIdle(); + + EXPECT_FALSE(IsUnsent(invalidation)); + return !invalidation_service_.GetMockAckHandler()->IsUnacked(invalidation); } bool CloudPolicyInvalidatorTest::IsInvalidatorRegistered() { @@ -422,7 +432,7 @@ TEST_F(CloudPolicyInvalidatorTest, Uninitialized) { StartInvalidator(false /* initialize */, true /* start_refresh_scheduler */); StorePolicy(POLICY_OBJECT_A); EXPECT_FALSE(IsInvalidatorRegistered()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); EXPECT_TRUE(CheckPolicyNotRefreshed()); } @@ -432,7 +442,7 @@ TEST_F(CloudPolicyInvalidatorTest, RefreshSchedulerNotStarted) { StartInvalidator(true /* initialize */, false /* start_refresh_scheduler */); StorePolicy(POLICY_OBJECT_A); EXPECT_FALSE(IsInvalidatorRegistered()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); EXPECT_TRUE(CheckPolicyNotRefreshed()); } @@ -444,7 +454,7 @@ TEST_F(CloudPolicyInvalidatorTest, DisconnectCoreThenInitialize) { InitializeInvalidator(); StorePolicy(POLICY_OBJECT_A); EXPECT_FALSE(IsInvalidatorRegistered()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); EXPECT_TRUE(CheckPolicyNotRefreshed()); } @@ -468,16 +478,16 @@ TEST_F(CloudPolicyInvalidatorTest, RegisterOnStoreLoaded) { StartInvalidator(); EXPECT_FALSE(IsInvalidatorRegistered()); EXPECT_FALSE(InvalidationsEnabled()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); EXPECT_TRUE(CheckPolicyNotRefreshed()); // No registration when store is loaded with no invalidation object id. StorePolicy(POLICY_OBJECT_NONE); EXPECT_FALSE(IsInvalidatorRegistered()); EXPECT_FALSE(InvalidationsEnabled()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); EXPECT_TRUE(CheckPolicyNotRefreshed()); // Check registration when store is loaded for object A. @@ -486,7 +496,7 @@ TEST_F(CloudPolicyInvalidatorTest, RegisterOnStoreLoaded) { EXPECT_TRUE(InvalidationsEnabled()); FireUnknownVersionInvalidation(POLICY_OBJECT_A); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); EXPECT_TRUE(CheckPolicyNotRefreshed()); } @@ -498,21 +508,21 @@ TEST_F(CloudPolicyInvalidatorTest, ChangeRegistration) { EXPECT_TRUE(InvalidationsEnabled()); FireUnknownVersionInvalidation(POLICY_OBJECT_A); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); EXPECT_TRUE(CheckPolicyNotRefreshed()); - syncer::AckHandle ack = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + syncer::Invalidation inv = FireUnknownVersionInvalidation(POLICY_OBJECT_A); // Check re-registration for object B. Make sure the pending invalidation for // object A is acknowledged without making the callback. StorePolicy(POLICY_OBJECT_B); EXPECT_TRUE(IsInvalidatorRegistered()); EXPECT_TRUE(InvalidationsEnabled()); - EXPECT_TRUE(IsInvalidationAcknowledged(ack)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv)); EXPECT_TRUE(CheckPolicyNotRefreshed()); // Make sure future invalidations for object A are ignored and for object B // are processed. - FireUnknownVersionInvalidation(POLICY_OBJECT_A); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); EXPECT_TRUE(CheckPolicyNotRefreshed()); FireUnknownVersionInvalidation(POLICY_OBJECT_B); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); @@ -528,14 +538,14 @@ TEST_F(CloudPolicyInvalidatorTest, UnregisterOnStoreLoaded) { EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); // Check unregistration when store is loaded with no invalidation object id. - syncer::AckHandle ack = FireUnknownVersionInvalidation(POLICY_OBJECT_A); - EXPECT_FALSE(IsInvalidationAcknowledged(ack)); + syncer::Invalidation inv = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + EXPECT_FALSE(IsInvalidationAcknowledged(inv)); StorePolicy(POLICY_OBJECT_NONE); EXPECT_FALSE(IsInvalidatorRegistered()); - EXPECT_TRUE(IsInvalidationAcknowledged(ack)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv)); EXPECT_FALSE(InvalidationsEnabled()); - FireUnknownVersionInvalidation(POLICY_OBJECT_A); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_A))); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); EXPECT_TRUE(CheckPolicyNotRefreshed()); // Check re-registration for object B. @@ -551,17 +561,18 @@ TEST_F(CloudPolicyInvalidatorTest, HandleInvalidation) { StorePolicy(POLICY_OBJECT_A); StartInvalidator(); EXPECT_TRUE(InvalidationsEnabled()); - syncer::AckHandle ack = FireInvalidation(POLICY_OBJECT_A, 12, "test_payload"); + syncer::Invalidation inv = + FireInvalidation(POLICY_OBJECT_A, 12, "test_payload"); // Make sure client info is set as soon as the invalidation is received. EXPECT_TRUE(CheckInvalidationInfo(12, "test_payload")); EXPECT_TRUE(CheckPolicyRefreshed()); // Make sure invalidation is not acknowledged until the store is loaded. - EXPECT_FALSE(IsInvalidationAcknowledged(ack)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv)); EXPECT_TRUE(CheckInvalidationInfo(12, "test_payload")); StorePolicy(POLICY_OBJECT_A, 12); - EXPECT_TRUE(IsInvalidationAcknowledged(ack)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv)); EXPECT_TRUE(CheckInvalidationInfo(0, std::string())); } @@ -569,7 +580,7 @@ TEST_F(CloudPolicyInvalidatorTest, HandleInvalidationWithUnknownVersion) { // Register and fire invalidation with unknown version. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + syncer::Invalidation inv = FireUnknownVersionInvalidation(POLICY_OBJECT_A); // Make sure client info is not set until after the invalidation callback is // made. @@ -578,9 +589,9 @@ TEST_F(CloudPolicyInvalidatorTest, HandleInvalidationWithUnknownVersion) { EXPECT_TRUE(CheckInvalidationInfo(-1, std::string())); // Make sure invalidation is not acknowledged until the store is loaded. - EXPECT_FALSE(IsInvalidationAcknowledged(ack)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv)); StorePolicy(POLICY_OBJECT_A, -1); - EXPECT_TRUE(IsInvalidationAcknowledged(ack)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv)); EXPECT_TRUE(CheckInvalidationInfo(0, std::string())); } @@ -588,16 +599,16 @@ TEST_F(CloudPolicyInvalidatorTest, HandleMultipleInvalidations) { // Generate multiple invalidations. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack1 = FireInvalidation(POLICY_OBJECT_A, 1, "test1"); + syncer::Invalidation inv1 = FireInvalidation(POLICY_OBJECT_A, 1, "test1"); EXPECT_TRUE(CheckInvalidationInfo(1, "test1")); - syncer::AckHandle ack2 = FireInvalidation(POLICY_OBJECT_A, 2, "test2"); + syncer::Invalidation inv2 = FireInvalidation(POLICY_OBJECT_A, 2, "test2"); EXPECT_TRUE(CheckInvalidationInfo(2, "test2")); - syncer::AckHandle ack3= FireInvalidation(POLICY_OBJECT_A, 3, "test3"); + syncer::Invalidation inv3 = FireInvalidation(POLICY_OBJECT_A, 3, "test3"); EXPECT_TRUE(CheckInvalidationInfo(3, "test3")); // Make sure the replaced invalidations are acknowledged. - EXPECT_TRUE(IsInvalidationAcknowledged(ack1)); - EXPECT_TRUE(IsInvalidationAcknowledged(ack2)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv1)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv2)); // Make sure the policy is refreshed once. EXPECT_TRUE(CheckPolicyRefreshed()); @@ -605,11 +616,11 @@ TEST_F(CloudPolicyInvalidatorTest, HandleMultipleInvalidations) { // Make sure that the last invalidation is only acknowledged after the store // is loaded with the latest version. StorePolicy(POLICY_OBJECT_A, 1); - EXPECT_FALSE(IsInvalidationAcknowledged(ack3)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv3)); StorePolicy(POLICY_OBJECT_A, 2); - EXPECT_FALSE(IsInvalidationAcknowledged(ack3)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv3)); StorePolicy(POLICY_OBJECT_A, 3); - EXPECT_TRUE(IsInvalidationAcknowledged(ack3)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv3)); } TEST_F(CloudPolicyInvalidatorTest, @@ -618,44 +629,44 @@ TEST_F(CloudPolicyInvalidatorTest, // unique invalidation version numbers. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack1 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + syncer::Invalidation inv1 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); EXPECT_TRUE(CheckInvalidationInfo(0, std::string())); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); EXPECT_TRUE(CheckInvalidationInfo(-1, std::string())); - syncer::AckHandle ack2 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + syncer::Invalidation inv2 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); EXPECT_TRUE(CheckInvalidationInfo(0, std::string())); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); EXPECT_TRUE(CheckInvalidationInfo(-2, std::string())); - syncer::AckHandle ack3 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); + syncer::Invalidation inv3 = FireUnknownVersionInvalidation(POLICY_OBJECT_A); EXPECT_TRUE(CheckInvalidationInfo(0, std::string())); EXPECT_TRUE(CheckPolicyRefreshedWithUnknownVersion()); EXPECT_TRUE(CheckInvalidationInfo(-3, std::string())); // Make sure the replaced invalidations are acknowledged. - EXPECT_TRUE(IsInvalidationAcknowledged(ack1)); - EXPECT_TRUE(IsInvalidationAcknowledged(ack2)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv1)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv2)); // Make sure that the last invalidation is only acknowledged after the store // is loaded with the last unknown version. StorePolicy(POLICY_OBJECT_A, -1); - EXPECT_FALSE(IsInvalidationAcknowledged(ack3)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv3)); StorePolicy(POLICY_OBJECT_A, -2); - EXPECT_FALSE(IsInvalidationAcknowledged(ack3)); + EXPECT_FALSE(IsInvalidationAcknowledged(inv3)); StorePolicy(POLICY_OBJECT_A, -3); - EXPECT_TRUE(IsInvalidationAcknowledged(ack3)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv3)); } TEST_F(CloudPolicyInvalidatorTest, AcknowledgeBeforeRefresh) { // Generate an invalidation. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack = FireInvalidation(POLICY_OBJECT_A, 3, "test"); + syncer::Invalidation inv = FireInvalidation(POLICY_OBJECT_A, 3, "test"); // Ensure that the policy is not refreshed and the invalidation is // acknowledged if the store is loaded with the latest version before the // refresh can occur. StorePolicy(POLICY_OBJECT_A, 3); - EXPECT_TRUE(IsInvalidationAcknowledged(ack)); + EXPECT_TRUE(IsInvalidationAcknowledged(inv)); EXPECT_TRUE(CheckPolicyNotRefreshed()); } @@ -663,7 +674,7 @@ TEST_F(CloudPolicyInvalidatorTest, NoCallbackAfterShutdown) { // Generate an invalidation. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack = FireInvalidation(POLICY_OBJECT_A, 3, "test"); + syncer::Invalidation inv = FireInvalidation(POLICY_OBJECT_A, 3, "test"); // Ensure that the policy refresh is not made after the invalidator is shut // down. @@ -713,7 +724,7 @@ TEST_F(CloudPolicyInvalidatorTest, Disconnect) { // Generate an invalidation. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - syncer::AckHandle ack = FireInvalidation(POLICY_OBJECT_A, 1, "test"); + syncer::Invalidation inv = FireInvalidation(POLICY_OBJECT_A, 1, "test"); EXPECT_TRUE(InvalidationsEnabled()); // Ensure that the policy is not refreshed after disconnecting the core, but @@ -723,17 +734,17 @@ TEST_F(CloudPolicyInvalidatorTest, Disconnect) { // Ensure that invalidation service events do not cause refreshes while the // invalidator is stopped. - FireInvalidation(POLICY_OBJECT_A, 2, "test"); + EXPECT_TRUE(IsUnsent(FireInvalidation(POLICY_OBJECT_A, 2, "test"))); EXPECT_TRUE(CheckPolicyNotRefreshed()); DisableInvalidationService(); EnableInvalidationService(); // Connect and disconnect without starting the refresh scheduler. ConnectCore(); - FireInvalidation(POLICY_OBJECT_A, 3, "test"); + EXPECT_TRUE(IsUnsent(FireInvalidation(POLICY_OBJECT_A, 3, "test"))); EXPECT_TRUE(CheckPolicyNotRefreshed()); DisconnectCore(); - FireInvalidation(POLICY_OBJECT_A, 4, "test"); + EXPECT_TRUE(IsUnsent(FireInvalidation(POLICY_OBJECT_A, 4, "test"))); EXPECT_TRUE(CheckPolicyNotRefreshed()); // Ensure that the invalidator returns to normal after reconnecting. @@ -832,9 +843,9 @@ TEST_F(CloudPolicyInvalidatorTest, InvalidationMetrics) { // Generate a mix of versioned and unknown-version invalidations. StorePolicy(POLICY_OBJECT_A); StartInvalidator(); - FireUnknownVersionInvalidation(POLICY_OBJECT_B); + EXPECT_TRUE(IsUnsent(FireUnknownVersionInvalidation(POLICY_OBJECT_B))); FireUnknownVersionInvalidation(POLICY_OBJECT_A); - FireInvalidation(POLICY_OBJECT_B, 1, "test"); + EXPECT_TRUE(IsUnsent(FireInvalidation(POLICY_OBJECT_B, 1, "test"))); FireInvalidation(POLICY_OBJECT_A, 1, "test"); FireInvalidation(POLICY_OBJECT_A, 2, "test"); FireUnknownVersionInvalidation(POLICY_OBJECT_A); diff --git a/chrome/browser/sync/glue/sync_backend_host_core.cc b/chrome/browser/sync/glue/sync_backend_host_core.cc index 50a8159..792f43b 100644 --- a/chrome/browser/sync/glue/sync_backend_host_core.cc +++ b/chrome/browser/sync/glue/sync_backend_host_core.cc @@ -324,7 +324,7 @@ void SyncBackendHostCore::DoOnInvalidatorStateChange( } void SyncBackendHostCore::DoOnIncomingInvalidation( - syncer::ObjectIdInvalidationMap invalidation_map) { + const syncer::ObjectIdInvalidationMap& invalidation_map) { DCHECK_EQ(base::MessageLoop::current(), sync_loop_); sync_manager_->OnIncomingInvalidation(invalidation_map); } diff --git a/chrome/browser/sync/glue/sync_backend_host_core.h b/chrome/browser/sync/glue/sync_backend_host_core.h index 4443972..ed2aefe 100644 --- a/chrome/browser/sync/glue/sync_backend_host_core.h +++ b/chrome/browser/sync/glue/sync_backend_host_core.h @@ -125,7 +125,7 @@ class SyncBackendHostCore // Forwards an invalidation to the sync manager. void DoOnIncomingInvalidation( - syncer::ObjectIdInvalidationMap invalidation_map); + const syncer::ObjectIdInvalidationMap& invalidation_map); // Note: // diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.cc b/chrome/browser/sync/glue/sync_backend_host_impl.cc index f9e7ff4..a82b2fa 100644 --- a/chrome/browser/sync/glue/sync_backend_host_impl.cc +++ b/chrome/browser/sync/glue/sync_backend_host_impl.cc @@ -26,6 +26,7 @@ #include "sync/internal_api/public/sync_manager_factory.h" #include "sync/internal_api/public/util/experiments.h" #include "sync/internal_api/public/util/sync_string_conversions.h" +#include "sync/notifier/object_id_invalidation_map.h" // Helper macros to log with the syncer thread name; useful when there // are multiple syncers involved. @@ -652,14 +653,7 @@ void SyncBackendHostImpl::OnIncomingInvalidation( const syncer::ObjectIdInvalidationMap& invalidation_map) { // TODO(rlarocque): Acknowledge these invalidations only after the syncer has // acted on them and saved the results to disk. - syncer::ObjectIdSet ids = invalidation_map.GetObjectIds(); - for (syncer::ObjectIdSet::const_iterator it = ids.begin(); - it != ids.end(); ++it) { - const syncer::AckHandle& handle = - invalidation_map.ForObject(*it).back().ack_handle(); - invalidator_->AcknowledgeInvalidation(*it, handle); - } - + invalidation_map.AcknowledgeAll(); registrar_->sync_thread()->message_loop()->PostTask( FROM_HERE, base::Bind(&SyncBackendHostCore::DoOnIncomingInvalidation, diff --git a/chrome/browser/sync/glue/sync_backend_host_impl.h b/chrome/browser/sync/glue/sync_backend_host_impl.h index 5d91209..8f819c6 100644 --- a/chrome/browser/sync/glue/sync_backend_host_impl.h +++ b/chrome/browser/sync/glue/sync_backend_host_impl.h @@ -244,12 +244,6 @@ class SyncBackendHostImpl void HandleConnectionStatusChangeOnFrontendLoop( syncer::ConnectionStatus status); - // syncer::InvalidationHandler-like functions. - void HandleInvalidatorStateChangeOnFrontendLoop( - syncer::InvalidatorState state); - void HandleIncomingInvalidationOnFrontendLoop( - const syncer::ObjectIdInvalidationMap& invalidation_map); - // NotificationObserver implementation. virtual void Observe( int type, diff --git a/chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc index 217ed04..609e20f 100644 --- a/chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc @@ -62,10 +62,6 @@ class MockSyncFrontend : public SyncFrontend { public: virtual ~MockSyncFrontend() {} - MOCK_METHOD1(OnInvalidatorStateChange, - void(syncer::InvalidatorState)); - MOCK_METHOD1(OnIncomingInvalidation, - void(const syncer::ObjectIdInvalidationMap&)); MOCK_METHOD3( OnBackendInitialized, void(const syncer::WeakHandle<syncer::JsBackend>&, diff --git a/chrome/browser/sync/profile_sync_service_android.cc b/chrome/browser/sync/profile_sync_service_android.cc index b1f82f7..ce904fe 100644 --- a/chrome/browser/sync/profile_sync_service_android.cc +++ b/chrome/browser/sync/profile_sync_service_android.cc @@ -33,6 +33,7 @@ #include "grit/generated_resources.h" #include "jni/ProfileSyncService_jni.h" #include "sync/internal_api/public/read_transaction.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "ui/base/l10n/l10n_util.h" using base::android::AttachCurrentThread; diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index edc5a0a..ce48aa0 100644 --- a/chrome/common/pref_names.cc +++ b/chrome/common/pref_names.cc @@ -1853,10 +1853,6 @@ const char kSyncSuppressStart[] = "sync.suppress_start"; // if a new sync type has rolled out so we can notify the user. const char kSyncAcknowledgedSyncTypes[] = "sync.acknowledged_types"; -// Dictionary from sync model type (as an int) to max invalidation -// version (int64 represented as a string). -const char kSyncMaxInvalidationVersions[] = "sync.max_invalidation_versions"; - // The GUID session sync will use to identify this client, even across sync // disable/enable events. const char kSyncSessionsGUID[] = "sync.session_sync_guid"; @@ -1868,11 +1864,9 @@ const char kInvalidatorClientId[] = "invalidator.client_id"; // The value is base 64 encoded. const char kInvalidatorInvalidationState[] = "invalidator.invalidation_state"; -// List of {source, name, max invalidation version} tuples. source is an int, -// while max invalidation version is an int64; both are stored as string -// representations though. -const char kInvalidatorMaxInvalidationVersions[] = - "invalidator.max_invalidation_versions"; +// List of received invalidations that have not been acted on by any clients +// yet. Used to keep invalidation clients in sync in case of a restart. +const char kInvalidatorSavedInvalidations[] = "invalidator.saved_invalidations"; // A string that can be used to restore sync encryption infrastructure on // startup so that the user doesn't need to provide credentials on each start. diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index 164ac2c..6cab9bc 100644 --- a/chrome/common/pref_names.h +++ b/chrome/common/pref_names.h @@ -642,13 +642,11 @@ extern const char kSyncUsingSecondaryPassphrase[]; extern const char kSyncEncryptionBootstrapToken[]; extern const char kSyncKeystoreEncryptionBootstrapToken[]; extern const char kSyncAcknowledgedSyncTypes[]; -// Deprecated in favor of kInvalidatorMaxInvalidationVersions. -extern const char kSyncMaxInvalidationVersions[]; extern const char kSyncSessionsGUID[]; extern const char kInvalidatorClientId[]; extern const char kInvalidatorInvalidationState[]; -extern const char kInvalidatorMaxInvalidationVersions[]; +extern const char kInvalidatorSavedInvalidations[]; extern const char kSignInPromoStartupCount[]; extern const char kSignInPromoUserSkipped[]; diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h index c613ec9..aef0118 100644 --- a/sync/engine/sync_scheduler.h +++ b/sync/engine/sync_scheduler.h @@ -13,7 +13,6 @@ #include "base/time/time.h" #include "sync/base/sync_export.h" #include "sync/engine/nudge_source.h" -#include "sync/notifier/object_id_invalidation_map.h" #include "sync/sessions/sync_session.h" namespace tracked_objects { @@ -22,6 +21,7 @@ class Location; namespace syncer { +class ObjectIdInvalidationMap; struct ServerConnectionEvent; struct SYNC_EXPORT_PRIVATE ConfigurationParams { diff --git a/sync/internal_api/public/base/invalidation.cc b/sync/internal_api/public/base/invalidation.cc index 1292a51..ff7a5a7 100644 --- a/sync/internal_api/public/base/invalidation.cc +++ b/sync/internal_api/public/base/invalidation.cc @@ -111,10 +111,6 @@ const AckHandle& Invalidation::ack_handle() const { return ack_handle_; } -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; } @@ -135,10 +131,12 @@ void Invalidation::Acknowledge() const { void Invalidation::Drop(DroppedInvalidationTracker* tracker) const { DCHECK(tracker->object_id() == object_id()); tracker->RecordDropEvent(ack_handler_, ack_handle_); - ack_handler_.Call(FROM_HERE, - &AckHandler::Drop, - id_, - ack_handle_); + if (SupportsAcknowledgement()) { + ack_handler_.Call(FROM_HERE, + &AckHandler::Drop, + id_, + ack_handle_); + } } bool Invalidation::Equals(const Invalidation& other) const { diff --git a/sync/internal_api/public/base/invalidation.h b/sync/internal_api/public/base/invalidation.h index 472f552..cf26112 100644 --- a/sync/internal_api/public/base/invalidation.h +++ b/sync/internal_api/public/base/invalidation.h @@ -51,20 +51,43 @@ class SYNC_EXPORT Invalidation { const AckHandle& ack_handle() const; - // TODO(rlarocque): Remove this method and use AckHandlers instead. - void set_ack_handle(const AckHandle& ack_handle); - - // Functions from the alternative ack tracking framework. - // Currently unused. + // Sets the AckHandler to be used to track this Invalidation. + // + // This should be set by the class that generates the invalidation. Clients + // of the Invalidations API should not need to call this. + // + // Note that some sources of invalidations do not support ack tracking, and do + // not set the ack_handler. This will be hidden from users of this class. void set_ack_handler(syncer::WeakHandle<AckHandler> ack_handler); + + // Returns whether or not this instance supports ack tracking. This will + // depend on whether or not the source of invaliadations supports + // invalidations. + // + // Clients can safely ignore this flag. They can assume that all + // invalidations support ack tracking. If they're wrong, then invalidations + // will be less reliable, but their behavior will be no less correct. bool SupportsAcknowledgement() const; + + // Acknowledges the receipt of this invalidation. + // + // Clients should call this on a received invalidation when they have fully + // processed the invalidation and persisted the results to disk. Once this + // function is called, the invalidations system is under no obligation to + // re-deliver this invalidation in the event of a crash or restart. void Acknowledge() const; - // Drops an invalidation. + // Informs the ack tracker that this invalidation will not be serviced. + // + // If a client's buffer reaches its limit and it is forced to start dropping + // invalidations, it should call this function before dropping its + // invalidations in order to allow the ack tracker to drop the invalidation, + // too. // // The drop record will be tracked by the specified // DroppedInvalidationTracker. The caller should hang on to this tracker. It - // will need to use it when it recovers from this drop event. See the + // will need to use it when it recovers from this drop event, or if it needs + // to record another drop event for the same ObjectID. Refer to the // documentation of DroppedInvalidationTracker for more details. void Drop(DroppedInvalidationTracker* tracker) const; diff --git a/sync/internal_api/public/base/model_type_test_util.cc b/sync/internal_api/public/base/model_type_test_util.cc index efad29c1..d9621bb 100644 --- a/sync/internal_api/public/base/model_type_test_util.cc +++ b/sync/internal_api/public/base/model_type_test_util.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "sync/internal_api/public/base/model_type_test_util.h" +#include "sync/internal_api/public/base/ack_handle.h" namespace syncer { diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 4dc8860..7e2d34b 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -40,6 +40,7 @@ #include "sync/js/js_reply_handler.h" #include "sync/notifier/invalidation_util.h" #include "sync/notifier/invalidator.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "sync/protocol/proto_value_conversions.h" #include "sync/protocol/sync.pb.h" #include "sync/syncable/directory.h" @@ -933,8 +934,8 @@ void SyncManagerImpl::OnSyncEngineEvent(const SyncEngineEvent& event) { // whether we should sync again. if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) { if (!initialized_) { - LOG(INFO) << "OnSyncCycleCompleted not sent because sync api is not " - << "initialized"; + DVLOG(1) << "OnSyncCycleCompleted not sent because sync api is not " + << "initialized"; return; } diff --git a/sync/notifier/ack_tracker.cc b/sync/notifier/ack_tracker.cc deleted file mode 100644 index 6461571..0000000 --- a/sync/notifier/ack_tracker.cc +++ /dev/null @@ -1,221 +0,0 @@ -// 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/notifier/ack_tracker.h" - -#include <algorithm> -#include <iterator> -#include <utility> - -#include "base/callback.h" -#include "base/stl_util.h" -#include "base/time/tick_clock.h" -#include "google/cacheinvalidation/include/types.h" - -namespace syncer { - -namespace { - -// All times are in milliseconds. -const net::BackoffEntry::Policy kDefaultBackoffPolicy = { - // Number of initial errors (in sequence) to ignore before applying - // exponential back-off rules. - // Note this value is set to 1 to work in conjunction with a hack in - // AckTracker::Track. - 1, - - // Initial delay. The interpretation of this value depends on - // always_use_initial_delay. It's either how long we wait between - // requests before backoff starts, or how much we delay the first request - // after backoff starts. - 60 * 1000, - - // Factor by which the waiting time will be multiplied. - 2, - - // Fuzzing percentage. ex: 10% will spread requests randomly - // between 90%-100% of the calculated time. - 0, - - // Maximum amount of time we are willing to delay our request, -1 - // for no maximum. - 60 * 10 * 1000, - - // Time to keep an entry from being discarded even when it - // has no significant state, -1 to never discard. - -1, - - // If true, we always use a delay of initial_delay_ms, even before - // we've seen num_errors_to_ignore errors. Otherwise, initial_delay_ms - // is the first delay once we start exponential backoff. - // - // So if we're ignoring 1 error, we'll see (N, N, Nm, Nm^2, ...) if true, - // and (0, 0, N, Nm, ...) when false, where N is initial_backoff_ms and - // m is multiply_factor, assuming we've already seen one success. - true, -}; - -scoped_ptr<net::BackoffEntry> CreateDefaultBackoffEntry( - const net::BackoffEntry::Policy* const policy) { - return scoped_ptr<net::BackoffEntry>(new net::BackoffEntry(policy)); -} - -} // namespace - -AckTracker::Delegate::~Delegate() { -} - -AckTracker::Entry::Entry(scoped_ptr<net::BackoffEntry> backoff, - const ObjectIdSet& ids) - : backoff(backoff.Pass()), ids(ids) { -} - -AckTracker::Entry::~Entry() { -} - -AckTracker::AckTracker(base::TickClock* tick_clock, Delegate* delegate) - : create_backoff_entry_callback_(base::Bind(&CreateDefaultBackoffEntry)), - tick_clock_(tick_clock), - delegate_(delegate) { - DCHECK(tick_clock_); - DCHECK(delegate_); -} - -AckTracker::~AckTracker() { - DCHECK(thread_checker_.CalledOnValidThread()); - - Clear(); -} - -void AckTracker::Clear() { - DCHECK(thread_checker_.CalledOnValidThread()); - - timer_.Stop(); - STLDeleteValues(&queue_); -} - -void AckTracker::Track(const ObjectIdSet& ids) { - DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(!ids.empty()); - - scoped_ptr<Entry> entry(new Entry( - create_backoff_entry_callback_.Run(&kDefaultBackoffPolicy), ids)); - // This is a small hack. When net::BackoffRequest is first created, - // GetReleaseTime() always returns the default base::TimeTicks value: 0. - // In order to work around that, we mark it as failed right away. - entry->backoff->InformOfRequest(false /* succeeded */); - const base::TimeTicks release_time = entry->backoff->GetReleaseTime(); - queue_.insert(std::make_pair(release_time, entry.release())); - NudgeTimer(); -} - -void AckTracker::Ack(const ObjectIdSet& ids) { - DCHECK(thread_checker_.CalledOnValidThread()); - - // We could be clever and maintain a mapping of object IDs to their position - // in the multimap, but that makes things a lot more complicated. - for (std::multimap<base::TimeTicks, Entry*>::iterator it = queue_.begin(); - it != queue_.end(); ) { - ObjectIdSet remaining_ids; - std::set_difference(it->second->ids.begin(), it->second->ids.end(), - ids.begin(), ids.end(), - std::inserter(remaining_ids, remaining_ids.begin()), - ids.value_comp()); - it->second->ids.swap(remaining_ids); - if (it->second->ids.empty()) { - std::multimap<base::TimeTicks, Entry*>::iterator erase_it = it; - ++it; - delete erase_it->second; - queue_.erase(erase_it); - } else { - ++it; - } - } - NudgeTimer(); -} - -void AckTracker::NudgeTimer() { - DCHECK(thread_checker_.CalledOnValidThread()); - - if (queue_.empty()) { - return; - } - - const base::TimeTicks now = tick_clock_->NowTicks(); - // There are two cases when the timer needs to be started: - // 1. |desired_run_time_| is in the past. By definition, the timer has already - // fired at this point. Since the queue is non-empty, we need to set the - // timer to fire again. - // 2. The timer is already running but we need it to fire sooner if the first - // entry's timeout occurs before |desired_run_time_|. - if (desired_run_time_ <= now || queue_.begin()->first < desired_run_time_) { - base::TimeDelta delay = queue_.begin()->first - now; - if (delay < base::TimeDelta()) { - delay = base::TimeDelta(); - } - timer_.Start(FROM_HERE, delay, this, &AckTracker::OnTimeout); - desired_run_time_ = queue_.begin()->first; - } -} - -void AckTracker::OnTimeout() { - DCHECK(thread_checker_.CalledOnValidThread()); - - OnTimeoutAt(tick_clock_->NowTicks()); -} - -void AckTracker::OnTimeoutAt(base::TimeTicks now) { - DCHECK(thread_checker_.CalledOnValidThread()); - - if (queue_.empty()) - return; - - ObjectIdSet expired_ids; - std::multimap<base::TimeTicks, Entry*>::iterator end = - queue_.upper_bound(now); - std::vector<Entry*> expired_entries; - for (std::multimap<base::TimeTicks, Entry*>::iterator it = queue_.begin(); - it != end; ++it) { - expired_ids.insert(it->second->ids.begin(), it->second->ids.end()); - it->second->backoff->InformOfRequest(false /* succeeded */); - expired_entries.push_back(it->second); - } - queue_.erase(queue_.begin(), end); - for (std::vector<Entry*>::const_iterator it = expired_entries.begin(); - it != expired_entries.end(); ++it) { - queue_.insert(std::make_pair((*it)->backoff->GetReleaseTime(), *it)); - } - delegate_->OnTimeout(expired_ids); - NudgeTimer(); -} - -// Testing helpers. -void AckTracker::SetCreateBackoffEntryCallbackForTest( - const CreateBackoffEntryCallback& create_backoff_entry_callback) { - DCHECK(thread_checker_.CalledOnValidThread()); - - create_backoff_entry_callback_ = create_backoff_entry_callback; -} - -bool AckTracker::TriggerTimeoutAtForTest(base::TimeTicks now) { - DCHECK(thread_checker_.CalledOnValidThread()); - - bool no_timeouts_before_now = (queue_.lower_bound(now) == queue_.begin()); - OnTimeoutAt(now); - return no_timeouts_before_now; -} - -bool AckTracker::IsQueueEmptyForTest() const { - DCHECK(thread_checker_.CalledOnValidThread()); - - return queue_.empty(); -} - -const base::Timer& AckTracker::GetTimerForTest() const { - DCHECK(thread_checker_.CalledOnValidThread()); - - return timer_; -} - -} // namespace syncer diff --git a/sync/notifier/ack_tracker.h b/sync/notifier/ack_tracker.h deleted file mode 100644 index 9f2ca1d..0000000 --- a/sync/notifier/ack_tracker.h +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright 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_NOTIFIER_ACK_TRACKER_H_ -#define SYNC_NOTIFIER_ACK_TRACKER_H_ - -#include <map> - -#include "base/basictypes.h" -#include "base/callback_forward.h" -#include "base/memory/scoped_ptr.h" -#include "base/threading/thread_checker.h" -#include "base/time/time.h" -#include "base/timer/timer.h" -#include "net/base/backoff_entry.h" -#include "sync/base/sync_export.h" -#include "sync/notifier/invalidation_util.h" - -namespace base { -class TickClock; -} // namespace base - -namespace syncer { - -// A simple class that tracks sets of object IDs that have not yet been -// acknowledged. Internally, it manages timeouts for the tracked object IDs and -// periodically triggers a callback for each timeout period. The timeout is a -// simple exponentially increasing time that starts at 60 seconds and is capped -// at 600 seconds. -class SYNC_EXPORT_PRIVATE AckTracker { - public: - class SYNC_EXPORT_PRIVATE Delegate { - public: - virtual ~Delegate(); - - // |ids| contains all object IDs that have timed out in this time interval. - virtual void OnTimeout(const ObjectIdSet& ids) = 0; - }; - - typedef base::Callback<scoped_ptr<net::BackoffEntry>( - const net::BackoffEntry::Policy* const)> CreateBackoffEntryCallback; - - AckTracker(base::TickClock* tick_clock, Delegate* delegate); - ~AckTracker(); - - // Equivalent to calling Ack() on all currently registered object IDs. - void Clear(); - - // Starts tracking timeouts for |ids|. Timeouts will be triggered for each - // object ID until it is acknowledged. Note that no de-duplication is - // performed; calling Track() twice on the same set of ids will result in two - // different timeouts being triggered for those ids. - void Track(const ObjectIdSet& ids); - // Marks a set of |ids| as acknowledged. - void Ack(const ObjectIdSet& ids); - - // Testing methods. - void SetCreateBackoffEntryCallbackForTest( - const CreateBackoffEntryCallback& create_backoff_entry_callback); - // Returns true iff there are no timeouts scheduled to occur before |now|. - // Used in testing to make sure we don't have timeouts set to expire before - // when they should. - bool TriggerTimeoutAtForTest(base::TimeTicks now); - bool IsQueueEmptyForTest() const; - const base::Timer& GetTimerForTest() const; - - private: - struct Entry { - Entry(scoped_ptr<net::BackoffEntry> backoff, const ObjectIdSet& ids); - ~Entry(); - - scoped_ptr<net::BackoffEntry> backoff; - ObjectIdSet ids; - - private: - DISALLOW_COPY_AND_ASSIGN(Entry); - }; - - void NudgeTimer(); - void OnTimeout(); - void OnTimeoutAt(base::TimeTicks now); - - static scoped_ptr<net::BackoffEntry> DefaultCreateBackoffEntryStrategy( - const net::BackoffEntry::Policy* const policy); - - // Used for testing purposes. - CreateBackoffEntryCallback create_backoff_entry_callback_; - - base::TickClock* const tick_clock_; - - Delegate* const delegate_; - - base::OneShotTimer<AckTracker> timer_; - // The time that the timer should fire at. We use this to determine if we need - // to start or update |timer_| in NudgeTimer(). We can't simply use - // timer_.desired_run_time() for this purpose because it always uses - // base::TimeTicks::Now() as a reference point when Timer::Start() is called, - // while NudgeTimer() needs a fixed reference point to avoid unnecessarily - // updating the timer. - base::TimeTicks desired_run_time_; - std::multimap<base::TimeTicks, Entry*> queue_; - - base::ThreadChecker thread_checker_; - - DISALLOW_COPY_AND_ASSIGN(AckTracker); -}; - -} // namespace syncer - -#endif // SYNC_NOTIFIER_ACK_TRACKER_H_ diff --git a/sync/notifier/ack_tracker_unittest.cc b/sync/notifier/ack_tracker_unittest.cc deleted file mode 100644 index c1a6773..0000000 --- a/sync/notifier/ack_tracker_unittest.cc +++ /dev/null @@ -1,352 +0,0 @@ -// 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/notifier/ack_tracker.h" - -#include "base/compiler_specific.h" -#include "base/memory/ref_counted.h" -#include "base/message_loop/message_loop.h" -#include "base/time/tick_clock.h" -#include "google/cacheinvalidation/include/types.h" -#include "google/cacheinvalidation/types.pb.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace syncer { - -namespace { - -class FakeTickClock : public base::TickClock { - public: - FakeTickClock() {} - - virtual ~FakeTickClock() {} - - void LeapForward(int seconds) { - ASSERT_GT(seconds, 0); - fake_now_ticks_ += base::TimeDelta::FromSeconds(seconds); - } - - // After the next call to Now(), immediately leap forward by |seconds|. - void DelayedLeapForward(int seconds) { - ASSERT_GT(seconds, 0); - delayed_leap_ = base::TimeDelta::FromSeconds(seconds); - } - - virtual base::TimeTicks NowTicks() OVERRIDE { - base::TimeTicks fake_now_ticks = fake_now_ticks_; - if (delayed_leap_ > base::TimeDelta()) { - fake_now_ticks_ += delayed_leap_; - delayed_leap_ = base::TimeDelta(); - } - return fake_now_ticks; - } - - private: - base::TimeTicks fake_now_ticks_; - base::TimeDelta delayed_leap_; -}; - -class FakeBackoffEntry : public net::BackoffEntry { - public: - FakeBackoffEntry(const Policy* const policy, base::TickClock* tick_clock) - : BackoffEntry(policy), - tick_clock_(tick_clock) { - } - - protected: - virtual base::TimeTicks ImplGetTimeNow() const OVERRIDE { - return tick_clock_->NowTicks(); - } - - private: - base::TickClock* const tick_clock_; -}; - -class MockDelegate : public AckTracker::Delegate { - public: - MOCK_METHOD1(OnTimeout, void(const ObjectIdSet&)); -}; - -scoped_ptr<net::BackoffEntry> CreateMockEntry( - base::TickClock* tick_clock, - const net::BackoffEntry::Policy* const policy) { - return scoped_ptr<net::BackoffEntry>(new FakeBackoffEntry( - policy, tick_clock)); -} - -} // namespace - -class AckTrackerTest : public testing::Test { - public: - AckTrackerTest() - : ack_tracker_(&fake_tick_clock_, &delegate_), - kIdOne(ipc::invalidation::ObjectSource::TEST, "one"), - kIdTwo(ipc::invalidation::ObjectSource::TEST, "two") { - ack_tracker_.SetCreateBackoffEntryCallbackForTest( - base::Bind(&CreateMockEntry, &fake_tick_clock_)); - } - - protected: - bool TriggerTimeoutNow() { - return ack_tracker_.TriggerTimeoutAtForTest(fake_tick_clock_.NowTicks()); - } - - base::TimeDelta GetTimerDelay() const { - const base::Timer& timer = ack_tracker_.GetTimerForTest(); - if (!timer.IsRunning()) - ADD_FAILURE() << "Timer is not running!"; - return timer.GetCurrentDelay(); - } - - FakeTickClock fake_tick_clock_; - ::testing::StrictMock<MockDelegate> delegate_; - AckTracker ack_tracker_; - - const invalidation::ObjectId kIdOne; - const invalidation::ObjectId kIdTwo; - - // AckTracker uses base::Timer internally, which depends on the existence of a - // MessageLoop. - base::MessageLoop message_loop_; -}; - -// Tests that various combinations of Track()/Ack() behave as -// expected. -TEST_F(AckTrackerTest, TrackAndAck) { - ObjectIdSet ids_one; - ids_one.insert(kIdOne); - ObjectIdSet ids_two; - ids_two.insert(kIdTwo); - ObjectIdSet ids_all; - ids_all.insert(kIdOne); - ids_all.insert(kIdTwo); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids_one); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids_two); - ack_tracker_.Ack(ids_one); - ack_tracker_.Ack(ids_two); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - - ack_tracker_.Track(ids_all); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Ack(ids_one); - ack_tracker_.Ack(ids_two); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - - ack_tracker_.Track(ids_one); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids_two); - ack_tracker_.Ack(ids_all); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - - ack_tracker_.Track(ids_all); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Ack(ids_all); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -TEST_F(AckTrackerTest, DoubleTrack) { - ObjectIdSet ids; - ids.insert(kIdOne); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids); - ack_tracker_.Ack(ids); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -TEST_F(AckTrackerTest, UntrackedAck) { - ObjectIdSet ids; - ids.insert(kIdOne); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Ack(ids); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -TEST_F(AckTrackerTest, Clear) { - ObjectIdSet ids; - ids.insert(kIdOne); - ids.insert(kIdOne); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Clear(); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -// Test that timeout behavior for one object ID. The timeout should increase -// exponentially until it hits the cap. -TEST_F(AckTrackerTest, SimpleTimeout) { - ObjectIdSet ids; - ids.insert(kIdOne); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(60), GetTimerDelay()); - fake_tick_clock_.LeapForward(60); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(120), GetTimerDelay()); - fake_tick_clock_.LeapForward(120); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(240), GetTimerDelay()); - fake_tick_clock_.LeapForward(240); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(480), GetTimerDelay()); - fake_tick_clock_.LeapForward(480); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(600), GetTimerDelay()); - fake_tick_clock_.LeapForward(600); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(600), GetTimerDelay()); - fake_tick_clock_.LeapForward(600); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Ack(ids); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - - // The backoff time should be reset after an Ack/Track cycle. - ack_tracker_.Track(ids); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(60), GetTimerDelay()); - fake_tick_clock_.LeapForward(60); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Ack(ids); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -// Tests that a sequence of Track() calls that results in interleaving -// timeouts occurs as expected. -TEST_F(AckTrackerTest, InterleavedTimeout) { - ObjectIdSet ids_one; - ids_one.insert(kIdOne); - ObjectIdSet ids_two; - ids_two.insert(kIdTwo); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids_one); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - fake_tick_clock_.LeapForward(30); - ack_tracker_.Track(ids_two); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(60), GetTimerDelay()); - fake_tick_clock_.LeapForward(30); - EXPECT_CALL(delegate_, OnTimeout(ids_one)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(30), GetTimerDelay()); - fake_tick_clock_.LeapForward(30); - EXPECT_CALL(delegate_, OnTimeout(ids_two)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(90), GetTimerDelay()); - fake_tick_clock_.LeapForward(90); - EXPECT_CALL(delegate_, OnTimeout(ids_one)); - EXPECT_TRUE(TriggerTimeoutNow()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(30), GetTimerDelay()); - fake_tick_clock_.LeapForward(30); - EXPECT_CALL(delegate_, OnTimeout(ids_two)); - EXPECT_TRUE(TriggerTimeoutNow()); - - ack_tracker_.Ack(ids_one); - ack_tracker_.Ack(ids_two); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -// Tests that registering a new object ID properly shortens the timeout when -// needed. -TEST_F(AckTrackerTest, ShortenTimeout) { - ObjectIdSet ids_one; - ids_one.insert(kIdOne); - ObjectIdSet ids_two; - ids_two.insert(kIdTwo); - - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids_one); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(60), GetTimerDelay()); - fake_tick_clock_.LeapForward(60); - EXPECT_CALL(delegate_, OnTimeout(ids_one)); - EXPECT_TRUE(TriggerTimeoutNow()); - - // Without this next register, the next timeout should occur in 120 seconds - // from the last timeout event. - EXPECT_EQ(base::TimeDelta::FromSeconds(120), GetTimerDelay()); - fake_tick_clock_.LeapForward(30); - ack_tracker_.Track(ids_two); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - // Now that we've registered another entry though, we should receive a timeout - // in 60 seconds. - EXPECT_EQ(base::TimeDelta::FromSeconds(60), GetTimerDelay()); - fake_tick_clock_.LeapForward(60); - EXPECT_CALL(delegate_, OnTimeout(ids_two)); - EXPECT_TRUE(TriggerTimeoutNow()); - - // Verify that the original timeout for kIdOne still occurs as expected. - EXPECT_EQ(base::TimeDelta::FromSeconds(30), GetTimerDelay()); - fake_tick_clock_.LeapForward(30); - EXPECT_CALL(delegate_, OnTimeout(ids_one)); - EXPECT_TRUE(TriggerTimeoutNow()); - - ack_tracker_.Ack(ids_one); - ack_tracker_.Ack(ids_two); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -// Tests that a delay between inserting a new object ID registration and start -// the timer that is greater than the initial timeout period (60 seconds) does -// not break things. This could happen on a heavily loaded system, for instance. -TEST_F(AckTrackerTest, ImmediateTimeout) { - ObjectIdSet ids; - ids.insert(kIdOne); - - fake_tick_clock_.DelayedLeapForward(90); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); - ack_tracker_.Track(ids); - EXPECT_FALSE(ack_tracker_.IsQueueEmptyForTest()); - - EXPECT_EQ(base::TimeDelta::FromSeconds(0), GetTimerDelay()); - EXPECT_CALL(delegate_, OnTimeout(ids)); - message_loop_.RunUntilIdle(); - - // The next timeout should still be scheduled normally. - EXPECT_EQ(base::TimeDelta::FromSeconds(120), GetTimerDelay()); - fake_tick_clock_.LeapForward(120); - EXPECT_CALL(delegate_, OnTimeout(ids)); - EXPECT_TRUE(TriggerTimeoutNow()); - - ack_tracker_.Ack(ids); - EXPECT_TRUE(ack_tracker_.IsQueueEmptyForTest()); -} - -} // namespace syncer diff --git a/sync/notifier/fake_invalidation_handler.h b/sync/notifier/fake_invalidation_handler.h index 7ab89f4..81b877a 100644 --- a/sync/notifier/fake_invalidation_handler.h +++ b/sync/notifier/fake_invalidation_handler.h @@ -10,6 +10,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "sync/notifier/invalidation_handler.h" +#include "sync/notifier/object_id_invalidation_map.h" namespace syncer { diff --git a/sync/notifier/fake_invalidation_state_tracker.cc b/sync/notifier/fake_invalidation_state_tracker.cc index 6e147fe..47e2f0f 100644 --- a/sync/notifier/fake_invalidation_state_tracker.cc +++ b/sync/notifier/fake_invalidation_state_tracker.cc @@ -18,35 +18,6 @@ FakeInvalidationStateTracker::FakeInvalidationStateTracker() {} FakeInvalidationStateTracker::~FakeInvalidationStateTracker() {} -int64 FakeInvalidationStateTracker::GetMaxVersion( - const invalidation::ObjectId& id) const { - InvalidationStateMap::const_iterator it = state_map_.find(id); - return (it == state_map_.end()) ? kMinVersion : it->second.version; -} - -InvalidationStateMap -FakeInvalidationStateTracker::GetAllInvalidationStates() const { - return state_map_; -} - -void FakeInvalidationStateTracker::SetMaxVersionAndPayload( - const invalidation::ObjectId& id, - int64 max_version, - const std::string& payload) { - InvalidationStateMap::const_iterator it = state_map_.find(id); - if ((it != state_map_.end()) && (max_version <= it->second.version)) { - ADD_FAILURE(); - return; - } - state_map_[id].version = max_version; -} - -void FakeInvalidationStateTracker::Forget(const ObjectIdSet& ids) { - for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { - state_map_.erase(*it); - } -} - void FakeInvalidationStateTracker::SetInvalidatorClientId( const std::string& client_id) { Clear(); @@ -66,31 +37,19 @@ std::string FakeInvalidationStateTracker::GetBootstrapData() const { return bootstrap_data_; } -void FakeInvalidationStateTracker::Clear() { - invalidator_client_id_ = ""; - state_map_ = InvalidationStateMap(); - bootstrap_data_ = ""; +void FakeInvalidationStateTracker::SetSavedInvalidations( + const UnackedInvalidationsMap& states) { + unacked_invalidations_map_ = states; } -void FakeInvalidationStateTracker::GenerateAckHandles( - const ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const AckHandleMap&)> callback) { - AckHandleMap ack_handles; - for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { - state_map_[*it].expected = AckHandle::CreateUnique(); - ack_handles.insert(std::make_pair(*it, state_map_[*it].expected)); - } - if (!task_runner->PostTask(FROM_HERE, base::Bind(callback, ack_handles))) - ADD_FAILURE(); +UnackedInvalidationsMap +FakeInvalidationStateTracker::GetSavedInvalidations() const { + return unacked_invalidations_map_; } -void FakeInvalidationStateTracker::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - InvalidationStateMap::iterator it = state_map_.find(id); - if (it == state_map_.end()) - ADD_FAILURE(); - it->second.current = ack_handle; +void FakeInvalidationStateTracker::Clear() { + invalidator_client_id_.clear(); + bootstrap_data_.clear(); } } // namespace syncer diff --git a/sync/notifier/fake_invalidation_state_tracker.h b/sync/notifier/fake_invalidation_state_tracker.h index b43699b..d1daaba 100644 --- a/sync/notifier/fake_invalidation_state_tracker.h +++ b/sync/notifier/fake_invalidation_state_tracker.h @@ -19,32 +19,22 @@ class FakeInvalidationStateTracker FakeInvalidationStateTracker(); virtual ~FakeInvalidationStateTracker(); - int64 GetMaxVersion(const invalidation::ObjectId& id) const; - // InvalidationStateTracker implementation. - virtual InvalidationStateMap GetAllInvalidationStates() const OVERRIDE; - virtual void SetMaxVersionAndPayload(const invalidation::ObjectId& id, - int64 max_version, - const std::string& payload) OVERRIDE; - virtual void Forget(const ObjectIdSet& ids) OVERRIDE; virtual void SetInvalidatorClientId(const std::string& client_id) OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; virtual void SetBootstrapData(const std::string& data) OVERRIDE; virtual std::string GetBootstrapData() const OVERRIDE; + virtual void SetSavedInvalidations( + const UnackedInvalidationsMap& states) OVERRIDE; + virtual UnackedInvalidationsMap GetSavedInvalidations() const OVERRIDE; virtual void Clear() OVERRIDE; - virtual void GenerateAckHandles( - const ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const AckHandleMap&)> callback) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; static const int64 kMinVersion; private: - InvalidationStateMap state_map_; std::string invalidator_client_id_; std::string bootstrap_data_; + UnackedInvalidationsMap unacked_invalidations_map_; }; } // namespace syncer diff --git a/sync/notifier/fake_invalidator.cc b/sync/notifier/fake_invalidator.cc index 0b217f7..3e1ce32 100644 --- a/sync/notifier/fake_invalidator.cc +++ b/sync/notifier/fake_invalidator.cc @@ -4,6 +4,8 @@ #include "sync/notifier/fake_invalidator.h" +#include "sync/notifier/object_id_invalidation_map.h" + namespace syncer { FakeInvalidator::FakeInvalidator() {} @@ -49,11 +51,6 @@ void FakeInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } -void FakeInvalidator::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - // Do nothing. -} - InvalidatorState FakeInvalidator::GetInvalidatorState() const { return registrar_.GetInvalidatorState(); } diff --git a/sync/notifier/fake_invalidator.h b/sync/notifier/fake_invalidator.h index 7913694..b2173ee 100644 --- a/sync/notifier/fake_invalidator.h +++ b/sync/notifier/fake_invalidator.h @@ -24,7 +24,6 @@ class FakeInvalidator : public Invalidator { const std::string& GetUniqueId() const; const std::string& GetCredentialsEmail() const; const std::string& GetCredentialsToken() const; - const ObjectIdInvalidationMap& GetLastSentInvalidationMap() const; void EmitOnInvalidatorStateChange(InvalidatorState state); void EmitOnIncomingInvalidation( @@ -34,8 +33,6 @@ class FakeInvalidator : public Invalidator { virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; diff --git a/sync/notifier/invalidation_handler.h b/sync/notifier/invalidation_handler.h index be85116..2f5149f 100644 --- a/sync/notifier/invalidation_handler.h +++ b/sync/notifier/invalidation_handler.h @@ -7,10 +7,11 @@ #include "sync/base/sync_export.h" #include "sync/notifier/invalidator_state.h" -#include "sync/notifier/object_id_invalidation_map.h" namespace syncer { +class ObjectIdInvalidationMap; + class SYNC_EXPORT InvalidationHandler { public: // Called when the invalidator state changes. diff --git a/sync/notifier/invalidation_notifier.cc b/sync/notifier/invalidation_notifier.cc index e3c79a4..a509409 100644 --- a/sync/notifier/invalidation_notifier.cc +++ b/sync/notifier/invalidation_notifier.cc @@ -20,17 +20,17 @@ namespace syncer { InvalidationNotifier::InvalidationNotifier( scoped_ptr<notifier::PushClient> push_client, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, const std::string& client_info) : state_(STOPPED), - initial_invalidation_state_map_(initial_invalidation_state_map), + saved_invalidations_(saved_invalidations), invalidation_state_tracker_(invalidation_state_tracker), client_info_(client_info), invalidator_client_id_(invalidator_client_id), invalidation_bootstrap_data_(invalidation_bootstrap_data), - invalidation_listener_(&tick_clock_, push_client.Pass()) { + invalidation_listener_(push_client.Pass()) { } InvalidationNotifier::~InvalidationNotifier() { @@ -54,12 +54,6 @@ void InvalidationNotifier::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } -void InvalidationNotifier::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - DCHECK(CalledOnValidThread()); - invalidation_listener_.Acknowledge(id, ack_handle); -} - InvalidatorState InvalidationNotifier::GetInvalidatorState() const { DCHECK(CalledOnValidThread()); return registrar_.GetInvalidatorState(); @@ -71,7 +65,7 @@ void InvalidationNotifier::UpdateCredentials( invalidation_listener_.Start( base::Bind(&invalidation::CreateInvalidationClient), invalidator_client_id_, client_info_, invalidation_bootstrap_data_, - initial_invalidation_state_map_, + saved_invalidations_, invalidation_state_tracker_, this); state_ = STARTED; diff --git a/sync/notifier/invalidation_notifier.h b/sync/notifier/invalidation_notifier.h index b7a98f8..a11608c 100644 --- a/sync/notifier/invalidation_notifier.h +++ b/sync/notifier/invalidation_notifier.h @@ -18,7 +18,6 @@ #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" #include "base/threading/non_thread_safe.h" -#include "base/time/default_tick_clock.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/util/weak_handle.h" @@ -34,7 +33,6 @@ class PushClient; namespace syncer { // This class must live on the IO thread. -// TODO(dcheng): Think of a name better than InvalidationInvalidator. class SYNC_EXPORT_PRIVATE InvalidationNotifier : public Invalidator, public SyncInvalidationListener::Delegate, @@ -44,7 +42,7 @@ class SYNC_EXPORT_PRIVATE InvalidationNotifier InvalidationNotifier( scoped_ptr<notifier::PushClient> push_client, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, @@ -57,8 +55,6 @@ class SYNC_EXPORT_PRIVATE InvalidationNotifier virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; @@ -83,7 +79,7 @@ class SYNC_EXPORT_PRIVATE InvalidationNotifier InvalidatorRegistrar registrar_; // Passed to |invalidation_listener_|. - const InvalidationStateMap initial_invalidation_state_map_; + const UnackedInvalidationsMap saved_invalidations_; // Passed to |invalidation_listener_|. const WeakHandle<InvalidationStateTracker> @@ -98,10 +94,6 @@ class SYNC_EXPORT_PRIVATE InvalidationNotifier // The initial bootstrap data to pass to |invalidation_listener_|. const std::string invalidation_bootstrap_data_; - // TODO(akalin): Clean up this reference to DefaultTickClock. Ideally, we - // should simply be using TaskRunner's tick clock. See http://crbug.com/179211 - base::DefaultTickClock tick_clock_; - // The invalidation listener. SyncInvalidationListener invalidation_listener_; diff --git a/sync/notifier/invalidation_notifier_unittest.cc b/sync/notifier/invalidation_notifier_unittest.cc index 1f14319..bc92e23 100644 --- a/sync/notifier/invalidation_notifier_unittest.cc +++ b/sync/notifier/invalidation_notifier_unittest.cc @@ -40,7 +40,7 @@ class InvalidationNotifierTestDelegate { new InvalidationNotifier( scoped_ptr<notifier::PushClient>(new notifier::FakePushClient()), invalidator_client_id, - InvalidationStateMap(), + UnackedInvalidationsMap(), initial_state, MakeWeakHandle(invalidation_state_tracker), "fake_client_info")); diff --git a/sync/notifier/invalidation_state_tracker.cc b/sync/notifier/invalidation_state_tracker.cc deleted file mode 100644 index 335f3b2..0000000 --- a/sync/notifier/invalidation_state_tracker.cc +++ /dev/null @@ -1,24 +0,0 @@ -// 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/notifier/invalidation_state_tracker.h" - -namespace syncer { - -InvalidationState::InvalidationState() - : version(kint64min), - expected(AckHandle::InvalidAckHandle()), - current(AckHandle::InvalidAckHandle()) { -} - -InvalidationState::~InvalidationState() { -} - -bool operator==(const InvalidationState& lhs, const InvalidationState& rhs) { - return lhs.version == rhs.version && - lhs.expected.Equals(rhs.expected) && - lhs.current.Equals(rhs.current); -} - -} // namespace syncer diff --git a/sync/notifier/invalidation_state_tracker.h b/sync/notifier/invalidation_state_tracker.h index e3e5bd2..81a07ea 100644 --- a/sync/notifier/invalidation_state_tracker.h +++ b/sync/notifier/invalidation_state_tracker.h @@ -22,6 +22,7 @@ #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/invalidation.h" #include "sync/notifier/invalidation_util.h" +#include "sync/notifier/unacked_invalidation_set.h" namespace base { class TaskRunner; @@ -29,39 +30,10 @@ class TaskRunner; namespace syncer { -struct SYNC_EXPORT InvalidationState { - InvalidationState(); - ~InvalidationState(); - - int64 version; - std::string payload; - AckHandle expected; - AckHandle current; -}; - -// TODO(dcheng): Remove this in favor of adding an Equals() method. -SYNC_EXPORT_PRIVATE bool operator==(const InvalidationState& lhs, - const InvalidationState& rhs); - -typedef std::map<invalidation::ObjectId, InvalidationState, ObjectIdLessThan> - InvalidationStateMap; -typedef std::map<invalidation::ObjectId, AckHandle, ObjectIdLessThan> - AckHandleMap; - class InvalidationStateTracker { public: InvalidationStateTracker() {} - virtual InvalidationStateMap GetAllInvalidationStates() const = 0; - - // |max_version| should be strictly greater than any existing max - // version for |model_type|. - virtual void SetMaxVersionAndPayload(const invalidation::ObjectId& id, - int64 max_version, - const std::string& payload) = 0; - // Removes all state tracked for |ids|. - virtual void Forget(const ObjectIdSet& ids) = 0; - // The per-client unique ID used to register the invalidation client with the // server. This is used to squelch invalidation notifications that originate // from changes made by this client. @@ -75,24 +47,15 @@ class InvalidationStateTracker { virtual void SetBootstrapData(const std::string& data) = 0; virtual std::string GetBootstrapData() const = 0; + // Used to store invalidations that have been acked to the server, but not yet + // handled by our clients. We store these invalidations on disk so we won't + // lose them if we need to restart. + virtual void SetSavedInvalidations(const UnackedInvalidationsMap& states) = 0; + virtual UnackedInvalidationsMap GetSavedInvalidations() const = 0; + // Erases invalidation versions, client ID, and state stored on disk. virtual void Clear() = 0; - // Used for generating our own local ack handles. Generates a new ack handle - // for each object id in |ids|. The result is returned via |callback| posted - // to |task_runner|. - virtual void GenerateAckHandles( - const ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const AckHandleMap&)> callback) = 0; - - // Records an acknowledgement for |id|. Note that no attempt at ordering is - // made. Acknowledge() only records the last ack_handle it received, even if - // the last ack_handle it received was generated before the value currently - // recorded. - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) = 0; - protected: virtual ~InvalidationStateTracker() {} }; diff --git a/sync/notifier/invalidator.h b/sync/notifier/invalidator.h index ccb6922..b6a7467 100644 --- a/sync/notifier/invalidator.h +++ b/sync/notifier/invalidator.h @@ -15,7 +15,6 @@ #include "sync/internal_api/public/base/model_type.h" #include "sync/notifier/invalidation_util.h" #include "sync/notifier/invalidator_state.h" -#include "sync/notifier/object_id_invalidation_map.h" namespace syncer { class InvalidationHandler; @@ -69,10 +68,6 @@ class SYNC_EXPORT Invalidator { // associated with |handler|. virtual void UnregisterHandler(InvalidationHandler* handler) = 0; - // Acknowledge that an invalidation for |id| was handled. - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) = 0; - // Returns the current invalidator state. When called from within // InvalidationHandler::OnInvalidatorStateChange(), this must return // the updated state. diff --git a/sync/notifier/invalidator_registrar_unittest.cc b/sync/notifier/invalidator_registrar_unittest.cc index 7a01401..c527bc5 100644 --- a/sync/notifier/invalidator_registrar_unittest.cc +++ b/sync/notifier/invalidator_registrar_unittest.cc @@ -42,11 +42,6 @@ class RegistrarInvalidator : public Invalidator { registrar_.UnregisterHandler(handler); } - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE { - // Do nothing. - } - virtual InvalidatorState GetInvalidatorState() const OVERRIDE { return registrar_.GetInvalidatorState(); } diff --git a/sync/notifier/invalidator_test_template.h b/sync/notifier/invalidator_test_template.h index b86cf50..67cd053 100644 --- a/sync/notifier/invalidator_test_template.h +++ b/sync/notifier/invalidator_test_template.h @@ -81,7 +81,6 @@ #include "base/compiler_specific.h" #include "google/cacheinvalidation/include/types.h" #include "google/cacheinvalidation/types.pb.h" -#include "sync/internal_api/public/base/invalidation_test_util.h" #include "sync/internal_api/public/base/object_id_invalidation_map_test_util.h" #include "sync/notifier/fake_invalidation_handler.h" #include "sync/notifier/fake_invalidation_state_tracker.h" diff --git a/sync/notifier/mock_ack_handler.h b/sync/notifier/mock_ack_handler.h index 6659558..bf6ecc9 100644 --- a/sync/notifier/mock_ack_handler.h +++ b/sync/notifier/mock_ack_handler.h @@ -19,7 +19,7 @@ class Invalidation; // This AckHandler implementation colaborates with the FakeInvalidationService // to enable unit tests to assert that invalidations are being acked properly. -class MockAckHandler +class SYNC_EXPORT MockAckHandler : public AckHandler, public base::SupportsWeakPtr<MockAckHandler> { public: diff --git a/sync/notifier/non_blocking_invalidator.cc b/sync/notifier/non_blocking_invalidator.cc index ca89132..bd05967 100644 --- a/sync/notifier/non_blocking_invalidator.cc +++ b/sync/notifier/non_blocking_invalidator.cc @@ -14,6 +14,7 @@ #include "base/threading/thread.h" #include "jingle/notifier/listener/push_client.h" #include "sync/notifier/invalidation_notifier.h" +#include "sync/notifier/object_id_invalidation_map.h" namespace syncer { @@ -31,14 +32,12 @@ class NonBlockingInvalidator::Core void Initialize( const notifier::NotifierOptions& notifier_options, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, const std::string& client_info); void Teardown(); void UpdateRegisteredIds(const ObjectIdSet& ids); - void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle); void UpdateCredentials(const std::string& email, const std::string& token); // InvalidationHandler implementation (all called on I/O thread by @@ -73,7 +72,7 @@ NonBlockingInvalidator::Core::~Core() { void NonBlockingInvalidator::Core::Initialize( const notifier::NotifierOptions& notifier_options, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, const std::string& client_info) { @@ -87,7 +86,7 @@ void NonBlockingInvalidator::Core::Initialize( new InvalidationNotifier( notifier::PushClient::CreateDefaultOnIOThread(notifier_options), invalidator_client_id, - initial_invalidation_state_map, + saved_invalidations, invalidation_bootstrap_data, invalidation_state_tracker, client_info)); @@ -106,12 +105,6 @@ void NonBlockingInvalidator::Core::UpdateRegisteredIds(const ObjectIdSet& ids) { invalidation_notifier_->UpdateRegisteredIds(this, ids); } -void NonBlockingInvalidator::Core::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - DCHECK(network_task_runner_->BelongsToCurrentThread()); - invalidation_notifier_->Acknowledge(id, ack_handle); -} - void NonBlockingInvalidator::Core::UpdateCredentials(const std::string& email, const std::string& token) { DCHECK(network_task_runner_->BelongsToCurrentThread()); @@ -136,7 +129,7 @@ void NonBlockingInvalidator::Core::OnIncomingInvalidation( NonBlockingInvalidator::NonBlockingInvalidator( const notifier::NotifierOptions& notifier_options, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, @@ -154,7 +147,7 @@ NonBlockingInvalidator::NonBlockingInvalidator( core_.get(), notifier_options, invalidator_client_id, - initial_invalidation_state_map, + saved_invalidations, invalidation_bootstrap_data, invalidation_state_tracker, client_info))) { @@ -196,20 +189,6 @@ void NonBlockingInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } -void NonBlockingInvalidator::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - DCHECK(parent_task_runner_->BelongsToCurrentThread()); - if (!network_task_runner_->PostTask( - FROM_HERE, - base::Bind( - &NonBlockingInvalidator::Core::Acknowledge, - core_.get(), - id, - ack_handle))) { - NOTREACHED(); - } -} - InvalidatorState NonBlockingInvalidator::GetInvalidatorState() const { DCHECK(parent_task_runner_->BelongsToCurrentThread()); return registrar_.GetInvalidatorState(); diff --git a/sync/notifier/non_blocking_invalidator.h b/sync/notifier/non_blocking_invalidator.h index 2cb5903..d40166a 100644 --- a/sync/notifier/non_blocking_invalidator.h +++ b/sync/notifier/non_blocking_invalidator.h @@ -28,8 +28,6 @@ class SingleThreadTaskRunner; namespace syncer { -// TODO(akalin): Generalize the interface so it can use any Invalidator. -// (http://crbug.com/140409). class SYNC_EXPORT_PRIVATE NonBlockingInvalidator : public Invalidator, // InvalidationHandler to "observe" our Core via WeakHandle. @@ -39,7 +37,7 @@ class SYNC_EXPORT_PRIVATE NonBlockingInvalidator NonBlockingInvalidator( const notifier::NotifierOptions& notifier_options, const std::string& invalidator_client_id, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& saved_invalidations, const std::string& invalidation_bootstrap_data, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, @@ -52,8 +50,6 @@ class SYNC_EXPORT_PRIVATE NonBlockingInvalidator virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; diff --git a/sync/notifier/non_blocking_invalidator_unittest.cc b/sync/notifier/non_blocking_invalidator_unittest.cc index e9cf31e..0c439c5 100644 --- a/sync/notifier/non_blocking_invalidator_unittest.cc +++ b/sync/notifier/non_blocking_invalidator_unittest.cc @@ -48,7 +48,7 @@ class NonBlockingInvalidatorTestDelegate { new NonBlockingInvalidator( invalidator_options, invalidator_client_id, - InvalidationStateMap(), + UnackedInvalidationsMap(), initial_state, MakeWeakHandle(invalidation_state_tracker), "fake_client_info")); diff --git a/sync/notifier/object_id_invalidation_map.cc b/sync/notifier/object_id_invalidation_map.cc index e1f584d..1082eaa 100644 --- a/sync/notifier/object_id_invalidation_map.cc +++ b/sync/notifier/object_id_invalidation_map.cc @@ -64,6 +64,15 @@ void ObjectIdInvalidationMap::GetAllInvalidations( out->insert(out->begin(), it->second.begin(), it->second.end()); } } +void ObjectIdInvalidationMap::AcknowledgeAll() const { + for (IdToListMap::const_iterator it1 = map_.begin(); + it1 != map_.end(); ++it1) { + for (SingleObjectInvalidationSet::const_iterator it2 = it1->second.begin(); + it2 != it1->second.end(); ++it2) { + it2->Acknowledge(); + } + } +} bool ObjectIdInvalidationMap::operator==( const ObjectIdInvalidationMap& other) const { diff --git a/sync/notifier/object_id_invalidation_map.h b/sync/notifier/object_id_invalidation_map.h index c17e101..3494a62 100644 --- a/sync/notifier/object_id_invalidation_map.h +++ b/sync/notifier/object_id_invalidation_map.h @@ -49,6 +49,9 @@ class SYNC_EXPORT ObjectIdInvalidationMap { // Returns the contents of this map in a single vector. void GetAllInvalidations(std::vector<syncer::Invalidation>* out) const; + // Call Acknowledge() on all contained Invalidations. + void AcknowledgeAll() const; + // Serialize this map to a value. scoped_ptr<base::ListValue> ToValue() const; diff --git a/sync/notifier/p2p_invalidator.cc b/sync/notifier/p2p_invalidator.cc index 3d47f41..cd82e61 100644 --- a/sync/notifier/p2p_invalidator.cc +++ b/sync/notifier/p2p_invalidator.cc @@ -180,12 +180,6 @@ void P2PInvalidator::UnregisterHandler(InvalidationHandler* handler) { registrar_.UnregisterHandler(handler); } -void P2PInvalidator::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - DCHECK(thread_checker_.CalledOnValidThread()); - // Do nothing for the P2P implementation. -} - InvalidatorState P2PInvalidator::GetInvalidatorState() const { DCHECK(thread_checker_.CalledOnValidThread()); return registrar_.GetInvalidatorState(); diff --git a/sync/notifier/p2p_invalidator.h b/sync/notifier/p2p_invalidator.h index bf622f0..e0bd5bc 100644 --- a/sync/notifier/p2p_invalidator.h +++ b/sync/notifier/p2p_invalidator.h @@ -106,8 +106,6 @@ class SYNC_EXPORT_PRIVATE P2PInvalidator virtual void UpdateRegisteredIds(InvalidationHandler* handler, const ObjectIdSet& ids) OVERRIDE; virtual void UnregisterHandler(InvalidationHandler* handler) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; virtual InvalidatorState GetInvalidatorState() const OVERRIDE; virtual void UpdateCredentials( const std::string& email, const std::string& token) OVERRIDE; diff --git a/sync/notifier/p2p_invalidator_unittest.cc b/sync/notifier/p2p_invalidator_unittest.cc index 3cbabaf..7898fee 100644 --- a/sync/notifier/p2p_invalidator_unittest.cc +++ b/sync/notifier/p2p_invalidator_unittest.cc @@ -178,7 +178,8 @@ TEST_F(P2PInvalidatorTest, P2PNotificationDataDefault) { // non-default-constructed P2PNotificationData. TEST_F(P2PInvalidatorTest, P2PNotificationDataNonDefault) { ObjectIdInvalidationMap invalidation_map = - MakeInvalidationMap(ModelTypeSet(BOOKMARKS, THEMES)); + ObjectIdInvalidationMap::InvalidateAll( + ModelTypeSetToObjectIdSet(ModelTypeSet(BOOKMARKS, THEMES))); const P2PNotificationData notification_data("sender", NOTIFY_ALL, invalidation_map); diff --git a/sync/notifier/sync_invalidation_listener.cc b/sync/notifier/sync_invalidation_listener.cc index 763cc87..f955131 100644 --- a/sync/notifier/sync_invalidation_listener.cc +++ b/sync/notifier/sync_invalidation_listener.cc @@ -16,14 +16,13 @@ #include "google/cacheinvalidation/types.pb.h" #include "jingle/notifier/listener/push_client.h" #include "sync/notifier/invalidation_util.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "sync/notifier/registration_manager.h" namespace { const char kApplicationName[] = "chrome-sync"; -static const int64 kUnknownVersion = -1; - } // namespace namespace syncer { @@ -31,10 +30,8 @@ namespace syncer { SyncInvalidationListener::Delegate::~Delegate() {} SyncInvalidationListener::SyncInvalidationListener( - base::TickClock* tick_clock, scoped_ptr<notifier::PushClient> push_client) - : ack_tracker_(tick_clock, this), - push_client_(push_client.get()), + : push_client_(push_client.get()), sync_system_resources_(push_client.Pass(), this), delegate_(NULL), ticl_state_(DEFAULT_INVALIDATION_ERROR), @@ -56,7 +53,7 @@ void SyncInvalidationListener::Start( create_invalidation_client_callback, const std::string& client_id, const std::string& client_info, const std::string& invalidation_bootstrap_data, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& initial_unacked_invalidations, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, Delegate* delegate) { DCHECK(CalledOnValidThread()); @@ -71,18 +68,7 @@ void SyncInvalidationListener::Start( sync_system_resources_.storage()->SetInitialState( invalidation_bootstrap_data); - invalidation_state_map_ = initial_invalidation_state_map; - if (invalidation_state_map_.empty()) { - DVLOG(2) << "No initial max invalidation versions for any id"; - } else { - for (InvalidationStateMap::const_iterator it = - invalidation_state_map_.begin(); - it != invalidation_state_map_.end(); ++it) { - DVLOG(2) << "Initial max invalidation version for " - << ObjectIdToString(it->first) << " is " - << it->second.version; - } - } + unacked_invalidations_map_ = initial_unacked_invalidations; invalidation_state_tracker_ = invalidation_state_tracker; DCHECK(invalidation_state_tracker_.IsInitialized()); @@ -103,19 +89,6 @@ void SyncInvalidationListener::Start( registration_manager_.reset( new RegistrationManager(invalidation_client_.get())); - - // Set up reminders for any invalidations that have not been locally - // acknowledged. - ObjectIdSet unacknowledged_ids; - for (InvalidationStateMap::const_iterator it = - invalidation_state_map_.begin(); - it != invalidation_state_map_.end(); ++it) { - if (it->second.expected.Equals(it->second.current)) - continue; - unacknowledged_ids.insert(it->first); - } - if (!unacknowledged_ids.empty()) - ack_tracker_.Track(unacknowledged_ids); } void SyncInvalidationListener::UpdateCredentials( @@ -135,27 +108,6 @@ void SyncInvalidationListener::UpdateRegisteredIds(const ObjectIdSet& ids) { } } -void SyncInvalidationListener::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - DCHECK(CalledOnValidThread()); - InvalidationStateMap::iterator state_it = invalidation_state_map_.find(id); - if (state_it == invalidation_state_map_.end()) - return; - invalidation_state_tracker_.Call( - FROM_HERE, - &InvalidationStateTracker::Acknowledge, - id, - ack_handle); - state_it->second.current = ack_handle; - if (state_it->second.expected.Equals(ack_handle)) { - // If the received ack matches the expected ack, then we no longer need to - // keep track of |id| since it is up-to-date. - ObjectIdSet ids; - ids.insert(id); - ack_tracker_.Ack(ids); - } -} - void SyncInvalidationListener::Ready( invalidation::InvalidationClient* client) { DCHECK(CalledOnValidThread()); @@ -171,43 +123,24 @@ void SyncInvalidationListener::Invalidate( const invalidation::AckHandle& ack_handle) { DCHECK(CalledOnValidThread()); DCHECK_EQ(client, invalidation_client_.get()); - DVLOG(1) << "Invalidate: " << InvalidationToString(invalidation); + client->Acknowledge(ack_handle); const invalidation::ObjectId& id = invalidation.object_id(); - // The invalidation API spec allows for the possibility of redundant - // invalidations, so keep track of the max versions and drop - // invalidations with old versions. - // - // TODO(akalin): Now that we keep track of registered ids, we - // should drop invalidations for unregistered ids. We may also - // have to filter it at a higher level, as invalidations for - // newly-unregistered ids may already be in flight. - InvalidationStateMap::const_iterator it = invalidation_state_map_.find(id); - if ((it != invalidation_state_map_.end()) && - (invalidation.version() <= it->second.version)) { - // Drop redundant invalidations. - client->Acknowledge(ack_handle); - return; - } - std::string payload; // payload() CHECK()'s has_payload(), so we must check it ourselves first. if (invalidation.has_payload()) payload = invalidation.payload(); - DVLOG(2) << "Setting max invalidation version for " << ObjectIdToString(id) - << " to " << invalidation.version(); - invalidation_state_map_[id].version = invalidation.version(); - invalidation_state_map_[id].payload = payload; - invalidation_state_tracker_.Call( - FROM_HERE, - &InvalidationStateTracker::SetMaxVersionAndPayload, - id, invalidation.version(), payload); + DVLOG(2) << "Received invalidation with version " << invalidation.version() + << " for " << ObjectIdToString(id); + + ObjectIdInvalidationMap invalidations; + Invalidation inv = Invalidation::Init(id, invalidation.version(), payload); + inv.set_ack_handler(GetThisAsAckHandler()); + invalidations.Insert(inv); - ObjectIdSet ids; - ids.insert(id); - PrepareInvalidation(ids, invalidation.version(), payload, client, ack_handle); + DispatchInvalidations(invalidations); } void SyncInvalidationListener::InvalidateUnknownVersion( @@ -217,15 +150,14 @@ void SyncInvalidationListener::InvalidateUnknownVersion( DCHECK(CalledOnValidThread()); DCHECK_EQ(client, invalidation_client_.get()); DVLOG(1) << "InvalidateUnknownVersion"; + client->Acknowledge(ack_handle); + + ObjectIdInvalidationMap invalidations; + Invalidation unknown_version = Invalidation::InitUnknownVersion(object_id); + unknown_version.set_ack_handler(GetThisAsAckHandler()); + invalidations.Insert(unknown_version); - ObjectIdSet ids; - ids.insert(object_id); - PrepareInvalidation( - ids, - kUnknownVersion, - std::string(), - client, - ack_handle); + DispatchInvalidations(invalidations); } // This should behave as if we got an invalidation with version @@ -236,86 +168,56 @@ void SyncInvalidationListener::InvalidateAll( DCHECK(CalledOnValidThread()); DCHECK_EQ(client, invalidation_client_.get()); DVLOG(1) << "InvalidateAll"; + client->Acknowledge(ack_handle); + + ObjectIdInvalidationMap invalidations; + for (ObjectIdSet::iterator it = registered_ids_.begin(); + it != registered_ids_.end(); ++it) { + Invalidation unknown_version = Invalidation::InitUnknownVersion(*it); + unknown_version.set_ack_handler(GetThisAsAckHandler()); + invalidations.Insert(unknown_version); + } - PrepareInvalidation( - registered_ids_, - kUnknownVersion, - std::string(), - client, - ack_handle); + DispatchInvalidations(invalidations); } -void SyncInvalidationListener::PrepareInvalidation( - const ObjectIdSet& ids, - int64 version, - const std::string& payload, - invalidation::InvalidationClient* client, - const invalidation::AckHandle& ack_handle) { +// If a handler is registered, emit right away. Otherwise, save it for later. +void SyncInvalidationListener::DispatchInvalidations( + const ObjectIdInvalidationMap& invalidations) { DCHECK(CalledOnValidThread()); - // A server invalidation resets the local retry count. - ack_tracker_.Ack(ids); - invalidation_state_tracker_.Call( - FROM_HERE, - &InvalidationStateTracker::GenerateAckHandles, - ids, - base::MessageLoopProxy::current(), - base::Bind(&SyncInvalidationListener::EmitInvalidation, - weak_ptr_factory_.GetWeakPtr(), - ids, - version, - payload, - client, - ack_handle)); -} + ObjectIdInvalidationMap to_save = invalidations; + ObjectIdInvalidationMap to_emit = + invalidations.GetSubsetWithObjectIds(registered_ids_); -void SyncInvalidationListener::EmitInvalidation( - const ObjectIdSet& ids, - int64 version, - const std::string& payload, - invalidation::InvalidationClient* client, - const invalidation::AckHandle& ack_handle, - const AckHandleMap& local_ack_handles) { - DCHECK(CalledOnValidThread()); + SaveInvalidations(to_save); + EmitSavedInvalidations(to_emit); +} - ObjectIdInvalidationMap invalidation_map; - for (AckHandleMap::const_iterator it = local_ack_handles.begin(); - it != local_ack_handles.end(); ++it) { - // Update in-memory copy of the invalidation state. - invalidation_state_map_[it->first].expected = it->second; - - if (version == kUnknownVersion) { - Invalidation inv = Invalidation::InitUnknownVersion(it->first); - inv.set_ack_handle(it->second); - invalidation_map.Insert(inv); - } else { - Invalidation inv = Invalidation::Init(it->first, version, payload); - inv.set_ack_handle(it->second); - invalidation_map.Insert(inv); +void SyncInvalidationListener::SaveInvalidations( + const ObjectIdInvalidationMap& to_save) { + ObjectIdSet objects_to_save = to_save.GetObjectIds(); + for (ObjectIdSet::const_iterator it = objects_to_save.begin(); + it != objects_to_save.end(); ++it) { + UnackedInvalidationsMap::iterator lookup = + unacked_invalidations_map_.find(*it); + if (lookup == unacked_invalidations_map_.end()) { + lookup = unacked_invalidations_map_.insert( + std::make_pair(*it, UnackedInvalidationSet(*it))).first; } + lookup->second.AddSet(to_save.ForObject(*it)); } - ack_tracker_.Track(ids); - delegate_->OnInvalidate(invalidation_map); - client->Acknowledge(ack_handle); + + invalidation_state_tracker_.Call( + FROM_HERE, + &InvalidationStateTracker::SetSavedInvalidations, + unacked_invalidations_map_); } -void SyncInvalidationListener::OnTimeout(const ObjectIdSet& ids) { - ObjectIdInvalidationMap invalidation_map; - for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { - if (invalidation_state_map_[*it].version == kUnknownVersion) { - Invalidation inv = Invalidation::InitUnknownVersion(*it); - inv.set_ack_handle(invalidation_state_map_[*it].expected); - invalidation_map.Insert(inv); - } else { - Invalidation inv = Invalidation::Init( - *it, - invalidation_state_map_[*it].version, - invalidation_state_map_[*it].payload); - inv.set_ack_handle(invalidation_state_map_[*it].expected); - invalidation_map.Insert(inv); - } - } - delegate_->OnInvalidate(invalidation_map); +void SyncInvalidationListener::EmitSavedInvalidations( + const ObjectIdInvalidationMap& to_emit) { + DVLOG(2) << "Emitting invalidations: " << to_emit.ToString(); + delegate_->OnInvalidate(to_emit); } void SyncInvalidationListener::InformRegistrationStatus( @@ -388,6 +290,38 @@ void SyncInvalidationListener::InformError( EmitStateChange(); } +void SyncInvalidationListener::Acknowledge( + const invalidation::ObjectId& id, + const syncer::AckHandle& handle) { + UnackedInvalidationsMap::iterator lookup = + unacked_invalidations_map_.find(id); + if (lookup == unacked_invalidations_map_.end()) { + DLOG(WARNING) << "Received acknowledgement for untracked object ID"; + return; + } + lookup->second.Acknowledge(handle); + invalidation_state_tracker_.Call( + FROM_HERE, + &InvalidationStateTracker::SetSavedInvalidations, + unacked_invalidations_map_); +} + +void SyncInvalidationListener::Drop( + const invalidation::ObjectId& id, + const syncer::AckHandle& handle) { + UnackedInvalidationsMap::iterator lookup = + unacked_invalidations_map_.find(id); + if (lookup == unacked_invalidations_map_.end()) { + DLOG(WARNING) << "Received drop for untracked object ID"; + return; + } + lookup->second.Drop(handle); + invalidation_state_tracker_.Call( + FROM_HERE, + &InvalidationStateTracker::SetSavedInvalidations, + unacked_invalidations_map_); +} + void SyncInvalidationListener::WriteState(const std::string& state) { DCHECK(CalledOnValidThread()); DVLOG(1) << "WriteState"; @@ -399,13 +333,31 @@ void SyncInvalidationListener::DoRegistrationUpdate() { DCHECK(CalledOnValidThread()); const ObjectIdSet& unregistered_ids = registration_manager_->UpdateRegisteredIds(registered_ids_); - for (ObjectIdSet::const_iterator it = unregistered_ids.begin(); + for (ObjectIdSet::iterator it = unregistered_ids.begin(); it != unregistered_ids.end(); ++it) { - invalidation_state_map_.erase(*it); + unacked_invalidations_map_.erase(*it); } invalidation_state_tracker_.Call( - FROM_HERE, &InvalidationStateTracker::Forget, unregistered_ids); - ack_tracker_.Ack(unregistered_ids); + FROM_HERE, + &InvalidationStateTracker::SetSavedInvalidations, + unacked_invalidations_map_); + + ObjectIdInvalidationMap object_id_invalidation_map; + for (UnackedInvalidationsMap::iterator map_it = + unacked_invalidations_map_.begin(); + map_it != unacked_invalidations_map_.end(); ++map_it) { + if (registered_ids_.find(map_it->first) == registered_ids_.end()) { + continue; + } + map_it->second.ExportInvalidations( + GetThisAsAckHandler(), + &object_id_invalidation_map); + } + + // There's no need to run these through DispatchInvalidations(); they've + // already been saved to storage (that's where we found them) so all we need + // to do now is emit them. + EmitSavedInvalidations(object_id_invalidation_map); } void SyncInvalidationListener::StopForTest() { @@ -413,23 +365,12 @@ void SyncInvalidationListener::StopForTest() { Stop(); } -InvalidationStateMap SyncInvalidationListener::GetStateMapForTest() const { - DCHECK(CalledOnValidThread()); - return invalidation_state_map_; -} - -AckTracker* SyncInvalidationListener::GetAckTrackerForTest() { - return &ack_tracker_; -} - void SyncInvalidationListener::Stop() { DCHECK(CalledOnValidThread()); if (!invalidation_client_) { return; } - ack_tracker_.Clear(); - registration_manager_.reset(); sync_system_resources_.Stop(); invalidation_client_->Stop(); @@ -437,8 +378,6 @@ void SyncInvalidationListener::Stop() { invalidation_client_.reset(); delegate_ = NULL; - invalidation_state_tracker_.Reset(); - invalidation_state_map_.clear(); ticl_state_ = DEFAULT_INVALIDATION_ERROR; push_client_state_ = DEFAULT_INVALIDATION_ERROR; } @@ -466,6 +405,11 @@ void SyncInvalidationListener::EmitStateChange() { delegate_->OnInvalidatorStateChange(GetState()); } +WeakHandle<AckHandler> SyncInvalidationListener::GetThisAsAckHandler() { + DCHECK(CalledOnValidThread()); + return WeakHandle<AckHandler>(weak_ptr_factory_.GetWeakPtr()); +} + void SyncInvalidationListener::OnNotificationsEnabled() { DCHECK(CalledOnValidThread()); push_client_state_ = INVALIDATIONS_ENABLED; diff --git a/sync/notifier/sync_invalidation_listener.h b/sync/notifier/sync_invalidation_listener.h index 7b7b2fd..a43e57b 100644 --- a/sync/notifier/sync_invalidation_listener.h +++ b/sync/notifier/sync_invalidation_listener.h @@ -20,16 +20,12 @@ #include "jingle/notifier/listener/push_client_observer.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/util/weak_handle.h" -#include "sync/notifier/ack_tracker.h" +#include "sync/notifier/ack_handler.h" #include "sync/notifier/invalidation_state_tracker.h" #include "sync/notifier/invalidator_state.h" -#include "sync/notifier/object_id_invalidation_map.h" #include "sync/notifier/state_writer.h" #include "sync/notifier/sync_system_resources.h" - -namespace base { -class TickClock; -} // namespace base +#include "sync/notifier/unacked_invalidation_set.h" namespace buzz { class XmppTaskParentInterface; @@ -41,6 +37,7 @@ class PushClient; namespace syncer { +class ObjectIdInvalidationMap; class RegistrationManager; // SyncInvalidationListener is not thread-safe and lives on the sync @@ -49,8 +46,8 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener : public NON_EXPORTED_BASE(invalidation::InvalidationListener), public StateWriter, public NON_EXPORTED_BASE(notifier::PushClientObserver), - public base::NonThreadSafe, - public AckTracker::Delegate { + public AckHandler, + public base::NonThreadSafe { public: typedef base::Callback<invalidation::InvalidationClient*( invalidation::SystemResources*, @@ -64,13 +61,12 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener virtual ~Delegate(); virtual void OnInvalidate( - const ObjectIdInvalidationMap& invalidation_map) = 0; + const ObjectIdInvalidationMap& invalidations) = 0; virtual void OnInvalidatorStateChange(InvalidatorState state) = 0; }; explicit SyncInvalidationListener( - base::TickClock* tick_clock, scoped_ptr<notifier::PushClient> push_client); // Calls Stop(). @@ -83,7 +79,7 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener create_invalidation_client_callback, const std::string& client_id, const std::string& client_info, const std::string& invalidation_bootstrap_data, - const InvalidationStateMap& initial_invalidation_state_map, + const UnackedInvalidationsMap& initial_object_states, const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker, Delegate* delegate); @@ -92,9 +88,6 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener // Update the set of object IDs that we're interested in getting // notifications for. May be called at any time. void UpdateRegisteredIds(const ObjectIdSet& ids); - // Acknowledge that an invalidation for |id| was handled. - void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle); // invalidation::InvalidationListener implementation. virtual void Ready( @@ -127,6 +120,14 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener invalidation::InvalidationClient* client, const invalidation::ErrorInfo& error_info) OVERRIDE; + // AckHandler implementation. + virtual void Acknowledge( + const invalidation::ObjectId& id, + const syncer::AckHandle& handle) OVERRIDE; + virtual void Drop( + const invalidation::ObjectId& id, + const syncer::AckHandle& handle) OVERRIDE; + // StateWriter implementation. virtual void WriteState(const std::string& state) OVERRIDE; @@ -140,8 +141,6 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener void DoRegistrationUpdate(); void StopForTest(); - InvalidationStateMap GetStateMapForTest() const; - AckTracker* GetAckTrackerForTest(); private: void Stop(); @@ -150,27 +149,31 @@ class SYNC_EXPORT_PRIVATE SyncInvalidationListener void EmitStateChange(); - void PrepareInvalidation(const ObjectIdSet& ids, - int64 version, - const std::string& payload, - invalidation::InvalidationClient* client, - const invalidation::AckHandle& ack_handle); - void EmitInvalidation(const ObjectIdSet& ids, - int64 version, - const std::string& payload, - invalidation::InvalidationClient* client, - const invalidation::AckHandle& ack_handle, - const AckHandleMap& local_ack_handles); + // Sends invalidations to their appropriate destination. + // + // If there are no observers registered for them, they will be saved for + // later. + // + // If there are observers registered, they will be saved (to make sure we + // don't drop them until they've been acted on) and emitted to the observers. + void DispatchInvalidations(const ObjectIdInvalidationMap& invalidations); + + // Saves invalidations. + // + // This call isn't synchronous so we can't guarantee these invalidations will + // be safely on disk by the end of the call, but it should ensure that the + // data makes it to disk eventually. + void SaveInvalidations(const ObjectIdInvalidationMap& to_save); - // AckTracker::Delegate implementation. - virtual void OnTimeout(const ObjectIdSet& ids) OVERRIDE; + // Emits previously saved invalidations to their registered observers. + void EmitSavedInvalidations(const ObjectIdInvalidationMap& to_emit); - AckTracker ack_tracker_; + WeakHandle<AckHandler> GetThisAsAckHandler(); // Owned by |sync_system_resources_|. notifier::PushClient* const push_client_; SyncSystemResources sync_system_resources_; - InvalidationStateMap invalidation_state_map_; + UnackedInvalidationsMap unacked_invalidations_map_; WeakHandle<InvalidationStateTracker> invalidation_state_tracker_; Delegate* delegate_; scoped_ptr<invalidation::InvalidationClient> invalidation_client_; diff --git a/sync/notifier/sync_invalidation_listener_unittest.cc b/sync/notifier/sync_invalidation_listener_unittest.cc index 18dd123..2808b97 100644 --- a/sync/notifier/sync_invalidation_listener_unittest.cc +++ b/sync/notifier/sync_invalidation_listener_unittest.cc @@ -3,23 +3,24 @@ // found in the LICENSE file. #include <cstddef> +#include <map> #include <set> #include <string> +#include <vector> #include "base/compiler_specific.h" #include "base/message_loop/message_loop.h" #include "base/stl_util.h" -#include "base/time/tick_clock.h" -#include "base/time/time.h" #include "google/cacheinvalidation/include/invalidation-client.h" #include "google/cacheinvalidation/include/types.h" #include "jingle/notifier/listener/fake_push_client.h" -#include "sync/internal_api/public/base/invalidation_test_util.h" #include "sync/internal_api/public/util/weak_handle.h" -#include "sync/notifier/ack_tracker.h" +#include "sync/notifier/dropped_invalidation_tracker.h" #include "sync/notifier/fake_invalidation_state_tracker.h" #include "sync/notifier/invalidation_util.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "sync/notifier/sync_invalidation_listener.h" +#include "sync/notifier/unacked_invalidation_set_test_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { @@ -38,7 +39,6 @@ const char kNewState[] = "new_state"; const char kPayload1[] = "payload1"; const char kPayload2[] = "payload2"; -const int64 kMinVersion = FakeInvalidationStateTracker::kMinVersion; const int64 kVersion1 = 1LL; const int64 kVersion2 = 2LL; @@ -137,8 +137,8 @@ class FakeInvalidationClient : public invalidation::InvalidationClient { class FakeDelegate : public SyncInvalidationListener::Delegate { public: explicit FakeDelegate(SyncInvalidationListener* listener) - : listener_(listener), - state_(TRANSIENT_INVALIDATION_ERROR) {} + : state_(TRANSIENT_INVALIDATION_ERROR), + drop_handlers_deleter_(&drop_handlers_) {} virtual ~FakeDelegate() {} size_t GetInvalidationCount(const ObjectId& id) const { @@ -164,7 +164,7 @@ class FakeDelegate : public SyncInvalidationListener::Delegate { Map::const_iterator it = invalidations_.find(id); if (it == invalidations_.end()) { ADD_FAILURE() << "No invalidations for ID " << ObjectIdToString(id); - return ""; + return 0; } else { return it->second.back().payload(); } @@ -179,16 +179,58 @@ class FakeDelegate : public SyncInvalidationListener::Delegate { return it->second.back().is_unknown_version(); } } + + bool StartsWithUnknownVersion(const ObjectId& id) const { + Map::const_iterator it = invalidations_.find(id); + if (it == invalidations_.end()) { + ADD_FAILURE() << "No invalidations for ID " << ObjectIdToString(id); + return false; + } else { + return it->second.front().is_unknown_version(); + } + } + InvalidatorState GetInvalidatorState() const { return state_; } - void Acknowledge(const ObjectId& id) { - listener_->Acknowledge(id, invalidations_[id].back().ack_handle()); + DroppedInvalidationTracker* GetDropTrackerForObject(const ObjectId& id) { + DropHandlers::iterator it = drop_handlers_.find(id); + if (it == drop_handlers_.end()) { + drop_handlers_.insert( + std::make_pair(id, new DroppedInvalidationTracker(id))); + return drop_handlers_.find(id)->second; + } else { + return it->second; + } } - // SyncInvalidationListener::Delegate implementation. + void AcknowledgeNthInvalidation(const ObjectId& id, size_t n) { + List& list = invalidations_[id]; + List::iterator it = list.begin() + n; + it->Acknowledge(); + } + void AcknowledgeAll(const ObjectId& id) { + List& list = invalidations_[id]; + for (List::iterator it = list.begin(); it != list.end(); ++it) { + it->Acknowledge(); + } + } + + void DropNthInvalidation(const ObjectId& id, size_t n) { + DroppedInvalidationTracker* drop_tracker = GetDropTrackerForObject(id); + List& list = invalidations_[id]; + List::iterator it = list.begin() + n; + it->Drop(drop_tracker); + } + + void RecoverFromDropEvent(const ObjectId& id) { + DroppedInvalidationTracker* drop_tracker = GetDropTrackerForObject(id); + drop_tracker->RecordRecoveryFromDropEvent(); + } + + // SyncInvalidationListener::Delegate implementation. virtual void OnInvalidate( const ObjectIdInvalidationMap& invalidation_map) OVERRIDE { ObjectIdSet ids = invalidation_map.GetObjectIds(); @@ -205,12 +247,16 @@ class FakeDelegate : public SyncInvalidationListener::Delegate { } private: - typedef std::map<ObjectId, int, ObjectIdLessThan> ObjectIdCountMap; typedef std::vector<Invalidation> List; typedef std::map<ObjectId, List, ObjectIdLessThan> Map; + typedef std::map<ObjectId, + DroppedInvalidationTracker*, + ObjectIdLessThan> DropHandlers; + Map invalidations_; - SyncInvalidationListener* listener_; InvalidatorState state_; + DropHandlers drop_handlers_; + STLValueDeleter<DropHandlers> drop_handlers_deleter_; }; invalidation::InvalidationClient* CreateFakeInvalidationClient( @@ -224,50 +270,6 @@ invalidation::InvalidationClient* CreateFakeInvalidationClient( return *fake_invalidation_client; } -// TODO(dcheng): FakeTickClock and FakeBackoffEntry ought to be factored out -// into a helpers file so it can be shared with the AckTracker unittest. -class FakeTickClock : public base::TickClock { - public: - FakeTickClock() {} - virtual ~FakeTickClock() {} - - void LeapForward(int seconds) { - ASSERT_GT(seconds, 0); - fake_now_ticks_ += base::TimeDelta::FromSeconds(seconds); - } - - virtual base::TimeTicks NowTicks() OVERRIDE { - return fake_now_ticks_; - } - - private: - base::TimeTicks fake_now_ticks_; - - DISALLOW_COPY_AND_ASSIGN(FakeTickClock); -}; - -class FakeBackoffEntry : public net::BackoffEntry { - public: - FakeBackoffEntry(const Policy *const policy, base::TickClock* tick_clock) - : BackoffEntry(policy), tick_clock_(tick_clock) { - } - - protected: - virtual base::TimeTicks ImplGetTimeNow() const OVERRIDE { - return tick_clock_->NowTicks(); - } - - private: - base::TickClock* const tick_clock_; -}; - -scoped_ptr<net::BackoffEntry> CreateMockEntry( - base::TickClock* tick_clock, - const net::BackoffEntry::Policy *const policy) { - return scoped_ptr<net::BackoffEntry>( - new FakeBackoffEntry(policy, tick_clock)); -} - class SyncInvalidationListenerTest : public testing::Test { protected: SyncInvalidationListenerTest() @@ -277,8 +279,7 @@ class SyncInvalidationListenerTest : public testing::Test { kAppsId_(kChromeSyncSourceId, "APP"), fake_push_client_(new notifier::FakePushClient()), fake_invalidation_client_(NULL), - listener_(&tick_clock_, - scoped_ptr<notifier::PushClient>(fake_push_client_)), + listener_(scoped_ptr<notifier::PushClient>(fake_push_client_)), fake_delegate_(&listener_) {} virtual void SetUp() { @@ -304,7 +305,7 @@ class SyncInvalidationListenerTest : public testing::Test { listener_.Start(base::Bind(&CreateFakeInvalidationClient, &fake_invalidation_client_), kClientId, kClientInfo, kState, - fake_tracker_.GetAllInvalidationStates(), + fake_tracker_.GetSavedInvalidations(), MakeWeakHandle(fake_tracker_.AsWeakPtr()), &fake_delegate_); DCHECK(fake_invalidation_client_); @@ -317,12 +318,12 @@ class SyncInvalidationListenerTest : public testing::Test { // avoid leaking the inner task. listener_.StopForTest() does not // schedule any tasks, so it's both necessary and sufficient to // drain the task queue before calling it. - message_loop_.RunUntilIdle(); + FlushPendingWrites(); fake_invalidation_client_ = NULL; listener_.StopForTest(); } - int GetInvalidationCount(const ObjectId& id) const { + size_t GetInvalidationCount(const ObjectId& id) const { return fake_delegate_.GetInvalidationCount(id); } @@ -338,12 +339,28 @@ class SyncInvalidationListenerTest : public testing::Test { return fake_delegate_.IsUnknownVersion(id); } - InvalidatorState GetInvalidatorState() const { - return fake_delegate_.GetInvalidatorState(); + bool StartsWithUnknownVersion(const ObjectId& id) const { + return fake_delegate_.StartsWithUnknownVersion(id); + } + + void AcknowledgeNthInvalidation(const ObjectId& id, size_t n) { + fake_delegate_.AcknowledgeNthInvalidation(id, n); + } + + void DropNthInvalidation(const ObjectId& id, size_t n) { + return fake_delegate_.DropNthInvalidation(id, n); + } + + void RecoverFromDropEvent(const ObjectId& id) { + return fake_delegate_.RecoverFromDropEvent(id); } - int64 GetMaxVersion(const ObjectId& id) const { - return fake_tracker_.GetMaxVersion(id); + void AcknowledgeAll(const ObjectId& id) { + fake_delegate_.AcknowledgeAll(id); + } + + InvalidatorState GetInvalidatorState() const { + return fake_delegate_.GetInvalidatorState(); } std::string GetInvalidatorClientId() const { @@ -354,6 +371,29 @@ class SyncInvalidationListenerTest : public testing::Test { return fake_tracker_.GetBootstrapData(); } + UnackedInvalidationsMap GetSavedInvalidations() { + // Allow any queued writes to go through first. + FlushPendingWrites(); + return fake_tracker_.GetSavedInvalidations(); + } + + SingleObjectInvalidationSet GetSavedInvalidationsForType(const ObjectId& id) { + const UnackedInvalidationsMap& saved_state = GetSavedInvalidations(); + UnackedInvalidationsMap::const_iterator it = + saved_state.find(kBookmarksId_); + if (it == saved_state.end()) { + ADD_FAILURE() << "No state saved for ID " << ObjectIdToString(id); + return SingleObjectInvalidationSet(); + } + ObjectIdInvalidationMap map; + it->second.ExportInvalidations(WeakHandle<AckHandler>(), &map); + if (map.Empty()) { + return SingleObjectInvalidationSet(); + } else { + return map.ForObject(id); + } + } + ObjectIdSet GetRegisteredIds() const { return fake_invalidation_client_->GetRegisteredIds(); } @@ -370,9 +410,6 @@ class SyncInvalidationListenerTest : public testing::Test { const AckHandle ack_handle("fakedata"); fake_invalidation_client_->ClearAckedHandles(); listener_.Invalidate(fake_invalidation_client_, inv, ack_handle); - // Pump message loop to trigger InvalidationStateTracker::SetMaxVersion() - // and callback from InvalidationStateTracker::GenerateAckHandles(). - message_loop_.RunUntilIdle(); EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle)); } @@ -380,11 +417,9 @@ class SyncInvalidationListenerTest : public testing::Test { void FireInvalidateUnknownVersion(const ObjectId& object_id) { const AckHandle ack_handle("fakedata_unknown"); fake_invalidation_client_->ClearAckedHandles(); - listener_.InvalidateUnknownVersion(fake_invalidation_client_, object_id, - ack_handle); - // Pump message loop to trigger callback from - // InvalidationStateTracker::GenerateAckHandles(). - message_loop_.RunUntilIdle(); + listener_.InvalidateUnknownVersion(fake_invalidation_client_, + object_id, + ack_handle); EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle)); } @@ -392,16 +427,18 @@ class SyncInvalidationListenerTest : public testing::Test { const AckHandle ack_handle("fakedata_all"); fake_invalidation_client_->ClearAckedHandles(); listener_.InvalidateAll(fake_invalidation_client_, ack_handle); - // Pump message loop to trigger callback from - // InvalidationStateTracker::GenerateAckHandles(). - message_loop_.RunUntilIdle(); EXPECT_TRUE(fake_invalidation_client_->IsAckedHandle(ack_handle)); } void WriteState(const std::string& new_state) { listener_.WriteState(new_state); + // Pump message loop to trigger // InvalidationStateTracker::WriteState(). + FlushPendingWrites(); + } + + void FlushPendingWrites() { message_loop_.RunUntilIdle(); } @@ -413,29 +450,6 @@ class SyncInvalidationListenerTest : public testing::Test { fake_push_client_->DisableNotifications(reason); } - void VerifyUnacknowledged(const ObjectId& object_id) { - InvalidationStateMap state_map = fake_tracker_.GetAllInvalidationStates(); - EXPECT_THAT(state_map[object_id].current, - Not(Eq(state_map[object_id].expected))); - EXPECT_EQ(listener_.GetStateMapForTest(), state_map); - } - - void VerifyAcknowledged(const ObjectId& object_id) { - InvalidationStateMap state_map = fake_tracker_.GetAllInvalidationStates(); - EXPECT_THAT(state_map[object_id].current, - Eq(state_map[object_id].expected)); - EXPECT_EQ(listener_.GetStateMapForTest(), state_map); - } - - void AcknowledgeAndVerify(const ObjectId& object_id) { - VerifyUnacknowledged(object_id); - fake_delegate_.Acknowledge(object_id); - // Pump message loop to trigger - // InvalidationStateTracker::Acknowledge(). - message_loop_.RunUntilIdle(); - VerifyAcknowledged(object_id); - } - const ObjectId kBookmarksId_; const ObjectId kPreferencesId_; const ObjectId kExtensionsId_; @@ -445,13 +459,14 @@ class SyncInvalidationListenerTest : public testing::Test { private: base::MessageLoop message_loop_; - FakeInvalidationStateTracker fake_tracker_; notifier::FakePushClient* const fake_push_client_; protected: + // A derrived test needs direct access to this. + FakeInvalidationStateTracker fake_tracker_; + // Tests need to access these directly. FakeInvalidationClient* fake_invalidation_client_; - FakeTickClock tick_clock_; SyncInvalidationListener listener_; private: @@ -475,11 +490,10 @@ TEST_F(SyncInvalidationListenerTest, InvalidateNoPayload) { FireInvalidate(id, kVersion1, NULL); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); EXPECT_EQ(kVersion1, GetVersion(id)); EXPECT_EQ("", GetPayload(id)); - EXPECT_EQ(kVersion1, GetMaxVersion(id)); - AcknowledgeAndVerify(id); } // Fire an invalidation with an empty payload. It should be @@ -490,11 +504,10 @@ TEST_F(SyncInvalidationListenerTest, InvalidateEmptyPayload) { FireInvalidate(id, kVersion1, ""); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); EXPECT_EQ(kVersion1, GetVersion(id)); EXPECT_EQ("", GetPayload(id)); - EXPECT_EQ(kVersion1, GetMaxVersion(id)); - AcknowledgeAndVerify(id); } // Fire an invalidation with a payload. It should be processed, and @@ -504,251 +517,133 @@ TEST_F(SyncInvalidationListenerTest, InvalidateWithPayload) { FireInvalidate(id, kVersion1, kPayload1); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); EXPECT_EQ(kVersion1, GetVersion(id)); EXPECT_EQ(kPayload1, GetPayload(id)); - EXPECT_EQ(kVersion1, GetMaxVersion(id)); - AcknowledgeAndVerify(id); +} + +// Fire ten invalidations in a row. All should be received. +TEST_F(SyncInvalidationListenerTest, ManyInvalidations_NoDrop) { + const int kRepeatCount = 10; + const ObjectId& id = kPreferencesId_; + int64 initial_version = kVersion1; + for (int64 i = initial_version; i < initial_version + kRepeatCount; ++i) { + FireInvalidate(id, i, kPayload1); + } + ASSERT_EQ(static_cast<size_t>(kRepeatCount), GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); + EXPECT_EQ(kPayload1, GetPayload(id)); + EXPECT_EQ(initial_version + kRepeatCount - 1, GetVersion(id)); } // Fire an invalidation for an unregistered object ID with a payload. It should // still be processed, and both the payload and the version should be updated. -TEST_F(SyncInvalidationListenerTest, InvalidateUnregisteredWithPayload) { - const ObjectId kUnregisteredId( - kChromeSyncSourceId, "unregistered"); +TEST_F(SyncInvalidationListenerTest, InvalidateBeforeRegistration_Simple) { + const ObjectId kUnregisteredId(kChromeSyncSourceId, "unregistered"); const ObjectId& id = kUnregisteredId; + ObjectIdSet ids; + ids.insert(id); - EXPECT_EQ(0, GetInvalidationCount(id)); - EXPECT_EQ(kMinVersion, GetMaxVersion(id)); + EXPECT_EQ(0U, GetInvalidationCount(id)); - FireInvalidate(id, kVersion1, "unregistered payload"); + FireInvalidate(id, kVersion1, kPayload1); + + ASSERT_EQ(0U, GetInvalidationCount(id)); + + EnableNotifications(); + listener_.Ready(fake_invalidation_client_); + listener_.UpdateRegisteredIds(ids); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); EXPECT_EQ(kVersion1, GetVersion(id)); - EXPECT_EQ("unregistered payload", GetPayload(id)); - EXPECT_EQ(kVersion1, GetMaxVersion(id)); - AcknowledgeAndVerify(id); + EXPECT_EQ(kPayload1, GetPayload(id)); } -// Fire an invalidation, then fire another one with a lower version. -// The first one should be processed and should update the payload and -// version, but the second one shouldn't. +// Fire ten invalidations before an object registers. Some invalidations will +// be dropped an replaced with an unknown version invalidation. +TEST_F(SyncInvalidationListenerTest, InvalidateBeforeRegistration_Drop) { + const int kRepeatCount = + UnackedInvalidationSet::kMaxBufferedInvalidations + 1; + const ObjectId kUnregisteredId(kChromeSyncSourceId, "unregistered"); + const ObjectId& id = kUnregisteredId; + ObjectIdSet ids; + ids.insert(id); + + EXPECT_EQ(0U, GetInvalidationCount(id)); + + int64 initial_version = kVersion1; + for (int64 i = initial_version; i < initial_version + kRepeatCount; ++i) { + FireInvalidate(id, i, kPayload1); + } + + EnableNotifications(); + listener_.Ready(fake_invalidation_client_); + listener_.UpdateRegisteredIds(ids); + + ASSERT_EQ(UnackedInvalidationSet::kMaxBufferedInvalidations, + GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); + EXPECT_EQ(initial_version + kRepeatCount - 1, GetVersion(id)); + EXPECT_EQ(kPayload1, GetPayload(id)); + EXPECT_TRUE(StartsWithUnknownVersion(id)); +} + +// Fire an invalidation, then fire another one with a lower version. Both +// should be received. TEST_F(SyncInvalidationListenerTest, InvalidateVersion) { const ObjectId& id = kPreferencesId_; FireInvalidate(id, kVersion2, kPayload2); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); EXPECT_EQ(kVersion2, GetVersion(id)); EXPECT_EQ(kPayload2, GetPayload(id)); - EXPECT_EQ(kVersion2, GetMaxVersion(id)); - AcknowledgeAndVerify(id); FireInvalidate(id, kVersion1, kPayload1); - EXPECT_EQ(1, GetInvalidationCount(id)); - EXPECT_EQ(kVersion2, GetVersion(id)); - EXPECT_EQ(kPayload2, GetPayload(id)); - EXPECT_EQ(kVersion2, GetMaxVersion(id)); - VerifyAcknowledged(id); + ASSERT_EQ(2U, GetInvalidationCount(id)); + ASSERT_FALSE(IsUnknownVersion(id)); + + EXPECT_EQ(kVersion1, GetVersion(id)); + EXPECT_EQ(kPayload1, GetPayload(id)); } -// Fire an invalidation with an unknown version twice. It shouldn't update the -// version either time, but it should still be processed. +// Fire an invalidation with an unknown version. TEST_F(SyncInvalidationListenerTest, InvalidateUnknownVersion) { const ObjectId& id = kBookmarksId_; FireInvalidateUnknownVersion(id); - EXPECT_EQ(1, GetInvalidationCount(id)); + ASSERT_EQ(1U, GetInvalidationCount(id)); EXPECT_TRUE(IsUnknownVersion(id)); - AcknowledgeAndVerify(id); - - FireInvalidateUnknownVersion(id); - EXPECT_EQ(kMinVersion, GetMaxVersion(id)); - AcknowledgeAndVerify(id); } -// Fire an invalidation for all enabled IDs. It shouldn't update the -// payload or version, but it should still invalidate the IDs. +// Fire an invalidation for all enabled IDs. TEST_F(SyncInvalidationListenerTest, InvalidateAll) { FireInvalidateAll(); for (ObjectIdSet::const_iterator it = registered_ids_.begin(); it != registered_ids_.end(); ++it) { - EXPECT_EQ(1, GetInvalidationCount(*it)); + ASSERT_EQ(1U, GetInvalidationCount(*it)); EXPECT_TRUE(IsUnknownVersion(*it)); - EXPECT_EQ(kMinVersion, GetMaxVersion(*it)); - AcknowledgeAndVerify(*it); } } -// Comprehensive test of various scenarios for multiple IDs. +// Test a simple scenario for multiple IDs. TEST_F(SyncInvalidationListenerTest, InvalidateMultipleIds) { FireInvalidate(kBookmarksId_, 3, NULL); - EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_)); + ASSERT_EQ(1U, GetInvalidationCount(kBookmarksId_)); + ASSERT_FALSE(IsUnknownVersion(kBookmarksId_)); EXPECT_EQ(3, GetVersion(kBookmarksId_)); EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - AcknowledgeAndVerify(kBookmarksId_); + // kExtensionId is not registered, so the invalidation should not get through. FireInvalidate(kExtensionsId_, 2, NULL); - - EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_)); - EXPECT_EQ(2, GetVersion(kExtensionsId_)); - EXPECT_EQ("", GetPayload(kExtensionsId_)); - EXPECT_EQ(2, GetMaxVersion(kExtensionsId_)); - AcknowledgeAndVerify(kExtensionsId_); - - // Invalidations with lower version numbers should be ignored. - - FireInvalidate(kBookmarksId_, 1, NULL); - - EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_)); - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - FireInvalidate(kExtensionsId_, 1, NULL); - - EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_)); - EXPECT_EQ(2, GetVersion(kExtensionsId_)); - EXPECT_EQ("", GetPayload(kExtensionsId_)); - EXPECT_EQ(2, GetMaxVersion(kExtensionsId_)); - - // InvalidateAll shouldn't change any version state. - - FireInvalidateAll(); - - EXPECT_EQ(2, GetInvalidationCount(kBookmarksId_)); - EXPECT_TRUE(IsUnknownVersion(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - AcknowledgeAndVerify(kBookmarksId_); - - EXPECT_EQ(1, GetInvalidationCount(kPreferencesId_)); - EXPECT_TRUE(IsUnknownVersion(kBookmarksId_)); - EXPECT_EQ(kMinVersion, GetMaxVersion(kPreferencesId_)); - AcknowledgeAndVerify(kPreferencesId_); - - // Note that kExtensionsId_ is not registered, so InvalidateAll() shouldn't - // affect it. - EXPECT_EQ(1, GetInvalidationCount(kExtensionsId_)); - EXPECT_EQ(2, GetVersion(kExtensionsId_)); - EXPECT_EQ("", GetPayload(kExtensionsId_)); - EXPECT_EQ(2, GetMaxVersion(kExtensionsId_)); - VerifyAcknowledged(kExtensionsId_); - - // Invalidations with higher version numbers should be processed. - - FireInvalidate(kPreferencesId_, 5, NULL); - EXPECT_EQ(2, GetInvalidationCount(kPreferencesId_)); - EXPECT_EQ(5, GetVersion(kPreferencesId_)); - EXPECT_EQ("", GetPayload(kPreferencesId_)); - EXPECT_EQ(5, GetMaxVersion(kPreferencesId_)); - AcknowledgeAndVerify(kPreferencesId_); - - FireInvalidate(kExtensionsId_, 3, NULL); - EXPECT_EQ(2, GetInvalidationCount(kExtensionsId_)); - EXPECT_EQ(3, GetVersion(kExtensionsId_)); - EXPECT_EQ("", GetPayload(kExtensionsId_)); - EXPECT_EQ(3, GetMaxVersion(kExtensionsId_)); - AcknowledgeAndVerify(kExtensionsId_); - - FireInvalidate(kBookmarksId_, 4, NULL); - EXPECT_EQ(3, GetInvalidationCount(kBookmarksId_)); - EXPECT_EQ(4, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(4, GetMaxVersion(kBookmarksId_)); - AcknowledgeAndVerify(kBookmarksId_); -} - -// Various tests for the local invalidation feature. -// Tests a "normal" scenario. We allow one timeout period to expire by sending -// ack handles that are not the "latest" ack handle. Once the timeout expires, -// we verify that we get a second callback and then acknowledge it. Once -// acknowledged, no further timeouts should occur. -TEST_F(SyncInvalidationListenerTest, InvalidateOneTimeout) { - listener_.GetAckTrackerForTest()->SetCreateBackoffEntryCallbackForTest( - base::Bind(&CreateMockEntry, &tick_clock_)); - - // Trigger the initial invalidation. - FireInvalidate(kBookmarksId_, 3, NULL); - EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_)); - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - VerifyUnacknowledged(kBookmarksId_); - - // Trigger one timeout. - tick_clock_.LeapForward(60); - EXPECT_TRUE(listener_.GetAckTrackerForTest()->TriggerTimeoutAtForTest( - tick_clock_.NowTicks())); - EXPECT_EQ(2, GetInvalidationCount(kBookmarksId_)); - // Other properties should remain the same. - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - AcknowledgeAndVerify(kBookmarksId_); - - // No more invalidations should remain in the queue. - EXPECT_TRUE(listener_.GetAckTrackerForTest()->IsQueueEmptyForTest()); -} - -// Test that an unacknowledged invalidation triggers reminders if the listener -// is restarted. -TEST_F(SyncInvalidationListenerTest, InvalidationTimeoutRestart) { - listener_.GetAckTrackerForTest()->SetCreateBackoffEntryCallbackForTest( - base::Bind(&CreateMockEntry, &tick_clock_)); - - FireInvalidate(kBookmarksId_, 3, NULL); - EXPECT_EQ(1, GetInvalidationCount(kBookmarksId_)); - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - // Trigger one timeout. - tick_clock_.LeapForward(60); - EXPECT_TRUE(listener_.GetAckTrackerForTest()->TriggerTimeoutAtForTest( - tick_clock_.NowTicks())); - EXPECT_EQ(2, GetInvalidationCount(kBookmarksId_)); - // Other properties should remain the same. - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - // Restarting the client should reset the retry count and the timeout period - // (e.g. it shouldn't increase to 120 seconds). Skip ahead 1200 seconds to be - // on the safe side. - StopClient(); - tick_clock_.LeapForward(1200); - StartClient(); - - // The bookmark invalidation state should not have changed. - EXPECT_EQ(2, GetInvalidationCount(kBookmarksId_)); - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - // Now trigger the invalidation reminder after the client restarts. - tick_clock_.LeapForward(60); - EXPECT_TRUE(listener_.GetAckTrackerForTest()->TriggerTimeoutAtForTest( - tick_clock_.NowTicks())); - EXPECT_EQ(3, GetInvalidationCount(kBookmarksId_)); - // Other properties should remain the same. - EXPECT_EQ(3, GetVersion(kBookmarksId_)); - EXPECT_EQ("", GetPayload(kBookmarksId_)); - EXPECT_EQ(3, GetMaxVersion(kBookmarksId_)); - - AcknowledgeAndVerify(kBookmarksId_); - - // No more invalidations should remain in the queue. - EXPECT_TRUE(listener_.GetAckTrackerForTest()->IsQueueEmptyForTest()); - - // The queue should remain empty when we restart now. - RestartClient(); - EXPECT_TRUE(listener_.GetAckTrackerForTest()->IsQueueEmptyForTest()); + ASSERT_EQ(0U, GetInvalidationCount(kExtensionsId_)); } // Registration tests. @@ -890,22 +785,183 @@ TEST_F(SyncInvalidationListenerTest, RegisterTypesPreserved) { // Make sure that state is correctly purged from the local invalidation state // map cache when an ID is unregistered. TEST_F(SyncInvalidationListenerTest, UnregisterCleansUpStateMapCache) { + const ObjectId& id = kBookmarksId_; listener_.Ready(fake_invalidation_client_); - EXPECT_TRUE(listener_.GetStateMapForTest().empty()); - FireInvalidate(kBookmarksId_, 1, "hello"); - EXPECT_EQ(1U, listener_.GetStateMapForTest().size()); - EXPECT_TRUE(ContainsKey(listener_.GetStateMapForTest(), kBookmarksId_)); + EXPECT_TRUE(GetSavedInvalidations().empty()); + FireInvalidate(id, 1, "hello"); + EXPECT_EQ(1U, GetSavedInvalidations().size()); + EXPECT_TRUE(ContainsKey(GetSavedInvalidations(), id)); FireInvalidate(kPreferencesId_, 2, "world"); - EXPECT_EQ(2U, listener_.GetStateMapForTest().size()); - EXPECT_TRUE(ContainsKey(listener_.GetStateMapForTest(), kBookmarksId_)); - EXPECT_TRUE(ContainsKey(listener_.GetStateMapForTest(), kPreferencesId_)); + EXPECT_EQ(2U, GetSavedInvalidations().size()); + + EXPECT_TRUE(ContainsKey(GetSavedInvalidations(), id)); + EXPECT_TRUE(ContainsKey(GetSavedInvalidations(), kPreferencesId_)); ObjectIdSet ids; - ids.insert(kBookmarksId_); + ids.insert(id); listener_.UpdateRegisteredIds(ids); - EXPECT_EQ(1U, listener_.GetStateMapForTest().size()); - EXPECT_TRUE(ContainsKey(listener_.GetStateMapForTest(), kBookmarksId_)); + EXPECT_EQ(1U, GetSavedInvalidations().size()); + EXPECT_TRUE(ContainsKey(GetSavedInvalidations(), id)); +} + +TEST_F(SyncInvalidationListenerTest, DuplicateInvaldiations_Simple) { + const ObjectId& id = kBookmarksId_; + listener_.Ready(fake_invalidation_client_); + + // Send a stream of invalidations, including two copies of the second. + FireInvalidate(id, 1, "one"); + FireInvalidate(id, 2, "two"); + FireInvalidate(id, 3, "three"); + FireInvalidate(id, 2, "two"); + + // Expect that the duplicate was discarded. + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + EXPECT_EQ(3U, list.GetSize()); + SingleObjectInvalidationSet::const_iterator it = list.begin(); + EXPECT_EQ(1, it->version()); + it++; + EXPECT_EQ(2, it->version()); + it++; + EXPECT_EQ(3, it->version()); +} + +TEST_F(SyncInvalidationListenerTest, DuplicateInvalidations_NearBufferLimit) { + const size_t kPairsToSend = UnackedInvalidationSet::kMaxBufferedInvalidations; + const ObjectId& id = kBookmarksId_; + listener_.Ready(fake_invalidation_client_); + + // We will have enough buffer space in the state tracker for all these + // invalidations only if duplicates are ignored. + for (size_t i = 0; i < kPairsToSend; ++i) { + FireInvalidate(id, i, "payload"); + FireInvalidate(id, i, "payload"); + } + + // Expect that the state map ignored duplicates. + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + EXPECT_EQ(kPairsToSend, list.GetSize()); + EXPECT_FALSE(list.begin()->is_unknown_version()); + + // Expect that all invalidations (including duplicates) were emitted. + EXPECT_EQ(kPairsToSend*2, GetInvalidationCount(id)); + + // Acknowledge all invalidations to clear the internal state. + AcknowledgeAll(id); + EXPECT_TRUE(GetSavedInvalidationsForType(id).IsEmpty()); +} + +TEST_F(SyncInvalidationListenerTest, DuplicateInvalidations_UnknownVersion) { + const ObjectId& id = kBookmarksId_; + listener_.Ready(fake_invalidation_client_); + + FireInvalidateUnknownVersion(id); + FireInvalidateUnknownVersion(id); + + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + EXPECT_EQ(1U, list.GetSize()); + } + + // Acknowledge the second. There should be no effect on the stored list. + ASSERT_EQ(2U, GetInvalidationCount(id)); + AcknowledgeNthInvalidation(id, 1); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + EXPECT_EQ(1U, list.GetSize()); + } + + // Acknowledge the first. This should remove the invalidation from the list. + ASSERT_EQ(2U, GetInvalidationCount(id)); + AcknowledgeNthInvalidation(id, 0); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + EXPECT_EQ(0U, list.GetSize()); + } +} + +// Make sure that acknowledgements erase items from the local store. +TEST_F(SyncInvalidationListenerTest, AcknowledgementsCleanUpStateMapCache) { + const ObjectId& id = kBookmarksId_; + listener_.Ready(fake_invalidation_client_); + + EXPECT_TRUE(GetSavedInvalidations().empty()); + FireInvalidate(id, 10, "hello"); + FireInvalidate(id, 20, "world"); + FireInvalidateUnknownVersion(id); + + // Expect that all three invalidations have been saved to permanent storage. + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(3U, list.GetSize()); + EXPECT_TRUE(list.begin()->is_unknown_version()); + EXPECT_EQ(20, list.back().version()); + } + + // Acknowledge the second sent invaldiation (version 20) and verify it was + // removed from storage. + AcknowledgeNthInvalidation(id, 1); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(2U, list.GetSize()); + EXPECT_TRUE(list.begin()->is_unknown_version()); + EXPECT_EQ(10, list.back().version()); + } + + // Acknowledge the last sent invalidation (unknown version) and verify it was + // removed from storage. + AcknowledgeNthInvalidation(id, 2); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(1U, list.GetSize()); + EXPECT_FALSE(list.begin()->is_unknown_version()); + EXPECT_EQ(10, list.back().version()); + } +} + +// Make sure that drops erase items from the local store. +TEST_F(SyncInvalidationListenerTest, DropsCleanUpStateMapCache) { + const ObjectId& id = kBookmarksId_; + listener_.Ready(fake_invalidation_client_); + + EXPECT_TRUE(GetSavedInvalidations().empty()); + FireInvalidate(id, 10, "hello"); + FireInvalidate(id, 20, "world"); + FireInvalidateUnknownVersion(id); + + // Expect that all three invalidations have been saved to permanent storage. + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(3U, list.GetSize()); + EXPECT_TRUE(list.begin()->is_unknown_version()); + EXPECT_EQ(20, list.back().version()); + } + + // Drop the second sent invalidation (version 20) and verify it was removed + // from storage. Also verify we still have an unknown version invalidation. + DropNthInvalidation(id, 1); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(2U, list.GetSize()); + EXPECT_TRUE(list.begin()->is_unknown_version()); + EXPECT_EQ(10, list.back().version()); + } + + // Drop the remaining invalidation. Verify an unknown version is all that + // remains. + DropNthInvalidation(id, 0); + { + SingleObjectInvalidationSet list = GetSavedInvalidationsForType(id); + ASSERT_EQ(1U, list.GetSize()); + EXPECT_TRUE(list.begin()->is_unknown_version()); + } + + // Announce that the delegate has recovered from the drop. Verify no + // invalidations remain saved. + RecoverFromDropEvent(id); + EXPECT_TRUE(GetSavedInvalidationsForType(id).IsEmpty()); + + RecoverFromDropEvent(id); } // Without readying the client, disable notifications, then enable @@ -1012,6 +1068,60 @@ TEST_F(SyncInvalidationListenerTest, InvalidationClientAuthError) { EXPECT_EQ(INVALIDATIONS_ENABLED, GetInvalidatorState()); } +// A variant of SyncInvalidationListenerTest that starts with some initial +// state. We make not attempt to abstract away the contents of this state. The +// tests that make use of this harness depend on its implementation details. +class SyncInvalidationListenerTest_WithInitialState + : public SyncInvalidationListenerTest { + public: + virtual void SetUp() { + UnackedInvalidationSet bm_state(kBookmarksId_); + UnackedInvalidationSet ext_state(kExtensionsId_); + + Invalidation bm_unknown = Invalidation::InitUnknownVersion(kBookmarksId_); + Invalidation bm_v100 = Invalidation::Init(kBookmarksId_, 100, "hundred"); + bm_state.Add(bm_unknown); + bm_state.Add(bm_v100); + + Invalidation ext_v10 = Invalidation::Init(kExtensionsId_, 10, "ten"); + Invalidation ext_v20 = Invalidation::Init(kExtensionsId_, 20, "twenty"); + ext_state.Add(ext_v10); + ext_state.Add(ext_v20); + + initial_state.insert(std::make_pair(kBookmarksId_, bm_state)); + initial_state.insert(std::make_pair(kExtensionsId_, ext_state)); + + fake_tracker_.SetSavedInvalidations(initial_state); + + SyncInvalidationListenerTest::SetUp(); + } + + UnackedInvalidationsMap initial_state; +}; + +// Verify that saved invalidations are forwarded when handlers register. +TEST_F(SyncInvalidationListenerTest_WithInitialState, + ReceiveSavedInvalidations) { + EnableNotifications(); + listener_.Ready(fake_invalidation_client_); + + EXPECT_THAT(initial_state, test_util::Eq(GetSavedInvalidations())); + + ASSERT_EQ(2U, GetInvalidationCount(kBookmarksId_)); + EXPECT_EQ(100, GetVersion(kBookmarksId_)); + + ASSERT_EQ(0U, GetInvalidationCount(kExtensionsId_)); + + FireInvalidate(kExtensionsId_, 30, "thirty"); + + ObjectIdSet ids = GetRegisteredIds(); + ids.insert(kExtensionsId_); + listener_.UpdateRegisteredIds(ids); + + ASSERT_EQ(3U, GetInvalidationCount(kExtensionsId_)); + EXPECT_EQ(30, GetVersion(kExtensionsId_)); +} + } // namespace } // namespace syncer diff --git a/sync/notifier/unacked_invalidation_set.h b/sync/notifier/unacked_invalidation_set.h index 2824e6a..aae9cda 100644 --- a/sync/notifier/unacked_invalidation_set.h +++ b/sync/notifier/unacked_invalidation_set.h @@ -18,6 +18,10 @@ class DictionaryValue; namespace syncer { +namespace test_util { +class UnackedInvalidationSetEqMatcher; +} // test_util + class SingleObjectInvalidationSet; class ObjectIdInvalidationMap; class AckHandle; @@ -88,7 +92,7 @@ class SYNC_EXPORT UnackedInvalidationSet { private: // Allow this test helper to have access to our internals. - friend class UnackedInvalidationSetEqMatcher; + friend class test_util::UnackedInvalidationSetEqMatcher; typedef std::set<Invalidation, InvalidationVersionLessThan> InvalidationsSet; diff --git a/sync/notifier/unacked_invalidation_set_test_util.cc b/sync/notifier/unacked_invalidation_set_test_util.cc new file mode 100644 index 0000000..8961574c --- /dev/null +++ b/sync/notifier/unacked_invalidation_set_test_util.cc @@ -0,0 +1,181 @@ +// Copyright 2013 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/notifier/unacked_invalidation_set_test_util.h" + +#include "base/json/json_string_value_serializer.h" +#include "sync/notifier/object_id_invalidation_map.h" +#include "testing/gmock/include/gmock/gmock-matchers.h" + +namespace syncer { + +using ::testing::MakeMatcher; +using ::testing::MatchResultListener; +using ::testing::Matcher; +using ::testing::MatcherInterface; +using ::testing::PrintToString; + +namespace test_util { + +// This class needs to be declared outside the null namespace so the +// UnackedInvalidationSet can declare it as a friend. This class needs access +// to the UnackedInvalidationSet internals to implement its comparispon +// function. +class UnackedInvalidationSetEqMatcher + : public testing::MatcherInterface<const UnackedInvalidationSet&> { + public: + explicit UnackedInvalidationSetEqMatcher( + const UnackedInvalidationSet& expected); + + virtual bool MatchAndExplain( + const UnackedInvalidationSet& actual, + MatchResultListener* listener) const OVERRIDE; + virtual void DescribeTo(::std::ostream* os) const OVERRIDE; + virtual void DescribeNegationTo(::std::ostream* os) const OVERRIDE; + + private: + const UnackedInvalidationSet expected_; + + DISALLOW_COPY_AND_ASSIGN(UnackedInvalidationSetEqMatcher); +}; + +namespace { + +struct InvalidationEq { + bool operator()(const syncer::Invalidation& a, + const syncer::Invalidation& b) const { + return a.Equals(b); + } +}; + +} // namespace + +UnackedInvalidationSetEqMatcher::UnackedInvalidationSetEqMatcher( + const UnackedInvalidationSet& expected) + : expected_(expected) {} + +bool UnackedInvalidationSetEqMatcher::MatchAndExplain( + const UnackedInvalidationSet& actual, + MatchResultListener* listener) const { + // Use our friendship with this class to compare the internals of two + // instances. + // + // Note that the registration status is intentionally not considered + // when performing this comparison. + return expected_.object_id_ == actual.object_id_ + && std::equal(expected_.invalidations_.begin(), + expected_.invalidations_.end(), + actual.invalidations_.begin(), + InvalidationEq()); +} + +void UnackedInvalidationSetEqMatcher::DescribeTo(::std::ostream* os) const { + *os << " is equal to " << PrintToString(expected_); +} + +void UnackedInvalidationSetEqMatcher::DescribeNegationTo( + ::std::ostream* os) const { + *os << " isn't equal to " << PrintToString(expected_); +} + +// We're done declaring UnackedInvalidationSetEqMatcher. Everything else can +// go into the null namespace. +namespace { + +ObjectIdInvalidationMap UnackedInvalidationsMapToObjectIdInvalidationMap( + const UnackedInvalidationsMap& state_map) { + ObjectIdInvalidationMap object_id_invalidation_map; + for (UnackedInvalidationsMap::const_iterator it = state_map.begin(); + it != state_map.end(); ++it) { + it->second.ExportInvalidations(syncer::WeakHandle<AckHandler>(), + &object_id_invalidation_map); + } + return object_id_invalidation_map; +} + +class UnackedInvalidationsMapEqMatcher + : public testing::MatcherInterface<const UnackedInvalidationsMap&> { + public: + explicit UnackedInvalidationsMapEqMatcher( + const UnackedInvalidationsMap& expected); + + virtual bool MatchAndExplain(const UnackedInvalidationsMap& actual, + MatchResultListener* listener) const; + virtual void DescribeTo(::std::ostream* os) const; + virtual void DescribeNegationTo(::std::ostream* os) const; + + private: + const UnackedInvalidationsMap expected_; + + DISALLOW_COPY_AND_ASSIGN(UnackedInvalidationsMapEqMatcher); +}; + +UnackedInvalidationsMapEqMatcher::UnackedInvalidationsMapEqMatcher( + const UnackedInvalidationsMap& expected) + : expected_(expected) { +} + +bool UnackedInvalidationsMapEqMatcher::MatchAndExplain( + const UnackedInvalidationsMap& actual, + MatchResultListener* listener) const { + ObjectIdInvalidationMap expected_inv = + UnackedInvalidationsMapToObjectIdInvalidationMap(expected_); + ObjectIdInvalidationMap actual_inv = + UnackedInvalidationsMapToObjectIdInvalidationMap(actual); + + return expected_inv == actual_inv; +} + +void UnackedInvalidationsMapEqMatcher::DescribeTo( + ::std::ostream* os) const { + *os << " is equal to " << PrintToString(expected_); +} + +void UnackedInvalidationsMapEqMatcher::DescribeNegationTo( + ::std::ostream* os) const { + *os << " isn't equal to " << PrintToString(expected_); +} + +} // namespace + +void PrintTo(const UnackedInvalidationSet& invalidations, + ::std::ostream* os) { + scoped_ptr<base::DictionaryValue> value = invalidations.ToValue(); + + std::string output; + JSONStringValueSerializer serializer(&output); + serializer.set_pretty_print(true); + serializer.Serialize(*value.get()); + + (*os) << output; +} + +void PrintTo(const UnackedInvalidationsMap& map, ::std::ostream* os) { + scoped_ptr<base::ListValue> list(new base::ListValue); + for (UnackedInvalidationsMap::const_iterator it = map.begin(); + it != map.end(); ++it) { + list->Append(it->second.ToValue().release()); + } + + std::string output; + JSONStringValueSerializer serializer(&output); + serializer.set_pretty_print(true); + serializer.Serialize(*list.get()); + + (*os) << output; +} + +Matcher<const UnackedInvalidationSet&> Eq( + const UnackedInvalidationSet& expected) { + return MakeMatcher(new UnackedInvalidationSetEqMatcher(expected)); +} + +Matcher<const UnackedInvalidationsMap&> Eq( + const UnackedInvalidationsMap& expected) { + return MakeMatcher(new UnackedInvalidationsMapEqMatcher(expected)); +} + +} // namespace test_util + +}; diff --git a/sync/notifier/unacked_invalidation_set_test_util.h b/sync/notifier/unacked_invalidation_set_test_util.h new file mode 100644 index 0000000..e93726b --- /dev/null +++ b/sync/notifier/unacked_invalidation_set_test_util.h @@ -0,0 +1,25 @@ +// Copyright 2013 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/notifier/unacked_invalidation_set.h" + +#include "testing/gmock/include/gmock/gmock-matchers.h" + +namespace syncer { + +namespace test_util { + +void PrintTo(const UnackedInvalidationSet& invalidations, ::std::ostream* os); + +void PrintTo(const UnackedInvalidationsMap& map, ::std::ostream* os); + +::testing::Matcher<const UnackedInvalidationSet&> Eq( + const UnackedInvalidationSet& expected); + +::testing::Matcher<const UnackedInvalidationsMap&> Eq( + const UnackedInvalidationsMap& expected); + +} // namespace test_util + +} // namespace syncer diff --git a/sync/notifier/unacked_invalidation_set_unittest.cc b/sync/notifier/unacked_invalidation_set_unittest.cc index 70cd232..d6549ab 100644 --- a/sync/notifier/unacked_invalidation_set_unittest.cc +++ b/sync/notifier/unacked_invalidation_set_unittest.cc @@ -7,183 +7,11 @@ #include "base/json/json_string_value_serializer.h" #include "sync/notifier/object_id_invalidation_map.h" #include "sync/notifier/single_object_invalidation_set.h" -#include "testing/gmock/include/gmock/gmock-matchers.h" +#include "sync/notifier/unacked_invalidation_set_test_util.h" #include "testing/gtest/include/gtest/gtest.h" namespace syncer { -// Start with some helper functions and classes. - -using ::testing::MakeMatcher; -using ::testing::MatchResultListener; -using ::testing::Matcher; -using ::testing::MatcherInterface; -using ::testing::PrintToString; - -void PrintTo( - const UnackedInvalidationSet& invalidations, ::std::ostream* os); - -void PrintTo( - const UnackedInvalidationsMap& map, ::std::ostream* os); - -::testing::Matcher<const UnackedInvalidationSet&> Eq( - const UnackedInvalidationSet& expected); - -::testing::Matcher<const UnackedInvalidationsMap&> Eq( - const UnackedInvalidationsMap& expected); - -class UnackedInvalidationSetEqMatcher - : public testing::MatcherInterface<const UnackedInvalidationSet&> { - public: - explicit UnackedInvalidationSetEqMatcher( - const UnackedInvalidationSet& expected); - - virtual bool MatchAndExplain( - const UnackedInvalidationSet& actual, - MatchResultListener* listener) const OVERRIDE; - virtual void DescribeTo(::std::ostream* os) const OVERRIDE; - virtual void DescribeNegationTo(::std::ostream* os) const OVERRIDE; - - private: - const UnackedInvalidationSet expected_; - - DISALLOW_COPY_AND_ASSIGN(UnackedInvalidationSetEqMatcher); -}; - -UnackedInvalidationSetEqMatcher::UnackedInvalidationSetEqMatcher( - const UnackedInvalidationSet& expected) - : expected_(expected) {} - -namespace { - -struct InvalidationEq { - bool operator()(const syncer::Invalidation& a, - const syncer::Invalidation& b) const { - return a.Equals(b); - } -}; - -} // namespace - -bool UnackedInvalidationSetEqMatcher::MatchAndExplain( - const UnackedInvalidationSet& actual, - MatchResultListener* listener) const { - // Use our friendship with this class to compare the internals of two - // instances. - // - // Note that the registration status is intentionally not considered - // when performing this comparison. - return expected_.object_id_ == actual.object_id_ - && std::equal(expected_.invalidations_.begin(), - expected_.invalidations_.end(), - actual.invalidations_.begin(), - InvalidationEq()); -} - -void UnackedInvalidationSetEqMatcher::DescribeTo(::std::ostream* os) const { - *os << " is equal to " << PrintToString(expected_); -} - -void UnackedInvalidationSetEqMatcher::DescribeNegationTo( - ::std::ostream* os) const { - *os << " isn't equal to " << PrintToString(expected_); -} - -namespace { - -ObjectIdInvalidationMap UnackedInvalidationsMapToObjectIdInvalidationMap( - const UnackedInvalidationsMap& state_map) { - ObjectIdInvalidationMap object_id_invalidation_map; - for (UnackedInvalidationsMap::const_iterator it = state_map.begin(); - it != state_map.end(); ++it) { - it->second.ExportInvalidations(syncer::WeakHandle<AckHandler>(), - &object_id_invalidation_map); - } - return object_id_invalidation_map; -} - -class UnackedInvalidationsMapEqMatcher - : public testing::MatcherInterface<const UnackedInvalidationsMap&> { - public: - explicit UnackedInvalidationsMapEqMatcher( - const UnackedInvalidationsMap& expected); - - virtual bool MatchAndExplain(const UnackedInvalidationsMap& actual, - MatchResultListener* listener) const; - virtual void DescribeTo(::std::ostream* os) const; - virtual void DescribeNegationTo(::std::ostream* os) const; - - private: - const UnackedInvalidationsMap expected_; - - DISALLOW_COPY_AND_ASSIGN(UnackedInvalidationsMapEqMatcher); -}; - -UnackedInvalidationsMapEqMatcher::UnackedInvalidationsMapEqMatcher( - const UnackedInvalidationsMap& expected) - : expected_(expected) { -} - -bool UnackedInvalidationsMapEqMatcher::MatchAndExplain( - const UnackedInvalidationsMap& actual, - MatchResultListener* listener) const { - ObjectIdInvalidationMap expected_inv = - UnackedInvalidationsMapToObjectIdInvalidationMap(expected_); - ObjectIdInvalidationMap actual_inv = - UnackedInvalidationsMapToObjectIdInvalidationMap(actual); - - return expected_inv == actual_inv; -} - -void UnackedInvalidationsMapEqMatcher::DescribeTo( - ::std::ostream* os) const { - *os << " is equal to " << PrintToString(expected_); -} - -void UnackedInvalidationsMapEqMatcher::DescribeNegationTo( - ::std::ostream* os) const { - *os << " isn't equal to " << PrintToString(expected_); -} - -} // namespace - -void PrintTo(const UnackedInvalidationSet& invalidations, - ::std::ostream* os) { - scoped_ptr<base::DictionaryValue> value = invalidations.ToValue(); - - std::string output; - JSONStringValueSerializer serializer(&output); - serializer.set_pretty_print(true); - serializer.Serialize(*value.get()); - - (*os) << output; -} - -void PrintTo(const UnackedInvalidationsMap& map, ::std::ostream* os) { - scoped_ptr<base::ListValue> list(new base::ListValue); - for (UnackedInvalidationsMap::const_iterator it = map.begin(); - it != map.end(); ++it) { - list->Append(it->second.ToValue().release()); - } - - std::string output; - JSONStringValueSerializer serializer(&output); - serializer.set_pretty_print(true); - serializer.Serialize(*list.get()); - - (*os) << output; -} - -Matcher<const UnackedInvalidationSet&> Eq( - const UnackedInvalidationSet& expected) { - return MakeMatcher(new UnackedInvalidationSetEqMatcher(expected)); -} - -Matcher<const UnackedInvalidationsMap&> Eq( - const UnackedInvalidationsMap& expected) { - return MakeMatcher(new UnackedInvalidationsMapEqMatcher(expected)); -} - class UnackedInvalidationSetTest : public testing::Test { public: UnackedInvalidationSetTest() @@ -363,7 +191,7 @@ class UnackedInvalidationSetSerializationTest TEST_F(UnackedInvalidationSetSerializationTest, Empty) { UnackedInvalidationSet deserialized = SerializeDeserialize(); - EXPECT_THAT(unacked_invalidations_, Eq(deserialized)); + EXPECT_THAT(unacked_invalidations_, test_util::Eq(deserialized)); } TEST_F(UnackedInvalidationSetSerializationTest, OneInvalidation) { @@ -371,7 +199,7 @@ TEST_F(UnackedInvalidationSetSerializationTest, OneInvalidation) { unacked_invalidations_.Add(inv); UnackedInvalidationSet deserialized = SerializeDeserialize(); - EXPECT_THAT(unacked_invalidations_, Eq(deserialized)); + EXPECT_THAT(unacked_invalidations_, test_util::Eq(deserialized)); } TEST_F(UnackedInvalidationSetSerializationTest, WithUnknownVersion) { @@ -383,7 +211,7 @@ TEST_F(UnackedInvalidationSetSerializationTest, WithUnknownVersion) { unacked_invalidations_.Add(inv3); UnackedInvalidationSet deserialized = SerializeDeserialize(); - EXPECT_THAT(unacked_invalidations_, Eq(deserialized)); + EXPECT_THAT(unacked_invalidations_, test_util::Eq(deserialized)); } } // namespace diff --git a/sync/sessions/nudge_tracker.h b/sync/sessions/nudge_tracker.h index aa4414c..fcd0150 100644 --- a/sync/sessions/nudge_tracker.h +++ b/sync/sessions/nudge_tracker.h @@ -13,11 +13,13 @@ #include "base/compiler_specific.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/model_type.h" -#include "sync/notifier/object_id_invalidation_map.h" #include "sync/protocol/sync.pb.h" #include "sync/sessions/data_type_tracker.h" namespace syncer { + +class ObjectIdInvalidationMap; + namespace sessions { class SYNC_EXPORT_PRIVATE NudgeTracker { diff --git a/sync/sessions/nudge_tracker_unittest.cc b/sync/sessions/nudge_tracker_unittest.cc index c76f3ec..450d17f 100644 --- a/sync/sessions/nudge_tracker_unittest.cc +++ b/sync/sessions/nudge_tracker_unittest.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include "sync/internal_api/public/base/model_type_test_util.h" +#include "sync/notifier/invalidation_util.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "sync/sessions/nudge_tracker.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/sync/sync_internal_api.gypi b/sync/sync_internal_api.gypi index f9aeeca..d750176 100644 --- a/sync/sync_internal_api.gypi +++ b/sync/sync_internal_api.gypi @@ -32,6 +32,7 @@ 'internal_api/js_sync_encryption_handler_observer.h', 'internal_api/js_sync_manager_observer.cc', 'internal_api/js_sync_manager_observer.h', + 'internal_api/public/base/enum_set.h', 'internal_api/public/base/ack_handle.cc', 'internal_api/public/base/ack_handle.h', 'internal_api/public/base/cancelation_observer.cc', diff --git a/sync/sync_notifier.gypi b/sync/sync_notifier.gypi index f745234..7986bd7 100644 --- a/sync/sync_notifier.gypi +++ b/sync/sync_notifier.gypi @@ -28,7 +28,6 @@ 'notifier/dropped_invalidation_tracker.cc', 'notifier/dropped_invalidation_tracker.h', 'notifier/invalidation_handler.h', - 'notifier/invalidation_state_tracker.cc', 'notifier/invalidation_state_tracker.h', 'notifier/invalidation_util.cc', 'notifier/invalidation_util.h', @@ -39,6 +38,8 @@ 'notifier/invalidator_registrar.h', 'notifier/invalidator_state.cc', 'notifier/invalidator_state.h', + 'notifier/mock_ack_handler.cc', + 'notifier/mock_ack_handler.h', 'notifier/object_id_invalidation_map.cc', 'notifier/object_id_invalidation_map.h', 'notifier/single_object_invalidation_set.cc', @@ -47,8 +48,6 @@ 'conditions': [ ['OS != "android"', { 'sources': [ - 'notifier/ack_tracker.cc', - 'notifier/ack_tracker.h', 'notifier/invalidation_notifier.cc', 'notifier/invalidation_notifier.h', 'notifier/non_blocking_invalidator.cc', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index 1b5871a85..010cb5b 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -121,8 +121,10 @@ 'notifier/fake_invalidator.h', 'notifier/invalidator_test_template.cc', 'notifier/invalidator_test_template.h', - 'notifier/mock_ack_handler.cc', - 'notifier/mock_ack_handler.h', + 'notifier/unacked_invalidation_set_test_util.cc', + 'notifier/unacked_invalidation_set_test_util.h', + 'internal_api/public/base/object_id_invalidation_map_test_util.h', + 'internal_api/public/base/object_id_invalidation_map_test_util.cc', ], }, @@ -151,8 +153,6 @@ 'sources': [ 'internal_api/public/base/invalidation_test_util.cc', 'internal_api/public/base/invalidation_test_util.h', - 'internal_api/public/base/object_id_invalidation_map_test_util.cc', - 'internal_api/public/base/object_id_invalidation_map_test_util.h', 'internal_api/public/test/fake_sync_manager.h', 'internal_api/public/test/sync_manager_factory_for_profile_sync_test.h', 'internal_api/public/test/test_entry_factory.h', @@ -318,7 +318,6 @@ 'conditions': [ ['OS != "android"', { 'sources': [ - 'notifier/ack_tracker_unittest.cc', 'notifier/fake_invalidator_unittest.cc', 'notifier/invalidation_notifier_unittest.cc', 'notifier/invalidator_registrar_unittest.cc', diff --git a/sync/test/engine/mock_connection_manager.cc b/sync/test/engine/mock_connection_manager.cc index 434f6c3..ef3022e 100644 --- a/sync/test/engine/mock_connection_manager.cc +++ b/sync/test/engine/mock_connection_manager.cc @@ -497,18 +497,6 @@ void MockConnectionManager::ProcessGetUpdates( gu.caller_info().source()); } - // Verify that the GetUpdates filter sent by the Syncer matches the test - // expectation. - ModelTypeSet protocol_types = ProtocolTypes(); - for (ModelTypeSet::Iterator iter = protocol_types.First(); iter.Good(); - iter.Inc()) { - ModelType model_type = iter.Get(); - sync_pb::DataTypeProgressMarker const* progress_marker = - GetProgressMarkerForType(gu.from_progress_marker(), model_type); - EXPECT_EQ(expected_filter_.Has(model_type), (progress_marker != NULL)) - << "Syncer requested_types differs from test expectation."; - } - // Verify that the items we're about to send back to the client are of // the types requested by the client. If this fails, it probably indicates // a test bug. diff --git a/sync/tools/null_invalidation_state_tracker.cc b/sync/tools/null_invalidation_state_tracker.cc index 192060b..ff21a6f 100644 --- a/sync/tools/null_invalidation_state_tracker.cc +++ b/sync/tools/null_invalidation_state_tracker.cc @@ -17,26 +17,6 @@ namespace syncer { NullInvalidationStateTracker::NullInvalidationStateTracker() {} NullInvalidationStateTracker::~NullInvalidationStateTracker() {} -InvalidationStateMap -NullInvalidationStateTracker::GetAllInvalidationStates() const { - return InvalidationStateMap(); -} - -void NullInvalidationStateTracker::SetMaxVersionAndPayload( - const invalidation::ObjectId& id, - int64 max_invalidation_version, - const std::string& payload) { - LOG(INFO) << "Setting max invalidation version for " - << ObjectIdToString(id) << " to " << max_invalidation_version - << " with payload " << payload; -} - -void NullInvalidationStateTracker::Forget(const ObjectIdSet& ids) { - for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { - LOG(INFO) << "Forgetting invalidation state for " << ObjectIdToString(*it); - } -} - void NullInvalidationStateTracker::SetInvalidatorClientId( const std::string& data) { LOG(INFO) << "Setting invalidator client ID to: " << data; @@ -66,20 +46,14 @@ void NullInvalidationStateTracker::Clear() { // We have no members to clear. } -void NullInvalidationStateTracker::GenerateAckHandles( - const ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const AckHandleMap&)> callback) { - AckHandleMap ack_handles; - for (ObjectIdSet::const_iterator it = ids.begin(); it != ids.end(); ++it) { - ack_handles.insert(std::make_pair(*it, AckHandle::InvalidAckHandle())); - } - CHECK(task_runner->PostTask(FROM_HERE, base::Bind(callback, ack_handles))); +void NullInvalidationStateTracker::SetSavedInvalidations( + const UnackedInvalidationsMap& states) { + // Do nothing. } -void NullInvalidationStateTracker::Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) { - LOG(INFO) << "Received ack for " << ObjectIdToString(id); +UnackedInvalidationsMap +NullInvalidationStateTracker::GetSavedInvalidations() const { + return UnackedInvalidationsMap(); } } // namespace syncer diff --git a/sync/tools/null_invalidation_state_tracker.h b/sync/tools/null_invalidation_state_tracker.h index ce05c33..a12844c 100644 --- a/sync/tools/null_invalidation_state_tracker.h +++ b/sync/tools/null_invalidation_state_tracker.h @@ -18,26 +18,17 @@ class NullInvalidationStateTracker NullInvalidationStateTracker(); virtual ~NullInvalidationStateTracker(); - virtual InvalidationStateMap GetAllInvalidationStates() const OVERRIDE; - virtual void SetMaxVersionAndPayload(const invalidation::ObjectId& id, - int64 max_invalidation_version, - const std::string& payload) OVERRIDE; - virtual void Forget(const ObjectIdSet& ids) OVERRIDE; - virtual void SetInvalidatorClientId(const std::string& data) OVERRIDE; virtual std::string GetInvalidatorClientId() const OVERRIDE; virtual std::string GetBootstrapData() const OVERRIDE; virtual void SetBootstrapData(const std::string& data) OVERRIDE; - virtual void Clear() OVERRIDE; + virtual void SetSavedInvalidations( + const UnackedInvalidationsMap& states) OVERRIDE; + virtual UnackedInvalidationsMap GetSavedInvalidations() const OVERRIDE; - virtual void GenerateAckHandles( - const ObjectIdSet& ids, - const scoped_refptr<base::TaskRunner>& task_runner, - base::Callback<void(const AckHandleMap&)> callback) OVERRIDE; - virtual void Acknowledge(const invalidation::ObjectId& id, - const AckHandle& ack_handle) OVERRIDE; + virtual void Clear() OVERRIDE; }; } // namespace syncer diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc index df2d02b..e505104 100644 --- a/sync/tools/sync_client.cc +++ b/sync/tools/sync_client.cc @@ -275,7 +275,7 @@ int SyncClientMain(int argc, char* argv[]) { scoped_ptr<Invalidator> invalidator(new NonBlockingInvalidator( notifier_options, invalidator_id, - null_invalidation_state_tracker.GetAllInvalidationStates(), + null_invalidation_state_tracker.GetSavedInvalidations(), null_invalidation_state_tracker.GetBootstrapData(), WeakHandle<InvalidationStateTracker>( null_invalidation_state_tracker.AsWeakPtr()), diff --git a/sync/tools/sync_listen_notifications.cc b/sync/tools/sync_listen_notifications.cc index f9d1989..5d212f3 100644 --- a/sync/tools/sync_listen_notifications.cc +++ b/sync/tools/sync_listen_notifications.cc @@ -28,6 +28,7 @@ #include "sync/notifier/invalidation_util.h" #include "sync/notifier/invalidator.h" #include "sync/notifier/non_blocking_invalidator.h" +#include "sync/notifier/object_id_invalidation_map.h" #include "sync/tools/null_invalidation_state_tracker.h" #if defined(OS_MACOSX) @@ -182,7 +183,7 @@ int SyncListenNotificationsMain(int argc, char* argv[]) { new NonBlockingInvalidator( notifier_options, base::RandBytesAsString(8), - null_invalidation_state_tracker.GetAllInvalidationStates(), + null_invalidation_state_tracker.GetSavedInvalidations(), null_invalidation_state_tracker.GetBootstrapData(), WeakHandle<InvalidationStateTracker>( null_invalidation_state_tracker.AsWeakPtr()), |