diff options
Diffstat (limited to 'sync')
| -rw-r--r-- | sync/engine/download.cc | 175 | ||||
| -rw-r--r-- | sync/engine/download.h | 60 | ||||
| -rw-r--r-- | sync/engine/get_updates_processor.cc | 167 | ||||
| -rw-r--r-- | sync/engine/get_updates_processor.h | 55 | ||||
| -rw-r--r-- | sync/engine/get_updates_processor_unittest.cc (renamed from sync/engine/download_unittest.cc) | 85 | ||||
| -rw-r--r-- | sync/engine/syncer.cc | 19 | ||||
| -rw-r--r-- | sync/sync_core.gypi | 2 | ||||
| -rw-r--r-- | sync/sync_tests.gypi | 2 |
8 files changed, 265 insertions, 300 deletions
diff --git a/sync/engine/download.cc b/sync/engine/download.cc deleted file mode 100644 index def78dd..0000000 --- a/sync/engine/download.cc +++ /dev/null @@ -1,175 +0,0 @@ -// 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/engine/download.h" - -#include <string> - -#include "sync/engine/syncer_proto_util.h" -#include "sync/sessions/sync_session.h" -#include "sync/sessions/sync_session_context.h" -#include "sync/syncable/directory.h" -#include "sync/syncable/nigori_handler.h" -#include "sync/syncable/syncable_read_transaction.h" - -namespace syncer { - -using sessions::StatusController; -using sessions::SyncSession; -using sessions::SyncSessionContext; -using std::string; - -namespace { - -SyncerError HandleGetEncryptionKeyResponse( - const sync_pb::ClientToServerResponse& update_response, - syncable::Directory* dir) { - bool success = false; - if (update_response.get_updates().encryption_keys_size() == 0) { - LOG(ERROR) << "Failed to receive encryption key from server."; - return SERVER_RESPONSE_VALIDATION_FAILED; - } - syncable::ReadTransaction trans(FROM_HERE, dir); - syncable::NigoriHandler* nigori_handler = dir->GetNigoriHandler(); - success = nigori_handler->SetKeystoreKeys( - update_response.get_updates().encryption_keys(), - &trans); - - DVLOG(1) << "GetUpdates returned " - << update_response.get_updates().encryption_keys_size() - << "encryption keys. Nigori keystore key " - << (success ? "" : "not ") << "updated."; - return (success ? SYNCER_OK : SERVER_RESPONSE_VALIDATION_FAILED); -} - -bool ShouldRequestEncryptionKey( - SyncSessionContext* context) { - bool need_encryption_key = false; - if (context->keystore_encryption_enabled()) { - syncable::Directory* dir = context->directory(); - syncable::ReadTransaction trans(FROM_HERE, dir); - syncable::NigoriHandler* nigori_handler = dir->GetNigoriHandler(); - need_encryption_key = nigori_handler->NeedKeystoreKey(&trans); - } - return need_encryption_key; -} - -} // namespace - -namespace download { - -void InitDownloadUpdatesContext( - SyncSession* session, - bool create_mobile_bookmarks_folder, - sync_pb::ClientToServerMessage* message) { - message->set_share(session->context()->account_name()); - message->set_message_contents(sync_pb::ClientToServerMessage::GET_UPDATES); - - sync_pb::GetUpdatesMessage* get_updates = message->mutable_get_updates(); - - // We want folders for our associated types, always. If we were to set - // this to false, the server would send just the non-container items - // (e.g. Bookmark URLs but not their containing folders). - get_updates->set_fetch_folders(true); - - get_updates->set_create_mobile_bookmarks_folder( - create_mobile_bookmarks_folder); - bool need_encryption_key = ShouldRequestEncryptionKey(session->context()); - get_updates->set_need_encryption_key(need_encryption_key); - - // Set legacy GetUpdatesMessage.GetUpdatesCallerInfo information. - get_updates->mutable_caller_info()->set_notifications_enabled( - session->context()->notifications_enabled()); -} - -SyncerError ExecuteDownloadUpdates( - ModelTypeSet request_types, - SyncSession* session, - GetUpdatesProcessor* get_updates_processor, - sync_pb::ClientToServerMessage* msg) { - sync_pb::ClientToServerResponse update_response; - StatusController* status = session->mutable_status_controller(); - bool need_encryption_key = ShouldRequestEncryptionKey(session->context()); - - if (session->context()->debug_info_getter()) { - sync_pb::DebugInfo* debug_info = msg->mutable_debug_info(); - CopyClientDebugInfo(session->context()->debug_info_getter(), debug_info); - } - - SyncerError result = SyncerProtoUtil::PostClientToServerMessage( - msg, - &update_response, - session); - - DVLOG(2) << SyncerProtoUtil::ClientToServerResponseDebugString( - update_response); - - if (result != SYNCER_OK) { - LOG(ERROR) << "PostClientToServerMessage() failed during GetUpdates"; - return result; - } - - DVLOG(1) << "GetUpdates " - << " returned " << update_response.get_updates().entries_size() - << " updates and indicated " - << update_response.get_updates().changes_remaining() - << " updates left on server."; - - if (session->context()->debug_info_getter()) { - // Clear debug info now that we have successfully sent it to the server. - DVLOG(1) << "Clearing client debug info."; - session->context()->debug_info_getter()->ClearDebugInfo(); - } - - if (need_encryption_key || - update_response.get_updates().encryption_keys_size() > 0) { - syncable::Directory* dir = session->context()->directory(); - status->set_last_get_key_result( - HandleGetEncryptionKeyResponse(update_response, dir)); - } - - return ProcessResponse(update_response.get_updates(), - request_types, - get_updates_processor, - status); -} - -SyncerError ProcessResponse( - const sync_pb::GetUpdatesResponse& gu_response, - ModelTypeSet request_types, - GetUpdatesProcessor* get_updates_processor, - StatusController* status) { - status->increment_num_updates_downloaded_by(gu_response.entries_size()); - - // The changes remaining field is used to prevent the client from looping. If - // that field is being set incorrectly, we're in big trouble. - if (!gu_response.has_changes_remaining()) { - return SERVER_RESPONSE_VALIDATION_FAILED; - } - status->set_num_server_changes_remaining(gu_response.changes_remaining()); - - - if (!get_updates_processor->ProcessGetUpdatesResponse(request_types, - gu_response, - status)) { - return SERVER_RESPONSE_VALIDATION_FAILED; - } - - if (gu_response.changes_remaining() == 0) { - return SYNCER_OK; - } else { - return SERVER_MORE_TO_DOWNLOAD; - } -} - -void CopyClientDebugInfo( - sessions::DebugInfoGetter* debug_info_getter, - sync_pb::DebugInfo* debug_info) { - DVLOG(1) << "Copying client debug info to send."; - debug_info_getter->GetDebugInfo(debug_info); -} - -} // namespace download - -} // namespace syncer diff --git a/sync/engine/download.h b/sync/engine/download.h deleted file mode 100644 index 0eeca97..0000000 --- a/sync/engine/download.h +++ /dev/null @@ -1,60 +0,0 @@ -// 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. - -#ifndef SYNC_ENGINE_DOWNLOAD_H_ -#define SYNC_ENGINE_DOWNLOAD_H_ - -#include "sync/base/sync_export.h" -#include "sync/engine/get_updates_processor.h" -#include "sync/internal_api/public/base/model_type.h" -#include "sync/internal_api/public/util/syncer_error.h" -#include "sync/protocol/sync.pb.h" - -namespace sync_pb { -class DebugInfo; -} // namespace sync_pb - -namespace syncer { - -namespace sessions { -class DebugInfoGetter; -class StatusController; -class SyncSession; -} // namespace sessions - -namespace download { - -// Generic initialization of a GetUpdates message. -SYNC_EXPORT_PRIVATE void InitDownloadUpdatesContext( - sessions::SyncSession* session, - bool create_mobile_bookmarks_folder, - sync_pb::ClientToServerMessage* message); - -// Sends the specified message to the server and stores the response in a member -// of the |session|'s StatusController. -SYNC_EXPORT_PRIVATE SyncerError - ExecuteDownloadUpdates(ModelTypeSet request_types, - sessions::SyncSession* session, - GetUpdatesProcessor* get_updates_processor, - sync_pb::ClientToServerMessage* msg); - -// Helper function for processing responses from the server. -// Defined here for testing. -SYNC_EXPORT_PRIVATE SyncerError ProcessResponse( - const sync_pb::GetUpdatesResponse& gu_response, - ModelTypeSet proto_request_types, - GetUpdatesProcessor* get_updates_processor, - sessions::StatusController* status); - -// Helper function to copy client debug info from debug_info_getter to -// debug_info. Defined here for testing. -SYNC_EXPORT_PRIVATE void CopyClientDebugInfo( - sessions::DebugInfoGetter* debug_info_getter, - sync_pb::DebugInfo* debug_info); - -} // namespace download - -} // namespace syncer - -#endif // SYNC_ENGINE_DOWNLOAD_H_ diff --git a/sync/engine/get_updates_processor.cc b/sync/engine/get_updates_processor.cc index 449f718..2ffc4b3 100644 --- a/sync/engine/get_updates_processor.cc +++ b/sync/engine/get_updates_processor.cc @@ -6,9 +6,16 @@ #include <map> +#include "base/debug/trace_event.h" #include "sync/engine/get_updates_delegate.h" +#include "sync/engine/syncer_proto_util.h" #include "sync/engine/update_handler.h" #include "sync/protocol/sync.pb.h" +#include "sync/sessions/status_controller.h" +#include "sync/sessions/sync_session.h" +#include "sync/syncable/directory.h" +#include "sync/syncable/nigori_handler.h" +#include "sync/syncable/syncable_read_transaction.h" typedef std::vector<const sync_pb::SyncEntity*> SyncEntityList; typedef std::map<syncer::ModelType, SyncEntityList> TypeSyncEntityMap; @@ -19,6 +26,35 @@ typedef std::map<ModelType, size_t> TypeToIndexMap; namespace { +bool ShouldRequestEncryptionKey(sessions::SyncSessionContext* context) { + syncable::Directory* dir = context->directory(); + syncable::ReadTransaction trans(FROM_HERE, dir); + syncable::NigoriHandler* nigori_handler = dir->GetNigoriHandler(); + return nigori_handler->NeedKeystoreKey(&trans); +} + + +SyncerError HandleGetEncryptionKeyResponse( + const sync_pb::ClientToServerResponse& update_response, + syncable::Directory* dir) { + bool success = false; + if (update_response.get_updates().encryption_keys_size() == 0) { + LOG(ERROR) << "Failed to receive encryption key from server."; + return SERVER_RESPONSE_VALIDATION_FAILED; + } + syncable::ReadTransaction trans(FROM_HERE, dir); + syncable::NigoriHandler* nigori_handler = dir->GetNigoriHandler(); + success = nigori_handler->SetKeystoreKeys( + update_response.get_updates().encryption_keys(), + &trans); + + DVLOG(1) << "GetUpdates returned " + << update_response.get_updates().encryption_keys_size() + << "encryption keys. Nigori keystore key " + << (success ? "" : "not ") << "updated."; + return (success ? SYNCER_OK : SERVER_RESPONSE_VALIDATION_FAILED); +} + // Given a GetUpdates response, iterates over all the returned items and // divides them according to their type. Outputs a map from model types to // received SyncEntities. The output map will have entries (possibly empty) @@ -74,6 +110,34 @@ void PartitionProgressMarkersByType( } } +// Initializes the parts of the GetUpdatesMessage that depend on shared state, +// like the ShouldRequestEncryptionKey() status. This is kept separate from the +// other of the message-building functions to make the rest of the code easier +// to test. +void InitDownloadUpdatesContext( + sessions::SyncSession* session, + bool create_mobile_bookmarks_folder, + sync_pb::ClientToServerMessage* message) { + message->set_share(session->context()->account_name()); + message->set_message_contents(sync_pb::ClientToServerMessage::GET_UPDATES); + + sync_pb::GetUpdatesMessage* get_updates = message->mutable_get_updates(); + + // We want folders for our associated types, always. If we were to set + // this to false, the server would send just the non-container items + // (e.g. Bookmark URLs but not their containing folders). + get_updates->set_fetch_folders(true); + + get_updates->set_create_mobile_bookmarks_folder( + create_mobile_bookmarks_folder); + bool need_encryption_key = ShouldRequestEncryptionKey(session->context()); + get_updates->set_need_encryption_key(need_encryption_key); + + // Set legacy GetUpdatesMessage.GetUpdatesCallerInfo information. + get_updates->mutable_caller_info()->set_notifications_enabled( + session->context()->notifications_enabled()); +} + } // namespace GetUpdatesProcessor::GetUpdatesProcessor(UpdateHandlerMap* update_handler_map, @@ -82,9 +146,29 @@ GetUpdatesProcessor::GetUpdatesProcessor(UpdateHandlerMap* update_handler_map, GetUpdatesProcessor::~GetUpdatesProcessor() {} +SyncerError GetUpdatesProcessor::DownloadUpdates( + ModelTypeSet request_types, + sessions::SyncSession* session, + bool create_mobile_bookmarks_folder) { + TRACE_EVENT0("sync", "DownloadUpdates"); + + sync_pb::ClientToServerMessage message; + InitDownloadUpdatesContext(session, + create_mobile_bookmarks_folder, + &message); + PrepareGetUpdates(request_types, &message); + + SyncerError result = ExecuteDownloadUpdates(request_types, session, &message); + session->mutable_status_controller()->set_last_download_updates_result( + result); + return result; +} + void GetUpdatesProcessor::PrepareGetUpdates( ModelTypeSet gu_types, - sync_pb::GetUpdatesMessage* get_updates) { + sync_pb::ClientToServerMessage* message) { + sync_pb::GetUpdatesMessage* get_updates = message->mutable_get_updates(); + for (ModelTypeSet::Iterator it = gu_types.First(); it.Good(); it.Inc()) { UpdateHandlerMap::iterator handler_it = update_handler_map_->find(it.Get()); DCHECK(handler_it != update_handler_map_->end()); @@ -95,6 +179,80 @@ void GetUpdatesProcessor::PrepareGetUpdates( delegate_.HelpPopulateGuMessage(get_updates); } +SyncerError GetUpdatesProcessor::ExecuteDownloadUpdates( + ModelTypeSet request_types, + sessions::SyncSession* session, + sync_pb::ClientToServerMessage* msg) { + sync_pb::ClientToServerResponse update_response; + sessions::StatusController* status = session->mutable_status_controller(); + bool need_encryption_key = ShouldRequestEncryptionKey(session->context()); + + if (session->context()->debug_info_getter()) { + sync_pb::DebugInfo* debug_info = msg->mutable_debug_info(); + CopyClientDebugInfo(session->context()->debug_info_getter(), debug_info); + } + + SyncerError result = SyncerProtoUtil::PostClientToServerMessage( + msg, + &update_response, + session); + + DVLOG(2) << SyncerProtoUtil::ClientToServerResponseDebugString( + update_response); + + if (result != SYNCER_OK) { + LOG(ERROR) << "PostClientToServerMessage() failed during GetUpdates"; + return result; + } + + DVLOG(1) << "GetUpdates " + << " returned " << update_response.get_updates().entries_size() + << " updates and indicated " + << update_response.get_updates().changes_remaining() + << " updates left on server."; + + if (session->context()->debug_info_getter()) { + // Clear debug info now that we have successfully sent it to the server. + DVLOG(1) << "Clearing client debug info."; + session->context()->debug_info_getter()->ClearDebugInfo(); + } + + if (need_encryption_key || + update_response.get_updates().encryption_keys_size() > 0) { + syncable::Directory* dir = session->context()->directory(); + status->set_last_get_key_result( + HandleGetEncryptionKeyResponse(update_response, dir)); + } + + return ProcessResponse(update_response.get_updates(), + request_types, + status); +} + +SyncerError GetUpdatesProcessor::ProcessResponse( + const sync_pb::GetUpdatesResponse& gu_response, + ModelTypeSet request_types, + sessions::StatusController* status) { + status->increment_num_updates_downloaded_by(gu_response.entries_size()); + + // The changes remaining field is used to prevent the client from looping. If + // that field is being set incorrectly, we're in big trouble. + if (!gu_response.has_changes_remaining()) { + return SERVER_RESPONSE_VALIDATION_FAILED; + } + status->set_num_server_changes_remaining(gu_response.changes_remaining()); + + if (!ProcessGetUpdatesResponse(request_types, gu_response, status)) { + return SERVER_RESPONSE_VALIDATION_FAILED; + } + + if (gu_response.changes_remaining() == 0) { + return SYNCER_OK; + } else { + return SERVER_MORE_TO_DOWNLOAD; + } +} + bool GetUpdatesProcessor::ProcessGetUpdatesResponse( ModelTypeSet gu_types, const sync_pb::GetUpdatesResponse& gu_response, @@ -148,4 +306,11 @@ void GetUpdatesProcessor::ApplyUpdates( delegate_.ApplyUpdates(status_controller, update_handler_map_); } +void GetUpdatesProcessor::CopyClientDebugInfo( + sessions::DebugInfoGetter* debug_info_getter, + sync_pb::DebugInfo* debug_info) { + DVLOG(1) << "Copying client debug info to send."; + debug_info_getter->GetDebugInfo(debug_info); +} + } // namespace syncer diff --git a/sync/engine/get_updates_processor.h b/sync/engine/get_updates_processor.h index 13a9c83..787f267 100644 --- a/sync/engine/get_updates_processor.h +++ b/sync/engine/get_updates_processor.h @@ -9,9 +9,11 @@ #include <vector> #include "base/basictypes.h" +#include "base/gtest_prod_util.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/engine/model_safe_worker.h" +#include "sync/protocol/sync.pb.h" #include "sync/sessions/model_type_registry.h" namespace sync_pb { @@ -23,6 +25,9 @@ namespace syncer { namespace sessions { class StatusController; +class SyncSession; +class SyncSessionContext; +class DebugInfoGetter; } // namespace sessions namespace syncable { @@ -43,9 +48,36 @@ class SYNC_EXPORT_PRIVATE GetUpdatesProcessor { const GetUpdatesDelegate& delegate); ~GetUpdatesProcessor(); + // Downloads and processes a batch of updates for the specified types. + // + // Returns SYNCER_OK if the download succeeds, SERVER_MORE_TO_DOWNLOAD if the + // download succeeded but there are still some updates left to fetch on the + // server, or an appropriate error value in case of failure. + SyncerError DownloadUpdates( + ModelTypeSet request_types, + sessions::SyncSession* session, + bool create_mobile_bookmarks_folder); + + // Applies any downloaded and processed updates. + void ApplyUpdates(sessions::StatusController* status_controller); + + private: // Populates a GetUpdates request message with per-type information. - void PrepareGetUpdates(ModelTypeSet gu_types, - sync_pb::GetUpdatesMessage* get_updates); + void PrepareGetUpdates( + ModelTypeSet gu_types, + sync_pb::ClientToServerMessage* message); + + // Sends the specified message to the server and stores the response in a + // member of the |session|'s StatusController. + SyncerError ExecuteDownloadUpdates(ModelTypeSet request_types, + sessions::SyncSession* session, + sync_pb::ClientToServerMessage* msg); + + // Helper function for processing responses from the server. Defined here for + // testing. + SyncerError ProcessResponse(const sync_pb::GetUpdatesResponse& gu_response, + ModelTypeSet proto_request_types, + sessions::StatusController* status); // Processes a GetUpdates responses for each type. bool ProcessGetUpdatesResponse( @@ -53,10 +85,23 @@ class SYNC_EXPORT_PRIVATE GetUpdatesProcessor { const sync_pb::GetUpdatesResponse& gu_response, sessions::StatusController* status_controller); - // Hands off control to the delegate so it can apply updates. - void ApplyUpdates(sessions::StatusController* status_controller); + static void CopyClientDebugInfo( + sessions::DebugInfoGetter* debug_info_getter, + sync_pb::DebugInfo* debug_info); + + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, BookmarkNudge); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, NotifyMany); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, ConfigureTest); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, PollTest); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, RetryTest); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, NudgeWithRetryTest); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, InvalidResponse); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, MoreToDownloadResponse); + FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, NormalResponseTest); + FRIEND_TEST_ALL_PREFIXES(DownloadUpdatesDebugInfoTest, + VerifyCopyClientDebugInfo_Empty); + FRIEND_TEST_ALL_PREFIXES(DownloadUpdatesDebugInfoTest, VerifyCopyOverwrites); - private: // A map of 'update handlers', one for each enabled type. // This must be kept in sync with the routing info. Our temporary solution to // that problem is to initialize this map in set_routing_info(). diff --git a/sync/engine/download_unittest.cc b/sync/engine/get_updates_processor_unittest.cc index e114022..f8fd274 100644 --- a/sync/engine/download_unittest.cc +++ b/sync/engine/get_updates_processor_unittest.cc @@ -1,8 +1,8 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. +// Copyright 2014 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/engine/download.h" +#include "sync/engine/get_updates_processor.h" #include "base/message_loop/message_loop.h" #include "base/stl_util.h" @@ -23,9 +23,9 @@ namespace syncer { using sessions::MockDebugInfoGetter; // A test fixture for tests exercising download updates functions. -class DownloadUpdatesTest : public ::testing::Test { +class GetUpdatesProcessorTest : public ::testing::Test { protected: - DownloadUpdatesTest() : + GetUpdatesProcessorTest() : kTestStartTime(base::TimeTicks::Now()), update_handler_deleter_(&update_handler_map_) {} @@ -73,20 +73,21 @@ class DownloadUpdatesTest : public ::testing::Test { STLValueDeleter<UpdateHandlerMap> update_handler_deleter_; scoped_ptr<GetUpdatesProcessor> get_updates_processor_; - DISALLOW_COPY_AND_ASSIGN(DownloadUpdatesTest); + DISALLOW_COPY_AND_ASSIGN(GetUpdatesProcessorTest); }; // Basic test to make sure nudges are expressed properly in the request. -TEST_F(DownloadUpdatesTest, BookmarkNudge) { +TEST_F(GetUpdatesProcessorTest, BookmarkNudge) { sessions::NudgeTracker nudge_tracker; nudge_tracker.RecordLocalChange(ModelTypeSet(BOOKMARKS)); - sync_pb::GetUpdatesMessage gu_msg; + sync_pb::ClientToServerMessage message; NormalGetUpdatesDelegate normal_delegate(nudge_tracker); scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::LOCAL, gu_msg.caller_info().source()); EXPECT_EQ(sync_pb::SyncEnums::GU_TRIGGER, gu_msg.get_updates_origin()); @@ -115,7 +116,7 @@ TEST_F(DownloadUpdatesTest, BookmarkNudge) { } // Basic test to ensure invalidation payloads are expressed in the request. -TEST_F(DownloadUpdatesTest, NotifyMany) { +TEST_F(GetUpdatesProcessorTest, NotifyMany) { sessions::NudgeTracker nudge_tracker; nudge_tracker.RecordRemoteInvalidation( BuildInvalidationMap(AUTOFILL, 1, "autofill_payload")); @@ -128,12 +129,13 @@ TEST_F(DownloadUpdatesTest, NotifyMany) { notified_types.Put(BOOKMARKS); notified_types.Put(PREFERENCES); - sync_pb::GetUpdatesMessage gu_msg; + sync_pb::ClientToServerMessage message; NormalGetUpdatesDelegate normal_delegate(nudge_tracker); scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::NOTIFICATION, gu_msg.caller_info().source()); EXPECT_EQ(sync_pb::SyncEnums::GU_TRIGGER, gu_msg.get_updates_origin()); @@ -159,14 +161,15 @@ TEST_F(DownloadUpdatesTest, NotifyMany) { } } -TEST_F(DownloadUpdatesTest, ConfigureTest) { - sync_pb::GetUpdatesMessage gu_msg; +TEST_F(GetUpdatesProcessorTest, ConfigureTest) { + sync_pb::ClientToServerMessage message; ConfigureGetUpdatesDelegate configure_delegate( sync_pb::GetUpdatesCallerInfo::RECONFIGURATION); scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(configure_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_EQ(sync_pb::SyncEnums::RECONFIGURATION, gu_msg.get_updates_origin()); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION, gu_msg.caller_info().source()); @@ -180,13 +183,14 @@ TEST_F(DownloadUpdatesTest, ConfigureTest) { EXPECT_TRUE(request_types().Equals(progress_types)); } -TEST_F(DownloadUpdatesTest, PollTest) { - sync_pb::GetUpdatesMessage gu_msg; +TEST_F(GetUpdatesProcessorTest, PollTest) { + sync_pb::ClientToServerMessage message; PollGetUpdatesDelegate poll_delegate; scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(poll_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_EQ(sync_pb::SyncEnums::PERIODIC, gu_msg.get_updates_origin()); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::PERIODIC, gu_msg.caller_info().source()); @@ -200,7 +204,7 @@ TEST_F(DownloadUpdatesTest, PollTest) { EXPECT_TRUE(request_types().Equals(progress_types)); } -TEST_F(DownloadUpdatesTest, RetryTest) { +TEST_F(GetUpdatesProcessorTest, RetryTest) { sessions::NudgeTracker nudge_tracker; // Schedule a retry. @@ -210,12 +214,13 @@ TEST_F(DownloadUpdatesTest, RetryTest) { // Get the nudge tracker to think the retry is due. nudge_tracker.SetSyncCycleStartTime(t1 + base::TimeDelta::FromSeconds(1)); - sync_pb::GetUpdatesMessage gu_msg; + sync_pb::ClientToServerMessage message; RetryGetUpdatesDelegate retry_delegate; scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(retry_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_EQ(sync_pb::SyncEnums::RETRY, gu_msg.get_updates_origin()); EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RETRY, gu_msg.caller_info().source()); @@ -230,7 +235,7 @@ TEST_F(DownloadUpdatesTest, RetryTest) { EXPECT_TRUE(request_types().Equals(progress_types)); } -TEST_F(DownloadUpdatesTest, NudgeWithRetryTest) { +TEST_F(GetUpdatesProcessorTest, NudgeWithRetryTest) { sessions::NudgeTracker nudge_tracker; // Schedule a retry. @@ -243,12 +248,13 @@ TEST_F(DownloadUpdatesTest, NudgeWithRetryTest) { // Record a local change, too. nudge_tracker.RecordLocalChange(ModelTypeSet(BOOKMARKS)); - sync_pb::GetUpdatesMessage gu_msg; + sync_pb::ClientToServerMessage message; NormalGetUpdatesDelegate normal_delegate(nudge_tracker); scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - processor->PrepareGetUpdates(request_types(), &gu_msg); + processor->PrepareGetUpdates(request_types(), &message); + const sync_pb::GetUpdatesMessage& gu_msg = message.get_updates(); EXPECT_NE(sync_pb::SyncEnums::RETRY, gu_msg.get_updates_origin()); EXPECT_NE(sync_pb::GetUpdatesCallerInfo::RETRY, gu_msg.caller_info().source()); @@ -257,7 +263,7 @@ TEST_F(DownloadUpdatesTest, NudgeWithRetryTest) { } // Verify that a bogus response message is detected. -TEST_F(DownloadUpdatesTest, InvalidResponse) { +TEST_F(GetUpdatesProcessorTest, InvalidResponse) { sync_pb::GetUpdatesResponse gu_response; InitFakeUpdateResponse(&gu_response); @@ -270,15 +276,14 @@ TEST_F(DownloadUpdatesTest, InvalidResponse) { sessions::StatusController status; scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - SyncerError error = download::ProcessResponse(gu_response, - request_types(), - processor.get(), - &status); + SyncerError error = processor->ProcessResponse(gu_response, + request_types(), + &status); EXPECT_EQ(error, SERVER_RESPONSE_VALIDATION_FAILED); } // Verify that we correctly detect when there's more work to be done. -TEST_F(DownloadUpdatesTest, MoreToDownloadResponse) { +TEST_F(GetUpdatesProcessorTest, MoreToDownloadResponse) { sync_pb::GetUpdatesResponse gu_response; InitFakeUpdateResponse(&gu_response); gu_response.set_changes_remaining(1); @@ -288,15 +293,14 @@ TEST_F(DownloadUpdatesTest, MoreToDownloadResponse) { sessions::StatusController status; scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - SyncerError error = download::ProcessResponse(gu_response, - request_types(), - processor.get(), - &status); + SyncerError error = processor->ProcessResponse(gu_response, + request_types(), + &status); EXPECT_EQ(error, SERVER_MORE_TO_DOWNLOAD); } // A simple scenario: No updates returned and nothing more to download. -TEST_F(DownloadUpdatesTest, NormalResponseTest) { +TEST_F(GetUpdatesProcessorTest, NormalResponseTest) { sync_pb::GetUpdatesResponse gu_response; InitFakeUpdateResponse(&gu_response); gu_response.set_changes_remaining(0); @@ -306,10 +310,9 @@ TEST_F(DownloadUpdatesTest, NormalResponseTest) { sessions::StatusController status; scoped_ptr<GetUpdatesProcessor> processor( BuildGetUpdatesProcessor(normal_delegate)); - SyncerError error = download::ProcessResponse(gu_response, - request_types(), - processor.get(), - &status); + SyncerError error = processor->ProcessResponse(gu_response, + request_types(), + &status); EXPECT_EQ(error, SYNCER_OK); } @@ -339,16 +342,16 @@ class DownloadUpdatesDebugInfoTest : public ::testing::Test { // Verify CopyClientDebugInfo when there are no events to upload. TEST_F(DownloadUpdatesDebugInfoTest, VerifyCopyClientDebugInfo_Empty) { sync_pb::DebugInfo debug_info; - download::CopyClientDebugInfo(debug_info_getter(), &debug_info); + GetUpdatesProcessor::CopyClientDebugInfo(debug_info_getter(), &debug_info); EXPECT_EQ(0, debug_info.events_size()); } TEST_F(DownloadUpdatesDebugInfoTest, VerifyCopyOverwrites) { sync_pb::DebugInfo debug_info; AddDebugEvent(); - download::CopyClientDebugInfo(debug_info_getter(), &debug_info); + GetUpdatesProcessor::CopyClientDebugInfo(debug_info_getter(), &debug_info); EXPECT_EQ(1, debug_info.events_size()); - download::CopyClientDebugInfo(debug_info_getter(), &debug_info); + GetUpdatesProcessor::CopyClientDebugInfo(debug_info_getter(), &debug_info); EXPECT_EQ(1, debug_info.events_size()); } diff --git a/sync/engine/syncer.cc b/sync/engine/syncer.cc index 21282de..240332c 100644 --- a/sync/engine/syncer.cc +++ b/sync/engine/syncer.cc @@ -14,7 +14,6 @@ #include "sync/engine/commit.h" #include "sync/engine/commit_processor.h" #include "sync/engine/conflict_resolver.h" -#include "sync/engine/download.h" #include "sync/engine/get_updates_delegate.h" #include "sync/engine/get_updates_processor.h" #include "sync/engine/net/server_connection_manager.h" @@ -142,20 +141,10 @@ bool Syncer::DownloadAndApplyUpdates( bool create_mobile_bookmarks_folder) { SyncerError download_result = UNSET; do { - TRACE_EVENT0("sync", "DownloadUpdates"); - sync_pb::ClientToServerMessage msg; - sync_pb::GetUpdatesMessage* gu_msg = msg.mutable_get_updates(); - - download::InitDownloadUpdatesContext( - session, create_mobile_bookmarks_folder, &msg); - get_updates_processor->PrepareGetUpdates(request_types, gu_msg); - - download_result = download::ExecuteDownloadUpdates(request_types, - session, - get_updates_processor, - &msg); - session->mutable_status_controller()->set_last_download_updates_result( - download_result); + download_result = get_updates_processor->DownloadUpdates( + request_types, + session, + create_mobile_bookmarks_folder); } while (download_result == SERVER_MORE_TO_DOWNLOAD); // Exit without applying if we're shutting down or an error was detected. diff --git a/sync/sync_core.gypi b/sync/sync_core.gypi index 5aa74f6..e973110 100644 --- a/sync/sync_core.gypi +++ b/sync/sync_core.gypi @@ -54,8 +54,6 @@ 'engine/directory_commit_contributor.h', 'engine/directory_update_handler.cc', 'engine/directory_update_handler.h', - 'engine/download.cc', - 'engine/download.h', 'engine/get_commit_ids.cc', 'engine/get_commit_ids.h', 'engine/get_updates_delegate.cc', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index e379107..634a741 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -274,9 +274,9 @@ 'internal_api/public/util/weak_handle_unittest.cc', 'engine/apply_control_data_updates_unittest.cc', 'engine/backoff_delay_provider_unittest.cc', - 'engine/download_unittest.cc', 'engine/directory_commit_contribution_unittest.cc', 'engine/directory_update_handler_unittest.cc', + 'engine/get_updates_processor_unittest.cc', 'engine/sync_scheduler_unittest.cc', 'engine/syncer_proto_util_unittest.cc', 'engine/syncer_unittest.cc', |