diff options
author | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-14 00:38:04 +0000 |
---|---|---|
committer | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-14 00:38:04 +0000 |
commit | 6ce93eac08ab2a2630b6cceeb4608f9f3f20ebde (patch) | |
tree | 770cb6d2035f7885733d576febf06e0d5ed279e8 | |
parent | 4588b3d14c1b2b91729ea4807d13c92e9e48b427 (diff) | |
download | chromium_src-6ce93eac08ab2a2630b6cceeb4608f9f3f20ebde.zip chromium_src-6ce93eac08ab2a2630b6cceeb4608f9f3f20ebde.tar.gz chromium_src-6ce93eac08ab2a2630b6cceeb4608f9f3f20ebde.tar.bz2 |
[Sync] Add datatype controller support for tracking association stats
SyncMergeResult is a new class that allows datatypes and sync itself to record
association information for later use by the debug info listener. We introduce
the class and add some plumbing at the DTC level to pass these on to the
model association manager (and from there to the debug listener).
Similarly, to track syncer changes, we pass a SyncMergeResult weak pointer
to the SharedChangeProcessor, which while it's valid will increment the deltas
as changes arrive (in a future patch). The weak pointer is invalidated at the end
of association by the DTC.
To simplify the DTC plumbing of merge results, StartFailed has been merged
into StartDone. Additionally, removed some old logging code attempting to
identify which datatype was being Stopped that isn't necessary anymore.
BUG=158576
Review URL: https://chromiumcodereview.appspot.com/11401002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167528 0039d316-1c4b-4281-b951-d872f2087c98
36 files changed, 723 insertions, 332 deletions
diff --git a/chrome/browser/sync/glue/app_notification_data_type_controller_unittest.cc b/chrome/browser/sync/glue/app_notification_data_type_controller_unittest.cc index b783276..59044af0 100644 --- a/chrome/browser/sync/glue/app_notification_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/app_notification_data_type_controller_unittest.cc @@ -171,7 +171,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, StartManagerReady) { SetActivateExpectations(); EXPECT_EQ(DataTypeController::NOT_RUNNING, app_notif_dtc_->state()); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); EXPECT_EQ(DataTypeController::RUNNING, app_notif_dtc_->state()); } @@ -204,7 +204,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, StartFirstRun) { SetStartExpectations(); InitAndLoadManager(); SetActivateExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _, _)); change_processor_->set_sync_model_has_user_created_nodes(false); Start(); @@ -216,7 +216,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, StartAssociationFailed) { SetStartExpectations(); InitAndLoadManager(); EXPECT_CALL(start_callback_, - Run(DataTypeController::ASSOCIATION_FAILED, _)); + Run(DataTypeController::ASSOCIATION_FAILED, _, _)); syncable_service_.set_merge_data_and_start_syncing_error( syncer::SyncError(FROM_HERE, "Error", syncer::APP_NOTIFICATIONS)); @@ -230,7 +230,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, SetStartExpectations(); InitAndLoadManager(); EXPECT_CALL(start_callback_, - Run(DataTypeController::UNRECOVERABLE_ERROR, _)); + Run(DataTypeController::UNRECOVERABLE_ERROR, _, _)); // Set up association to fail with an unrecoverable error. change_processor_->set_sync_model_has_user_created_nodes_success(false); @@ -244,7 +244,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, Stop) { InitAndLoadManager(); SetActivateExpectations(); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, app_notif_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); @@ -266,7 +266,7 @@ TEST_F(SyncAppNotificationDataTypeControllerTest, &AppNotificationDataTypeController::Stop)); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); EXPECT_EQ(DataTypeController::RUNNING, app_notif_dtc_->state()); EXPECT_TRUE(syncable_service_.syncing()); diff --git a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc b/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc index 3476a17..91de7eb 100644 --- a/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/autofill_data_type_controller_unittest.cc @@ -96,9 +96,10 @@ class SyncAutofillDataTypeControllerTest : public testing::Test { // Passed to AutofillDTC::Start(). void OnStartFinished(DataTypeController::StartResult result, - const syncer::SyncError& error) { + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { last_start_result_ = result; - last_start_error_ = error; + last_start_error_ = local_merge_result.error(); } void OnLoadFinished(syncer::ModelType type, syncer::SyncError error) { diff --git a/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc index 2cc0a1c..21961f1 100644 --- a/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/bookmark_data_type_controller_unittest.cc @@ -150,7 +150,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartDependentsReady) { EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state()); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state()); } @@ -160,7 +160,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartBookmarkModelNotReady) { EXPECT_CALL(*bookmark_model_, IsLoaded()).WillRepeatedly(Return(false)); SetAssociateExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); bookmark_dtc_->LoadModels( base::Bind(&ModelLoadCallbackMock::Run, base::Unretained(&model_load_callback_))); @@ -208,7 +208,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartFirstRun) { SetAssociateExpectations(); EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)). WillRepeatedly(DoAll(SetArgumentPointee<0>(false), Return(true))); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _, _)); Start(); } @@ -231,7 +231,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartOk) { EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)). WillRepeatedly(DoAll(SetArgumentPointee<0>(true), Return(true))); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); } @@ -249,7 +249,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, StartAssociationFailed) { syncer::BOOKMARKS))); EXPECT_CALL(start_callback_, - Run(DataTypeController::ASSOCIATION_FAILED, _)); + Run(DataTypeController::ASSOCIATION_FAILED, _, _)); Start(); EXPECT_EQ(DataTypeController::DISABLED, bookmark_dtc_->state()); } @@ -264,7 +264,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, EXPECT_CALL(*model_associator_, SyncModelHasUserCreatedNodes(_)). WillRepeatedly(DoAll(SetArgumentPointee<0>(false), Return(false))); EXPECT_CALL(start_callback_, - Run(DataTypeController::UNRECOVERABLE_ERROR, _)); + Run(DataTypeController::UNRECOVERABLE_ERROR, _, _)); Start(); EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state()); } @@ -289,7 +289,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, Stop) { EXPECT_EQ(DataTypeController::NOT_RUNNING, bookmark_dtc_->state()); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); EXPECT_EQ(DataTypeController::RUNNING, bookmark_dtc_->state()); bookmark_dtc_->Stop(); @@ -306,7 +306,7 @@ TEST_F(SyncBookmarkDataTypeControllerTest, OnSingleDatatypeUnrecoverableError) { &BookmarkDataTypeController::Stop)); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); // This should cause bookmark_dtc_->Stop() to be called. bookmark_dtc_->OnSingleDatatypeUnrecoverableError(FROM_HERE, "Test"); diff --git a/chrome/browser/sync/glue/data_type_controller.cc b/chrome/browser/sync/glue/data_type_controller.cc index 2cd7340..d2dcc21 100644 --- a/chrome/browser/sync/glue/data_type_controller.cc +++ b/chrome/browser/sync/glue/data_type_controller.cc @@ -13,6 +13,10 @@ bool DataTypeController::IsUnrecoverableResult(StartResult result) { return (result == UNRECOVERABLE_ERROR); } +bool DataTypeController::IsSuccessfulResult(StartResult result) { + return (result == OK || result == OK_FIRST_RUN); +} + syncer::SyncError DataTypeController::CreateAndUploadError( const tracked_objects::Location& location, const std::string& message, diff --git a/chrome/browser/sync/glue/data_type_controller.h b/chrome/browser/sync/glue/data_type_controller.h index 2df59fa..7df8eed 100644 --- a/chrome/browser/sync/glue/data_type_controller.h +++ b/chrome/browser/sync/glue/data_type_controller.h @@ -13,6 +13,7 @@ #include "base/sequenced_task_runner_helpers.h" #include "chrome/browser/sync/glue/data_type_error_handler.h" #include "content/public/browser/browser_thread.h" +#include "sync/api/sync_merge_result.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/internal_api/public/util/unrecoverable_error_handler.h" @@ -63,7 +64,8 @@ class DataTypeController }; typedef base::Callback<void(StartResult, - const syncer::SyncError&)> StartCallback; + const syncer::SyncMergeResult&, + const syncer::SyncMergeResult&)> StartCallback; typedef base::Callback<void(syncer::ModelType, syncer::SyncError)> ModelLoadCallback; @@ -76,6 +78,9 @@ class DataTypeController // Public so unit tests can use this function as well. static bool IsUnrecoverableResult(StartResult result); + // Returns true if the datatype started successfully. + static bool IsSuccessfulResult(StartResult result); + // Begins asynchronous operation of loading the model to get it ready for // model association. Once the models are loaded the callback will be invoked // with the result. If the models are already loaded it is safe to call the diff --git a/chrome/browser/sync/glue/data_type_controller_mock.h b/chrome/browser/sync/glue/data_type_controller_mock.h index 796fb88..40ab896 100644 --- a/chrome/browser/sync/glue/data_type_controller_mock.h +++ b/chrome/browser/sync/glue/data_type_controller_mock.h @@ -16,8 +16,9 @@ class StartCallbackMock { StartCallbackMock(); virtual ~StartCallbackMock(); - MOCK_METHOD2(Run, void(DataTypeController::StartResult result, - const syncer::SyncError& error)); + MOCK_METHOD3(Run, void(DataTypeController::StartResult result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result)); }; class ModelLoadCallbackMock { diff --git a/chrome/browser/sync/glue/data_type_manager_impl.cc b/chrome/browser/sync/glue/data_type_manager_impl.cc index f3d7730..fed635f 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl.cc @@ -37,7 +37,8 @@ DataTypeManagerImpl::DataTypeManagerImpl( needs_reconfigure_(false), last_configure_reason_(syncer::CONFIGURE_REASON_UNKNOWN), weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), - model_association_manager_(controllers, + model_association_manager_(debug_info_listener, + controllers, ALLOW_THIS_IN_INITIALIZER_LIST(this)), observer_(observer) { DCHECK(configurer_); diff --git a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc index 7176123..a5aac50 100644 --- a/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc +++ b/chrome/browser/sync/glue/data_type_manager_impl_unittest.cc @@ -14,7 +14,6 @@ #include "content/public/test/test_browser_thread.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/configure_reason.h" -#include "sync/internal_api/public/util/weak_handle.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chrome/browser/sync/glue/fake_data_type_controller.cc b/chrome/browser/sync/glue/fake_data_type_controller.cc index 09feff2..b0316bfa 100644 --- a/chrome/browser/sync/glue/fake_data_type_controller.cc +++ b/chrome/browser/sync/glue/fake_data_type_controller.cc @@ -57,17 +57,22 @@ void FakeDataTypeController::FinishStart(StartResult result) { } // Set |state_| first below since the callback may call state(). - syncer::SyncError error; + syncer::SyncMergeResult local_merge_result(type()); + syncer::SyncMergeResult syncer_merge_result(type()); if (result <= OK_FIRST_RUN) { state_ = RUNNING; } else if (result == ASSOCIATION_FAILED) { state_ = DISABLED; - error.Reset(FROM_HERE, "Association failed", type_); + local_merge_result.set_error( + syncer::SyncError(FROM_HERE, "Association failed", type())); } else { state_ = NOT_RUNNING; - error.Reset(FROM_HERE, "Fake error", type_); + local_merge_result.set_error( + syncer::SyncError(FROM_HERE, "Fake error", type())); } - last_start_callback_.Run(result, error); + last_start_callback_.Run(result, + local_merge_result, + syncer_merge_result); last_start_callback_.Reset(); } @@ -84,7 +89,11 @@ void FakeDataTypeController::Stop() { // The DTM still expects |last_start_callback_| to be called back. if (!last_start_callback_.is_null()) { syncer::SyncError error(FROM_HERE, "Fake error", type_); - last_start_callback_.Run(ABORTED, error); + syncer::SyncMergeResult local_merge_result(type_); + local_merge_result.set_error(error); + last_start_callback_.Run(ABORTED, + local_merge_result, + syncer::SyncMergeResult(type_)); } } diff --git a/chrome/browser/sync/glue/frontend_data_type_controller.cc b/chrome/browser/sync/glue/frontend_data_type_controller.cc index 234dd17..704063c 100644 --- a/chrome/browser/sync/glue/frontend_data_type_controller.cc +++ b/chrome/browser/sync/glue/frontend_data_type_controller.cc @@ -160,26 +160,31 @@ bool FrontendDataTypeController::StartModels() { bool FrontendDataTypeController::Associate() { DCHECK_EQ(state_, ASSOCIATING); + syncer::SyncMergeResult local_merge_result(type()); + syncer::SyncMergeResult syncer_merge_result(type()); CreateSyncComponents(); if (!model_associator()->CryptoReadyIfNecessary()) { - StartFailed(NEEDS_CRYPTO, syncer::SyncError()); + StartDone(NEEDS_CRYPTO, local_merge_result, syncer_merge_result); return false; } bool sync_has_nodes = false; if (!model_associator()->SyncModelHasUserCreatedNodes(&sync_has_nodes)) { syncer::SyncError error(FROM_HERE, "Failed to load sync nodes", type()); - StartFailed(UNRECOVERABLE_ERROR, error); + local_merge_result.set_error(error); + StartDone(UNRECOVERABLE_ERROR, local_merge_result, syncer_merge_result); return false; } + // TODO(zea): Have AssociateModels fill the local and syncer merge results. base::TimeTicks start_time = base::TimeTicks::Now(); syncer::SyncError error; error = model_associator()->AssociateModels(); // TODO(lipalani): crbug.com/122690 - handle abort. RecordAssociationTime(base::TimeTicks::Now() - start_time); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, local_merge_result, syncer_merge_result); return false; } @@ -189,7 +194,9 @@ bool FrontendDataTypeController::Associate() { // FinishStart() invokes the DataTypeManager callback, which can lead to a // call to Stop() if one of the other data types being started generates an // error. - FinishStart(!sync_has_nodes ? OK_FIRST_RUN : OK); + StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, + local_merge_result, + syncer_merge_result); // Return false if we're not in the RUNNING state (due to Stop() being called // from FinishStart()). // TODO(zea/atwilson): Should we maybe move the call to FinishStart() out of @@ -208,29 +215,6 @@ void FrontendDataTypeController::CleanUp() { change_processor_.reset(); } -void FrontendDataTypeController::StartFailed(StartResult result, - const syncer::SyncError& error) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - - if (IsUnrecoverableResult(result)) - RecordUnrecoverableError(FROM_HERE, "StartFailed"); - - CleanUp(); - if (result == ASSOCIATION_FAILED) { - state_ = DISABLED; - } else { - state_ = NOT_RUNNING; - } - RecordStartFailure(result); - - // We have to release the callback before we call it, since it's possible - // invoking the callback will trigger a call to STOP(), which will get - // confused by the non-NULL start_callback_. - StartCallback callback = start_callback_; - start_callback_.Reset(); - callback.Run(result, error); -} - void FrontendDataTypeController::AbortModelLoad() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CleanUp(); @@ -242,15 +226,30 @@ void FrontendDataTypeController::AbortModelLoad() { type())); } -void FrontendDataTypeController::FinishStart(StartResult result) { +void FrontendDataTypeController::StartDone( + StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!IsSuccessfulResult(start_result)) { + if (IsUnrecoverableResult(start_result)) + RecordUnrecoverableError(FROM_HERE, "StartFailed"); + + CleanUp(); + if (start_result == ASSOCIATION_FAILED) { + state_ = DISABLED; + } else { + state_ = NOT_RUNNING; + } + RecordStartFailure(start_result); + } // We have to release the callback before we call it, since it's possible // invoking the callback will trigger a call to STOP(), which will get // confused by the non-NULL start_callback_. StartCallback callback = start_callback_; start_callback_.Reset(); - callback.Run(result, syncer::SyncError()); + callback.Run(start_result, local_merge_result, syncer_merge_result); } void FrontendDataTypeController::RecordAssociationTime(base::TimeDelta time) { diff --git a/chrome/browser/sync/glue/frontend_data_type_controller.h b/chrome/browser/sync/glue/frontend_data_type_controller.h index e18288f..aec7348 100644 --- a/chrome/browser/sync/glue/frontend_data_type_controller.h +++ b/chrome/browser/sync/glue/frontend_data_type_controller.h @@ -87,9 +87,11 @@ class FrontendDataTypeController : public DataTypeController { // the datatype controller. The default implementation is a no-op. virtual void CleanUpState(); - // Helper methods for cleaning up state an running the start callback. - virtual void StartFailed(StartResult result, const syncer::SyncError& error); - virtual void FinishStart(StartResult result); + // Helper method for cleaning up state and running the start callback. + virtual void StartDone( + StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result); // Record association time. virtual void RecordAssociationTime(base::TimeDelta time); diff --git a/chrome/browser/sync/glue/frontend_data_type_controller_unittest.cc b/chrome/browser/sync/glue/frontend_data_type_controller_unittest.cc index ec34e50..40e9864 100644 --- a/chrome/browser/sync/glue/frontend_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/frontend_data_type_controller_unittest.cc @@ -119,7 +119,7 @@ class SyncFrontendDataTypeControllerTest : public testing::Test { void SetActivateExpectations(DataTypeController::StartResult result) { EXPECT_CALL(service_, ActivateDataType(_, _, _)); - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result, _, _)); } void SetStopExpectations() { @@ -134,7 +134,7 @@ class SyncFrontendDataTypeControllerTest : public testing::Test { EXPECT_CALL(*dtc_mock_, RecordUnrecoverableError(_, _)); EXPECT_CALL(*dtc_mock_, CleanUpState()); EXPECT_CALL(*dtc_mock_, RecordStartFailure(result)); - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result, _, _)); } void Start() { @@ -209,7 +209,7 @@ TEST_F(SyncFrontendDataTypeControllerTest, StartAssociationFailed) { EXPECT_CALL(*model_associator_, AssociateModels()). WillOnce(Return(syncer::SyncError(FROM_HERE, "error", - syncer::PREFERENCES))); + syncer::BOOKMARKS))); EXPECT_CALL(*dtc_mock_, RecordAssociationTime(_)); SetStartFailExpectations(DataTypeController::ASSOCIATION_FAILED); diff --git a/chrome/browser/sync/glue/model_association_manager.cc b/chrome/browser/sync/glue/model_association_manager.cc index 433c212..d8bd5b3 100644 --- a/chrome/browser/sync/glue/model_association_manager.cc +++ b/chrome/browser/sync/glue/model_association_manager.cc @@ -2,23 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <algorithm> -#include <functional> - -#include "base/debug/trace_event.h" - -#include "base/logging.h" -#include "base/message_loop.h" -#include "base/metrics/histogram.h" - #include "chrome/browser/sync/glue/model_association_manager.h" #include <algorithm> #include <functional> -#include "content/public/browser/browser_thread.h" #include "base/debug/trace_event.h" - #include "base/logging.h" #include "base/message_loop.h" #include "base/metrics/histogram.h" @@ -77,15 +66,49 @@ class SortComparator : public std::binary_function<DataTypeController*, std::map<syncer::ModelType, int>* order_; }; +syncer::DataTypeAssociationStats BuildAssociationStatsFromMergeResults( + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { + DCHECK_EQ(local_merge_result.model_type(), syncer_merge_result.model_type()); + syncer::DataTypeAssociationStats stats; + stats.model_type = local_merge_result.model_type(); + stats.had_error = local_merge_result.error().IsSet() || + syncer_merge_result.error().IsSet(); + stats.num_local_items_before_association = + local_merge_result.num_items_before_association(); + stats.num_sync_items_before_association = + syncer_merge_result.num_items_before_association(); + stats.num_local_items_after_association = + local_merge_result.num_items_after_association(); + stats.num_sync_items_after_association = + syncer_merge_result.num_items_after_association(); + stats.num_local_items_added = + local_merge_result.num_items_added(); + stats.num_local_items_deleted = + local_merge_result.num_items_deleted(); + stats.num_local_items_modified = + local_merge_result.num_items_modified(); + stats.num_sync_items_added = + syncer_merge_result.num_items_added(); + stats.num_sync_items_deleted = + syncer_merge_result.num_items_deleted(); + stats.num_sync_items_modified = + syncer_merge_result.num_items_modified(); + return stats; +} + } // namespace ModelAssociationManager::ModelAssociationManager( + const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>& + debug_info_listener, const DataTypeController::TypeMap* controllers, ModelAssociationResultProcessor* processor) : state_(IDLE), currently_associating_(NULL), controllers_(controllers), result_processor_(processor), + debug_info_listener_(debug_info_listener), weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { // Ensure all data type controllers are stopped. @@ -276,8 +299,9 @@ void ModelAssociationManager::AppendToFailedDatatypesAndLogError( } void ModelAssociationManager::TypeStartCallback( - DataTypeController::StartResult result, - const syncer::SyncError& error) { + DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); TRACE_EVENT_END0("sync", "ModelAssociation"); @@ -297,32 +321,47 @@ void ModelAssociationManager::TypeStartCallback( DataTypeController* started_dtc = currently_associating_; currently_associating_ = NULL; - if (result == DataTypeController::ASSOCIATION_FAILED) { + if (start_result == DataTypeController::ASSOCIATION_FAILED) { DVLOG(1) << "ModelAssociationManager: Encountered a failed type"; - AppendToFailedDatatypesAndLogError(result, error); + AppendToFailedDatatypesAndLogError(start_result, + local_merge_result.error()); + } + + // Track the merge results if we succeeded or an association failure + // occurred. + if ((DataTypeController::IsSuccessfulResult(start_result) || + start_result == DataTypeController::ASSOCIATION_FAILED) && + debug_info_listener_.IsInitialized()) { + syncer::DataTypeAssociationStats stats = + BuildAssociationStatsFromMergeResults(local_merge_result, + syncer_merge_result); + debug_info_listener_.Call( + FROM_HERE, + &syncer::DataTypeDebugInfoListener::OnDataTypeAssociationComplete, + stats); } // If the type started normally, continue to the next type. // If the type is waiting for the cryptographer, continue to the next type. // Once the cryptographer is ready, we'll attempt to restart this type. // If this type encountered a type specific error continue to the next type. - if (result == DataTypeController::NEEDS_CRYPTO || - result == DataTypeController::OK || - result == DataTypeController::OK_FIRST_RUN || - result == DataTypeController::ASSOCIATION_FAILED) { + if (start_result == DataTypeController::NEEDS_CRYPTO || + DataTypeController::IsSuccessfulResult(start_result) || + start_result == DataTypeController::ASSOCIATION_FAILED) { + DVLOG(1) << "ModelAssociationManager: type start callback returned " - << result << " so calling LoadModelForNextType"; + << start_result << " so calling LoadModelForNextType"; LoadModelForNextType(); return; } // Any other result requires reconfiguration. Pass it on through the callback. LOG(ERROR) << "Failed to configure " << started_dtc->name(); - DCHECK(error.IsSet()); - DCHECK_EQ(started_dtc->type(), error.type()); + DCHECK(local_merge_result.error().IsSet()); + DCHECK_EQ(started_dtc->type(), local_merge_result.error().type()); DataTypeManager::ConfigureStatus configure_status = DataTypeManager::ABORTED; - switch (result) { + switch (start_result) { case DataTypeController::ABORTED: configure_status = DataTypeManager::ABORTED; break; @@ -335,7 +374,7 @@ void ModelAssociationManager::TypeStartCallback( } std::list<syncer::SyncError> errors; - errors.push_back(error); + errors.push_back(local_merge_result.error()); // Put our state to idle. state_ = IDLE; diff --git a/chrome/browser/sync/glue/model_association_manager.h b/chrome/browser/sync/glue/model_association_manager.h index 520fdde..082289a 100644 --- a/chrome/browser/sync/glue/model_association_manager.h +++ b/chrome/browser/sync/glue/model_association_manager.h @@ -11,6 +11,8 @@ #include "base/timer.h" #include "chrome/browser/sync/glue/data_type_manager.h" +#include "sync/internal_api/public/data_type_debug_info_listener.h" +#include "sync/internal_api/public/util/weak_handle.h" // |ModelAssociationManager| does the heavy lifting for doing the actual model // association. It instructs DataTypeControllers to load models, start @@ -39,8 +41,11 @@ class ModelAssociationResultProcessor { // The class that is responsible for model association. class ModelAssociationManager { public: - ModelAssociationManager(const DataTypeController::TypeMap* controllers, - ModelAssociationResultProcessor* processor); + ModelAssociationManager( + const syncer::WeakHandle<syncer::DataTypeDebugInfoListener>& + debug_info_listener, + const DataTypeController::TypeMap* controllers, + ModelAssociationResultProcessor* processor); virtual ~ModelAssociationManager(); // Initializes the state to do the model association in future. This @@ -95,8 +100,9 @@ class ModelAssociationManager { // Callback passed to each data type controller on starting association. This // callback will be invoked when the model association is done. - void TypeStartCallback(DataTypeController::StartResult result, - const syncer::SyncError& error); + void TypeStartCallback(DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result); // Callback that will be invoked when the models finish loading. This callback // will be passed to |LoadModels| function. @@ -159,9 +165,20 @@ class ModelAssociationManager { // Set of all registered controllers. const DataTypeController::TypeMap* controllers_; + + // The processor in charge of handling model association results. ModelAssociationResultProcessor* result_processor_; - base::WeakPtrFactory<ModelAssociationManager> weak_ptr_factory_; + + // Timer to track and limit how long a datatype takes to model associate. base::OneShotTimer<ModelAssociationManager> timer_; + + // Sync's datatype debug info listener, which we pass model association + // statistics to. + const syncer::WeakHandle<syncer::DataTypeDebugInfoListener> + debug_info_listener_; + + base::WeakPtrFactory<ModelAssociationManager> weak_ptr_factory_; + DISALLOW_COPY_AND_ASSIGN(ModelAssociationManager); }; } // namespace browser_sync diff --git a/chrome/browser/sync/glue/model_association_manager_unittest.cc b/chrome/browser/sync/glue/model_association_manager_unittest.cc index e8a13d9..8d38176 100644 --- a/chrome/browser/sync/glue/model_association_manager_unittest.cc +++ b/chrome/browser/sync/glue/model_association_manager_unittest.cc @@ -53,9 +53,9 @@ ACTION_P(VerifyResult, expected_result) { EXPECT_TRUE(arg0.waiting_to_start.Equals(expected_result.waiting_to_start)); } -class ModelAssociationManagerTest : public testing::Test { +class SyncModelAssociationManagerTest : public testing::Test { public: - ModelAssociationManagerTest() : + SyncModelAssociationManagerTest() : ui_thread_(content::BrowserThread::UI, &ui_loop_) { } @@ -68,11 +68,13 @@ class ModelAssociationManagerTest : public testing::Test { // Start a type and make sure ModelAssociationManager callst the |Start| // method and calls the callback when it is done. -TEST_F(ModelAssociationManagerTest, SimpleModelStart) { +TEST_F(SyncModelAssociationManagerTest, SimpleModelStart) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); DataTypeManager::ConfigureResult expected_result( @@ -94,11 +96,13 @@ TEST_F(ModelAssociationManagerTest, SimpleModelStart) { } // Start a type and call stop before it finishes associating. -TEST_F(ModelAssociationManagerTest, StopModelBeforeFinish) { +TEST_F(SyncModelAssociationManagerTest, StopModelBeforeFinish) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); @@ -124,11 +128,13 @@ TEST_F(ModelAssociationManagerTest, StopModelBeforeFinish) { } // Start a type, let it finish and then call stop. -TEST_F(ModelAssociationManagerTest, StopAfterFinish) { +TEST_F(SyncModelAssociationManagerTest, StopAfterFinish) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); DataTypeManager::ConfigureResult expected_result( @@ -154,11 +160,13 @@ TEST_F(ModelAssociationManagerTest, StopAfterFinish) { } // Make a type fail model association and verify correctness. -TEST_F(ModelAssociationManagerTest, TypeFailModelAssociation) { +TEST_F(SyncModelAssociationManagerTest, TypeFailModelAssociation) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); std::list<syncer::SyncError> errors; @@ -183,11 +191,13 @@ TEST_F(ModelAssociationManagerTest, TypeFailModelAssociation) { } // Ensure configuring stops when a type returns a unrecoverable error. -TEST_F(ModelAssociationManagerTest, TypeReturnUnrecoverableError) { +TEST_F(SyncModelAssociationManagerTest, TypeReturnUnrecoverableError) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); std::list<syncer::SyncError> errors; @@ -211,15 +221,17 @@ TEST_F(ModelAssociationManagerTest, TypeReturnUnrecoverableError) { DataTypeController::UNRECOVERABLE_ERROR); } -TEST_F(ModelAssociationManagerTest, InitializeAbortsLoad) { +TEST_F(SyncModelAssociationManagerTest, InitializeAbortsLoad) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); controllers_[syncer::THEMES] = new FakeDataTypeController(syncer::THEMES); GetController(controllers_, syncer::BOOKMARKS)->SetDelayModelLoad(); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types(syncer::BOOKMARKS, syncer::THEMES); syncer::ModelTypeSet expected_types_waiting_to_load; @@ -283,14 +295,16 @@ TEST_F(ModelAssociationManagerTest, InitializeAbortsLoad) { // Start 2 types. One of which timeout loading. Ensure that type is // fully configured eventually. -TEST_F(ModelAssociationManagerTest, ModelStartWithSlowLoadingType) { +TEST_F(SyncModelAssociationManagerTest, ModelStartWithSlowLoadingType) { controllers_[syncer::BOOKMARKS] = new FakeDataTypeController(syncer::BOOKMARKS); controllers_[syncer::APPS] = new FakeDataTypeController(syncer::APPS); GetController(controllers_, syncer::BOOKMARKS)->SetDelayModelLoad(); - ModelAssociationManager model_association_manager(&controllers_, - &result_processor_); + ModelAssociationManager model_association_manager( + syncer::WeakHandle<syncer::DataTypeDebugInfoListener>(), + &controllers_, + &result_processor_); syncer::ModelTypeSet types; types.Put(syncer::BOOKMARKS); types.Put(syncer::APPS); diff --git a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.cc b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.cc index 627684a..f510c99 100644 --- a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.cc +++ b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.cc @@ -5,6 +5,7 @@ #include "chrome/browser/sync/glue/new_non_frontend_data_type_controller.h" #include "base/logging.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/sync/glue/shared_change_processor_ref.h" #include "chrome/browser/sync/profile_sync_components_factory.h" #include "chrome/browser/sync/profile_sync_service.h" @@ -77,7 +78,12 @@ void NewNonFrontendDataTypeController::StartAssociating( if (!StartAssociationAsync()) { syncer::SyncError error( FROM_HERE, "Failed to post StartAssociation", type()); - StartDoneImpl(ASSOCIATION_FAILED, NOT_RUNNING, error); + syncer::SyncMergeResult local_merge_result(type()); + local_merge_result.set_error(error); + StartDoneImpl(ASSOCIATION_FAILED, + NOT_RUNNING, + local_merge_result, + syncer::SyncMergeResult(type())); // StartDoneImpl should have called ClearSharedChangeProcessor(); DCHECK(!shared_change_processor_.get()); return; @@ -105,7 +111,10 @@ void NewNonFrontendDataTypeController::Stop() { return; // The datatype was never activated, we're done. case ASSOCIATING: set_state(STOPPING); - StartDoneImpl(ABORTED, NOT_RUNNING, syncer::SyncError()); + StartDoneImpl(ABORTED, + NOT_RUNNING, + syncer::SyncMergeResult(type()), + syncer::SyncMergeResult(type())); // We continue on to deactivate the datatype and stop the local service. break; case DISABLED: @@ -140,31 +149,49 @@ NewNonFrontendDataTypeController::NewNonFrontendDataTypeController() {} NewNonFrontendDataTypeController::~NewNonFrontendDataTypeController() {} void NewNonFrontendDataTypeController::StartDone( - DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error) { + DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI)); + + DataTypeController::State new_state; + if (IsSuccessfulResult(start_result)) { + new_state = RUNNING; + } else { + new_state = (start_result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING); + } + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind( &NewNonFrontendDataTypeController::StartDoneImpl, this, - result, + start_result, new_state, - error)); + local_merge_result, + syncer_merge_result)); } void NewNonFrontendDataTypeController::StartDoneImpl( - DataTypeController::StartResult result, + DataTypeController::StartResult start_result, DataTypeController::State new_state, - const syncer::SyncError& error) { + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + if (IsUnrecoverableResult(start_result)) + RecordUnrecoverableError(FROM_HERE, "StartFailed"); + // If we failed to start up, and we haven't been stopped yet, we need to // ensure we clean up the local service and shared change processor properly. if (new_state != RUNNING && state() != NOT_RUNNING && state() != STOPPING) { ClearSharedChangeProcessor(); StopLocalServiceAsync(); } - NonFrontendDataTypeController::StartDoneImpl(result, new_state, error); + + NonFrontendDataTypeController::StartDoneImpl(start_result, + new_state, + local_merge_result, + syncer_merge_result); } void NewNonFrontendDataTypeController::AbortModelStarting() { @@ -199,53 +226,79 @@ void NewNonFrontendDataTypeController:: const scoped_refptr<SharedChangeProcessor>& shared_change_processor) { DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(shared_change_processor.get()); + syncer::SyncMergeResult local_merge_result(type()); + syncer::SyncMergeResult syncer_merge_result(type()); + base::WeakPtrFactory<syncer::SyncMergeResult> weak_ptr_factory( + &syncer_merge_result); // Connect |shared_change_processor| to the syncer and get the // syncer::SyncableService associated with type(). // Note that it's possible the shared_change_processor has already been // disconnected at this point, so all our accesses to the syncer from this // point on are through it. - local_service_ = shared_change_processor->Connect(profile_sync_factory(), - profile_sync_service(), - this, - type()); + local_service_ = shared_change_processor->Connect( + profile_sync_factory(), + profile_sync_service(), + this, + type(), + weak_ptr_factory.GetWeakPtr()); if (!local_service_.get()) { syncer::SyncError error(FROM_HERE, "Failed to connect to syncer.", type()); - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } if (!shared_change_processor->CryptoReadyIfNecessary()) { - StartFailed(NEEDS_CRYPTO, syncer::SyncError()); + StartDone(NEEDS_CRYPTO, + local_merge_result, + syncer_merge_result); return; } bool sync_has_nodes = false; if (!shared_change_processor->SyncModelHasUserCreatedNodes(&sync_has_nodes)) { syncer::SyncError error(FROM_HERE, "Failed to load sync nodes", type()); - StartFailed(UNRECOVERABLE_ERROR, error); + local_merge_result.set_error(error); + StartDone(UNRECOVERABLE_ERROR, + local_merge_result, + syncer_merge_result); return; } base::TimeTicks start_time = base::TimeTicks::Now(); - syncer::SyncError error; syncer::SyncDataList initial_sync_data; - error = shared_change_processor->GetSyncData(&initial_sync_data); + syncer::SyncError error = + shared_change_processor->GetSyncData(&initial_sync_data); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } // Passes a reference to |shared_change_processor|. - error = local_service_->MergeDataAndStartSyncing( - type(), - initial_sync_data, - scoped_ptr<syncer::SyncChangeProcessor>( - new SharedChangeProcessorRef(shared_change_processor)), - scoped_ptr<syncer::SyncErrorFactory>( - new SharedChangeProcessorRef(shared_change_processor))); + // TODO(zea): have SyncableService return a SyncMergeResult and pass that on + // to the ModelAssociationManager. + // TODO(zea): Call shared_change_processor_->GetAllSyncData(..) before + // and after MergeDataAndStartSyncing and store the item counts into + // syncer_merge_result. + error = + local_service_->MergeDataAndStartSyncing( + type(), + initial_sync_data, + scoped_ptr<syncer::SyncChangeProcessor>( + new SharedChangeProcessorRef(shared_change_processor)), + scoped_ptr<syncer::SyncErrorFactory>( + new SharedChangeProcessorRef(shared_change_processor))); RecordAssociationTime(base::TimeTicks::Now() - start_time); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } @@ -256,7 +309,9 @@ void NewNonFrontendDataTypeController:: // doesn't start trying to push changes from its thread before we activate // the datatype. shared_change_processor->ActivateDataType(model_safe_group()); - StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, RUNNING, syncer::SyncError()); + StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, + local_merge_result, + syncer_merge_result); } void NewNonFrontendDataTypeController::ClearSharedChangeProcessor() { diff --git a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h index fd45115..e8180b0 100644 --- a/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h +++ b/chrome/browser/sync/glue/new_non_frontend_data_type_controller.h @@ -40,12 +40,15 @@ class NewNonFrontendDataTypeController : public NonFrontendDataTypeController { virtual void OnModelLoaded() OVERRIDE; // Overrides of NonFrontendDataTypeController methods. - virtual void StartDone(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error) OVERRIDE; - virtual void StartDoneImpl(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error) OVERRIDE; + virtual void StartDone( + DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) OVERRIDE; + virtual void StartDoneImpl( + DataTypeController::StartResult start_result, + DataTypeController::State new_state, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) OVERRIDE; private: // This overrides the same method in |NonFrontendDataTypeController|. diff --git a/chrome/browser/sync/glue/new_non_frontend_data_type_controller_mock.h b/chrome/browser/sync/glue/new_non_frontend_data_type_controller_mock.h index b57ea1d..9841378 100644 --- a/chrome/browser/sync/glue/new_non_frontend_data_type_controller_mock.h +++ b/chrome/browser/sync/glue/new_non_frontend_data_type_controller_mock.h @@ -38,14 +38,15 @@ class NewNonFrontendDataTypeControllerMock const base::Closure&)); MOCK_METHOD0(StartAssociation, void()); MOCK_METHOD0(CreateSyncComponents, void()); - MOCK_METHOD2(StartFailed, void(StartResult result, - const syncer::SyncError& error)); - MOCK_METHOD3(StartDone, void(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error)); - MOCK_METHOD3(StartDoneImpl, void(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error)); + MOCK_METHOD3(StartDone, + void(DataTypeController::StartResult result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result)); + MOCK_METHOD4(StartDoneImpl, + void(DataTypeController::StartResult result, + DataTypeController::State new_state, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result)); MOCK_METHOD0(StopModels, void()); MOCK_METHOD0(StopAssociationAsync, bool()); MOCK_METHOD0(StopAssociation, void()); diff --git a/chrome/browser/sync/glue/new_non_frontend_data_type_controller_unittest.cc b/chrome/browser/sync/glue/new_non_frontend_data_type_controller_unittest.cc index 4005e41..b66644d 100644 --- a/chrome/browser/sync/glue/new_non_frontend_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/new_non_frontend_data_type_controller_unittest.cc @@ -197,7 +197,7 @@ class SyncNewNonFrontendDataTypeControllerTest : public testing::Test { } void SetAssociateExpectations() { - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillOnce(Return(true)); @@ -210,7 +210,7 @@ class SyncNewNonFrontendDataTypeControllerTest : public testing::Test { } void SetActivateExpectations(DataTypeController::StartResult result) { - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result,_,_)); } void SetStopExpectations() { @@ -224,7 +224,7 @@ class SyncNewNonFrontendDataTypeControllerTest : public testing::Test { if (DataTypeController::IsUnrecoverableResult(result)) EXPECT_CALL(*dtc_mock_, RecordUnrecoverableError(_, _)); EXPECT_CALL(*dtc_mock_, RecordStartFailure(result)); - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result,_,_)); } void Start() { @@ -268,7 +268,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, StartOk) { TEST_F(SyncNewNonFrontendDataTypeControllerTest, StartFirstRun) { SetStartExpectations(); - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillOnce(Return(true)); @@ -308,7 +308,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, AbortDuringStartModels) { // cleanly. TEST_F(SyncNewNonFrontendDataTypeControllerTest, StartAssociationFailed) { SetStartExpectations(); - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillOnce(Return(true)); @@ -335,7 +335,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, SetStartExpectations(); SetStartFailExpectations(DataTypeController::UNRECOVERABLE_ERROR); // Set up association to fail with an unrecoverable error. - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillRepeatedly(Return(true)); @@ -352,7 +352,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, SetStartExpectations(); SetStartFailExpectations(DataTypeController::NEEDS_CRYPTO); // Set up association to fail with a NEEDS_CRYPTO error. - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillRepeatedly(Return(false)); @@ -370,7 +370,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, AbortDuringAssociation) { SetStartExpectations(); SetStartFailExpectations(DataTypeController::ABORTED); - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(GetWeakPtrToSyncableService(&syncable_service_)); EXPECT_CALL(*change_processor_, CryptoReadyIfNecessary()). WillOnce(Return(true)); @@ -409,7 +409,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, FLAKY_StartAfterSyncShutdown) { EXPECT_CALL(*dtc_mock_, StopModels()); EXPECT_CALL(service_, DeactivateDataType(_)); EXPECT_CALL(*dtc_mock_, RecordStartFailure(DataTypeController::ABORTED)); - EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, new_non_frontend_dtc_->state()); Start(); new_non_frontend_dtc_->Stop(); @@ -419,7 +419,7 @@ TEST_F(SyncNewNonFrontendDataTypeControllerTest, FLAKY_StartAfterSyncShutdown) { Mock::VerifyAndClearExpectations(change_processor_); Mock::VerifyAndClearExpectations(dtc_mock_); - EXPECT_CALL(*change_processor_, Connect(_,_,_,_)). + EXPECT_CALL(*change_processor_, Connect(_,_,_,_,_)). WillOnce(Return(base::WeakPtr<syncer::SyncableService>())); new_non_frontend_dtc_->UnblockBackendTasks(); EXPECT_CALL(*dtc_mock_, RecordUnrecoverableError(_, _)); diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc index 1c3ddeb..938b817 100644 --- a/chrome/browser/sync/glue/non_frontend_data_type_controller.cc +++ b/chrome/browser/sync/glue/non_frontend_data_type_controller.cc @@ -89,7 +89,12 @@ void NonFrontendDataTypeController::StartAssociating( if (!StartAssociationAsync()) { syncer::SyncError error( FROM_HERE, "Failed to post StartAssociation", type()); - StartDoneImpl(ASSOCIATION_FAILED, DISABLED, error); + syncer::SyncMergeResult local_merge_result(type()); + local_merge_result.set_error(error); + StartDoneImpl(ASSOCIATION_FAILED, + DISABLED, + local_merge_result, + syncer::SyncMergeResult(type())); } } @@ -101,7 +106,10 @@ void NonFrontendDataTypeController::StopWhileAssociating() { if (model_associator_.get()) model_associator_->AbortAssociation(); if (!start_association_called_.IsSignaled()) { - StartDoneImpl(ABORTED, NOT_RUNNING, syncer::SyncError()); + StartDoneImpl(ABORTED, + NOT_RUNNING, + syncer::SyncMergeResult(type()), + syncer::SyncMergeResult(type())); return; // There is nothing more for us to do. } } @@ -128,7 +136,10 @@ void NonFrontendDataTypeController::StopWhileAssociating() { } - StartDoneImpl(ABORTED, STOPPING, syncer::SyncError()); + StartDoneImpl(ABORTED, + STOPPING, + syncer::SyncMergeResult(type()), + syncer::SyncMergeResult(type())); } namespace { @@ -154,55 +165,7 @@ void NonFrontendDataTypeController::Stop() { // thread to finish the StartImpl() task. switch (state_) { case ASSOCIATING: - if (type() == syncer::PASSWORDS) { - LOG(INFO) << " Type is Passwords"; - StopWhileAssociating(); - } else if (type() == syncer::TYPED_URLS) { - LOG(INFO) << " Type is TypedUrl"; - StopWhileAssociating(); - } else if (type() == syncer::APPS) { - LOG(INFO) << "Type is Apps"; - StopWhileAssociating(); - } else if (type() == syncer::EXTENSIONS) { - LOG(INFO) << "Type is Extension"; - StopWhileAssociating(); - } else if (type() == syncer::PREFERENCES) { - LOG(INFO) << "Type is Preferences(Does not belong to non-frontend)"; - StopWhileAssociating(); - } else if (type() == syncer::EXTENSION_SETTINGS) { - LOG(INFO) << "Type is Extension Settings"; - StopWhileAssociating(); - } else if (type() == syncer::APP_SETTINGS) { - LOG(INFO) << "Type is App Settings."; - StopWhileAssociating(); - } else if (type() == syncer::BOOKMARKS) { - LOG(INFO) << "Type is BOOKMARKS."; - StopWhileAssociating(); - } else if (type() == syncer::AUTOFILL_PROFILE) { - LOG(INFO) << "Type is AUTOFILL_PROFILE."; - StopWhileAssociating(); - } else if (type() == syncer::AUTOFILL) { - LOG(INFO) << "Type is AUTOFILL."; - StopWhileAssociating(); - } else if (type() == syncer::THEMES) { - LOG(INFO) << "Type is THEMES."; - StopWhileAssociating(); - } else if (type() == syncer::NIGORI) { - LOG(INFO) << "Type is NIGORI."; - StopWhileAssociating(); - } else if (type() == syncer::SEARCH_ENGINES) { - LOG(INFO) << "Type is SEARCH_ENGINES."; - StopWhileAssociating(); - } else if (type() == syncer::SESSIONS) { - LOG(INFO) << "Type is SESSIONS."; - StopWhileAssociating(); - } else if (type() == syncer::APP_NOTIFICATIONS) { - LOG(INFO) << "Type is APP_NOTIFICATIONS."; - StopWhileAssociating(); - } else { - LOG(INFO) << "Type is unknown"; - StopWhileAssociating(); - } + StopWhileAssociating(); // TODO(sync) : This should be cleaned up. Once we move to the new api // this should not be a problem. @@ -284,40 +247,40 @@ bool NonFrontendDataTypeController::StartModels() { return true; } -void NonFrontendDataTypeController::StartFailed( - StartResult result, - const syncer::SyncError& error) { +void NonFrontendDataTypeController::StartDone( + DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI)); + DataTypeController::State new_state; - if (IsUnrecoverableResult(result)) - RecordUnrecoverableError(FROM_HERE, "StartFailed"); - StopAssociation(); - StartDone(result, - result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING, - error); -} + if (IsSuccessfulResult(start_result)) { + new_state = RUNNING; + } else { + new_state = (start_result == ASSOCIATION_FAILED ? DISABLED : NOT_RUNNING); + if (IsUnrecoverableResult(start_result)) + RecordUnrecoverableError(FROM_HERE, "StartFailed"); + StopAssociation(); + } -void NonFrontendDataTypeController::StartDone( - DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error) { - DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI)); abort_association_complete_.Signal(); base::AutoLock lock(abort_association_lock_); if (!abort_association_) { BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(&NonFrontendDataTypeController::StartDoneImpl, this, - result, + start_result, new_state, - error)); + local_merge_result, + syncer_merge_result)); } } void NonFrontendDataTypeController::StartDoneImpl( - DataTypeController::StartResult result, + DataTypeController::StartResult start_result, DataTypeController::State new_state, - const syncer::SyncError& error) { + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // It's possible to have StartDoneImpl called first from the UI thread // (due to Stop being called) and then posted from the non-UI thread. In @@ -331,7 +294,7 @@ void NonFrontendDataTypeController::StartDoneImpl( if (state_ != RUNNING) { // Start failed. StopModels(); - RecordStartFailure(result); + RecordStartFailure(start_result); } // We have to release the callback before we call it, since it's possible @@ -339,7 +302,7 @@ void NonFrontendDataTypeController::StartDoneImpl( // confused by the non-NULL start_callback_. StartCallback callback = start_callback_; start_callback_.Reset(); - callback.Run(result, error); + callback.Run(start_result, local_merge_result, syncer_merge_result); } void NonFrontendDataTypeController::StopModels() { @@ -427,6 +390,8 @@ void NonFrontendDataTypeController::set_change_processor( void NonFrontendDataTypeController::StartAssociation() { DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI)); + syncer::SyncMergeResult local_merge_result(type()); + syncer::SyncMergeResult syncer_merge_result(type()); { base::AutoLock lock(abort_association_lock_); @@ -441,30 +406,40 @@ void NonFrontendDataTypeController::StartAssociation() { DCHECK_EQ(state_, ASSOCIATING); if (!model_associator_->CryptoReadyIfNecessary()) { - StartFailed(NEEDS_CRYPTO, syncer::SyncError()); + StartDone(NEEDS_CRYPTO, + local_merge_result, + syncer_merge_result); return; } bool sync_has_nodes = false; if (!model_associator_->SyncModelHasUserCreatedNodes(&sync_has_nodes)) { syncer::SyncError error(FROM_HERE, "Failed to load sync nodes", type()); - StartFailed(UNRECOVERABLE_ERROR, error); + local_merge_result.set_error(error); + StartDone(UNRECOVERABLE_ERROR, + local_merge_result, + syncer_merge_result); return; } + // TODO(zea): Have AssociateModels fill the local and syncer merge results. base::TimeTicks start_time = base::TimeTicks::Now(); - syncer::SyncError error; - error = model_associator_->AssociateModels(); + syncer::SyncError error = model_associator_->AssociateModels(); // TODO(lipalani): crbug.com/122690 - handle abort. RecordAssociationTime(base::TimeTicks::Now() - start_time); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } profile_sync_service_->ActivateDataType(type(), model_safe_group(), change_processor()); - StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, RUNNING, syncer::SyncError()); + StartDone(!sync_has_nodes ? OK_FIRST_RUN : OK, + local_merge_result, + syncer_merge_result); } bool NonFrontendDataTypeController::StopAssociationAsync() { diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller.h b/chrome/browser/sync/glue/non_frontend_data_type_controller.h index 52ceba1..60507cf 100644 --- a/chrome/browser/sync/glue/non_frontend_data_type_controller.h +++ b/chrome/browser/sync/glue/non_frontend_data_type_controller.h @@ -97,21 +97,19 @@ class NonFrontendDataTypeController : public DataTypeController { // Note: this is performed on the datatype's thread. virtual void CreateSyncComponents() = 0; - // Start failed, make sure we record it, clean up state, and invoke the - // callback on the frontend thread. - // Note: this is performed on the datatype's thread. - virtual void StartFailed(StartResult result, const syncer::SyncError& error); - // Start up complete, update the state and invoke the callback. // Note: this is performed on the datatype's thread. - virtual void StartDone(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error); + virtual void StartDone( + DataTypeController::StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result); // UI thread implementation of StartDone. - virtual void StartDoneImpl(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error); + virtual void StartDoneImpl( + DataTypeController::StartResult start_result, + DataTypeController::State new_state, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result); // Perform any DataType controller specific state cleanup before stopping // the datatype controller. The default implementation is a no-op. diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller_mock.h b/chrome/browser/sync/glue/non_frontend_data_type_controller_mock.h index df07dff..a9cbfaf 100644 --- a/chrome/browser/sync/glue/non_frontend_data_type_controller_mock.h +++ b/chrome/browser/sync/glue/non_frontend_data_type_controller_mock.h @@ -37,14 +37,15 @@ class NonFrontendDataTypeControllerMock : public NonFrontendDataTypeController { const base::Closure&)); MOCK_METHOD0(StartAssociation, void()); MOCK_METHOD0(CreateSyncComponents, void()); - MOCK_METHOD2(StartFailed, void(StartResult result, - const syncer::SyncError& error)); - MOCK_METHOD3(StartDone, void(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error)); - MOCK_METHOD3(StartDoneImpl, void(DataTypeController::StartResult result, - DataTypeController::State new_state, - const syncer::SyncError& error)); + MOCK_METHOD3(StartDone, + void(DataTypeController::StartResult result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result)); + MOCK_METHOD4(StartDoneImpl, + void(DataTypeController::StartResult result, + DataTypeController::State new_state, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result)); MOCK_METHOD0(StopModels, void()); MOCK_METHOD0(StopAssociation, void()); MOCK_METHOD2(OnUnrecoverableErrorImpl, void(const tracked_objects::Location&, diff --git a/chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc b/chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc index 6c43830..d1878ae 100644 --- a/chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/non_frontend_data_type_controller_unittest.cc @@ -155,7 +155,7 @@ class SyncNonFrontendDataTypeControllerTest : public testing::Test { void SetActivateExpectations(DataTypeController::StartResult result) { EXPECT_CALL(service_, ActivateDataType(_, _, _)); - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result, _, _)); } void SetStopExpectations() { @@ -174,7 +174,7 @@ class SyncNonFrontendDataTypeControllerTest : public testing::Test { WillOnce(Return(syncer::SyncError())); } EXPECT_CALL(*dtc_mock_, RecordStartFailure(result)); - EXPECT_CALL(start_callback_, Run(result,_)); + EXPECT_CALL(start_callback_, Run(result, _, _)); } static void SignalDone(WaitableEvent* done) { @@ -249,7 +249,7 @@ TEST_F(SyncNonFrontendDataTypeControllerTest, StartAssociationFailed) { WillOnce(DoAll(SetArgumentPointee<0>(true), Return(true))); EXPECT_CALL(*model_associator_, AssociateModels()). WillOnce( - Return(syncer::SyncError(FROM_HERE, "Error", syncer::AUTOFILL))); + Return(syncer::SyncError(FROM_HERE, "Error", syncer::BOOKMARKS))); EXPECT_CALL(*dtc_mock_, RecordAssociationTime(_)); SetStartFailExpectations(DataTypeController::ASSOCIATION_FAILED); // Set up association to fail with an association failed error. @@ -307,7 +307,7 @@ TEST_F(SyncNonFrontendDataTypeControllerTest, AbortDuringAssociationInactive) { WillOnce(Return(syncer::SyncError())); EXPECT_CALL(*dtc_mock_, RecordAssociationTime(_)); EXPECT_CALL(service_, ActivateDataType(_, _, _)); - EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED,_)); + EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED,_,_)); EXPECT_CALL(*dtc_mock_, RecordStartFailure(DataTypeController::ABORTED)); SetStopExpectations(); EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); @@ -338,7 +338,7 @@ TEST_F(SyncNonFrontendDataTypeControllerTest, AbortDuringAssociationActivated) { EXPECT_CALL(service_, ActivateDataType(_, _, _)).WillOnce(DoAll( SignalEvent(&wait_for_db_thread_pause), WaitOnEvent(&pause_db_thread))); - EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED,_)); + EXPECT_CALL(start_callback_, Run(DataTypeController::ABORTED,_,_)); EXPECT_CALL(*dtc_mock_, RecordStartFailure(DataTypeController::ABORTED)); SetStopExpectations(); EXPECT_EQ(DataTypeController::NOT_RUNNING, non_frontend_dtc_->state()); diff --git a/chrome/browser/sync/glue/password_data_type_controller.cc b/chrome/browser/sync/glue/password_data_type_controller.cc index d071cb9..c2c244c 100644 --- a/chrome/browser/sync/glue/password_data_type_controller.cc +++ b/chrome/browser/sync/glue/password_data_type_controller.cc @@ -57,7 +57,12 @@ bool PasswordDataTypeController::StartModels() { FROM_HERE, "PasswordStore not initialized, password datatype controller aborting.", type()); - StartDoneImpl(ASSOCIATION_FAILED, DISABLED, error); + syncer::SyncMergeResult local_merge_result(type()); + local_merge_result.set_error(error); + StartDoneImpl(ASSOCIATION_FAILED, + DISABLED, + local_merge_result, + syncer::SyncMergeResult(type())); return false; } return true; diff --git a/chrome/browser/sync/glue/search_engine_data_type_controller_unittest.cc b/chrome/browser/sync/glue/search_engine_data_type_controller_unittest.cc index 77456ef..0db4f7a 100644 --- a/chrome/browser/sync/glue/search_engine_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/search_engine_data_type_controller_unittest.cc @@ -113,7 +113,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, StartURLServiceReady) { // We want to start ready. PreloadTemplateURLService(); SetActivateExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, search_engine_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); @@ -148,7 +148,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, StartFirstRun) { PreloadTemplateURLService(); SetActivateExpectations(); change_processor_->set_sync_model_has_user_created_nodes(false); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _, _)); Start(); EXPECT_TRUE(syncable_service_.syncing()); @@ -159,7 +159,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, StartAssociationFailed) { PreloadTemplateURLService(); SetStopExpectations(); EXPECT_CALL(start_callback_, - Run(DataTypeController::ASSOCIATION_FAILED, _)); + Run(DataTypeController::ASSOCIATION_FAILED, _, _)); syncable_service_.set_merge_data_and_start_syncing_error( syncer::SyncError(FROM_HERE, "Error", syncer::SEARCH_ENGINES)); @@ -176,7 +176,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, SetStartExpectations(); PreloadTemplateURLService(); EXPECT_CALL(start_callback_, - Run(DataTypeController::UNRECOVERABLE_ERROR, _)); + Run(DataTypeController::UNRECOVERABLE_ERROR, _, _)); // Set up association to fail with an unrecoverable error. change_processor_->set_sync_model_has_user_created_nodes_success(false); @@ -190,7 +190,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, Stop) { PreloadTemplateURLService(); SetActivateExpectations(); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, search_engine_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); @@ -212,7 +212,7 @@ TEST_F(SyncSearchEngineDataTypeControllerTest, &SearchEngineDataTypeController::Stop)); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); Start(); // This should cause search_engine_dtc_->Stop() to be called. search_engine_dtc_->OnSingleDatatypeUnrecoverableError(FROM_HERE, "Test"); diff --git a/chrome/browser/sync/glue/shared_change_processor.cc b/chrome/browser/sync/glue/shared_change_processor.cc index 1e536d1..324e7a4 100644 --- a/chrome/browser/sync/glue/shared_change_processor.cc +++ b/chrome/browser/sync/glue/shared_change_processor.cc @@ -49,7 +49,8 @@ base::WeakPtr<syncer::SyncableService> SharedChangeProcessor::Connect( ProfileSyncComponentsFactory* sync_factory, ProfileSyncService* sync_service, DataTypeErrorHandler* error_handler, - syncer::ModelType type) { + syncer::ModelType type, + const base::WeakPtr<syncer::SyncMergeResult>& merge_result) { DCHECK(sync_factory); DCHECK(sync_service); DCHECK(error_handler); @@ -68,6 +69,8 @@ base::WeakPtr<syncer::SyncableService> SharedChangeProcessor::Connect( disconnected_ = true; return base::WeakPtr<syncer::SyncableService>(); } + + // TODO(zea): Pass |merge_result| to the generic change processor. generic_change_processor_ = sync_factory->CreateGenericChangeProcessor(sync_service_, error_handler, diff --git a/chrome/browser/sync/glue/shared_change_processor.h b/chrome/browser/sync/glue/shared_change_processor.h index 7bc5af0..a775fae 100644 --- a/chrome/browser/sync/glue/shared_change_processor.h +++ b/chrome/browser/sync/glue/shared_change_processor.h @@ -14,6 +14,7 @@ #include "sync/api/sync_change_processor.h" #include "sync/api/sync_error.h" #include "sync/api/sync_error_factory.h" +#include "sync/api/sync_merge_result.h" #include "sync/internal_api/public/engine/model_safe_worker.h" class ProfileSyncComponentsFactory; @@ -63,7 +64,8 @@ class SharedChangeProcessor ProfileSyncComponentsFactory* sync_factory, ProfileSyncService* sync_service, DataTypeErrorHandler* error_handler, - syncer::ModelType type); + syncer::ModelType type, + const base::WeakPtr<syncer::SyncMergeResult>& merge_result); // Disconnects from the generic change processor. May be called from any // thread. After this, all attempts to interact with the change processor by diff --git a/chrome/browser/sync/glue/shared_change_processor_mock.h b/chrome/browser/sync/glue/shared_change_processor_mock.h index e19c19f..f0b05ca 100644 --- a/chrome/browser/sync/glue/shared_change_processor_mock.h +++ b/chrome/browser/sync/glue/shared_change_processor_mock.h @@ -16,11 +16,12 @@ class SharedChangeProcessorMock : public SharedChangeProcessor { public: SharedChangeProcessorMock(); - MOCK_METHOD4(Connect, base::WeakPtr<syncer::SyncableService>( + MOCK_METHOD5(Connect, base::WeakPtr<syncer::SyncableService>( ProfileSyncComponentsFactory*, ProfileSyncService*, DataTypeErrorHandler*, - syncer::ModelType)); + syncer::ModelType, + const base::WeakPtr<syncer::SyncMergeResult>&)); MOCK_METHOD0(Disconnect, bool()); MOCK_METHOD2(ProcessSyncChanges, syncer::SyncError(const tracked_objects::Location&, diff --git a/chrome/browser/sync/glue/shared_change_processor_unittest.cc b/chrome/browser/sync/glue/shared_change_processor_unittest.cc index 16e0c7c..755990c 100644 --- a/chrome/browser/sync/glue/shared_change_processor_unittest.cc +++ b/chrome/browser/sync/glue/shared_change_processor_unittest.cc @@ -104,10 +104,12 @@ class SyncSharedChangeProcessorTest : public testing::Test { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); EXPECT_CALL(sync_factory_, GetSyncableServiceForType(syncer::AUTOFILL)). WillOnce(GetWeakPtrToSyncableService(db_syncable_service_.get())); - EXPECT_TRUE(shared_change_processor->Connect(&sync_factory_, - &sync_service_, - &error_handler_, - syncer::AUTOFILL)); + EXPECT_TRUE(shared_change_processor->Connect( + &sync_factory_, + &sync_service_, + &error_handler_, + syncer::AUTOFILL, + base::WeakPtr<syncer::SyncMergeResult>())); } MessageLoopForUI ui_loop_; diff --git a/chrome/browser/sync/glue/ui_data_type_controller.cc b/chrome/browser/sync/glue/ui_data_type_controller.cc index 0b5f755..51fbf2d 100644 --- a/chrome/browser/sync/glue/ui_data_type_controller.cc +++ b/chrome/browser/sync/glue/ui_data_type_controller.cc @@ -5,6 +5,7 @@ #include "chrome/browser/sync/glue/ui_data_type_controller.h" #include "base/logging.h" +#include "base/memory/weak_ptr.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/glue/shared_change_processor_ref.h" #include "chrome/browser/sync/profile_sync_components_factory.h" @@ -116,21 +117,32 @@ bool UIDataTypeController::StartModels() { void UIDataTypeController::Associate() { DCHECK_EQ(state_, ASSOCIATING); + syncer::SyncMergeResult local_merge_result(type()); + syncer::SyncMergeResult syncer_merge_result(type()); + base::WeakPtrFactory<syncer::SyncMergeResult> weak_ptr_factory( + &syncer_merge_result); // Connect |shared_change_processor_| to the syncer and get the // syncer::SyncableService associated with type(). - local_service_ = shared_change_processor_->Connect(profile_sync_factory_, - sync_service_, - this, - type()); + local_service_ = shared_change_processor_->Connect( + profile_sync_factory_, + sync_service_, + this, + type(), + weak_ptr_factory.GetWeakPtr()); if (!local_service_.get()) { syncer::SyncError error(FROM_HERE, "Failed to connect to syncer.", type()); - StartFailed(UNRECOVERABLE_ERROR, error); + local_merge_result.set_error(error); + StartDone(UNRECOVERABLE_ERROR, + local_merge_result, + syncer_merge_result); return; } if (!shared_change_processor_->CryptoReadyIfNecessary()) { - StartFailed(NEEDS_CRYPTO, syncer::SyncError()); + StartDone(NEEDS_CRYPTO, + local_merge_result, + syncer_merge_result); return; } @@ -138,7 +150,10 @@ void UIDataTypeController::Associate() { if (!shared_change_processor_->SyncModelHasUserCreatedNodes( &sync_has_nodes)) { syncer::SyncError error(FROM_HERE, "Failed to load sync nodes", type()); - StartFailed(UNRECOVERABLE_ERROR, error); + local_merge_result.set_error(error); + StartDone(UNRECOVERABLE_ERROR, + local_merge_result, + syncer_merge_result); return; } @@ -147,11 +162,19 @@ void UIDataTypeController::Associate() { syncer::SyncDataList initial_sync_data; error = shared_change_processor_->GetSyncData(&initial_sync_data); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } // Passes a reference to |shared_change_processor_|. + // TODO(zea): have SyncableService return a SyncMergeResult and pass that on + // to the ModelAssociationManager. + // TODO(zea): Call shared_change_processor_->GetAllSyncData(..) before + // and after MergeDataAndStartSyncing and store the item counts into + // syncer_merge_result. error = local_service_->MergeDataAndStartSyncing( type(), initial_sync_data, @@ -161,39 +184,18 @@ void UIDataTypeController::Associate() { new SharedChangeProcessorRef(shared_change_processor_))); RecordAssociationTime(base::TimeTicks::Now() - start_time); if (error.IsSet()) { - StartFailed(ASSOCIATION_FAILED, error); + local_merge_result.set_error(error); + StartDone(ASSOCIATION_FAILED, + local_merge_result, + syncer_merge_result); return; } shared_change_processor_->ActivateDataType(model_safe_group()); state_ = RUNNING; - StartDone(sync_has_nodes ? OK : OK_FIRST_RUN); -} - -void UIDataTypeController::StartFailed(StartResult result, - const syncer::SyncError& error) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (IsUnrecoverableResult(result)) - RecordUnrecoverableError(FROM_HERE, "StartFailed"); - StopModels(); - if (result == ASSOCIATION_FAILED) { - state_ = DISABLED; - } else { - state_ = NOT_RUNNING; - } - RecordStartFailure(result); - - if (shared_change_processor_.get()) { - shared_change_processor_->Disconnect(); - shared_change_processor_ = NULL; - } - - // We have to release the callback before we call it, since it's possible - // invoking the callback will trigger a call to Stop(), which will get - // confused by the non-NULL start_callback_. - StartCallback callback = start_callback_; - start_callback_.Reset(); - callback.Run(result, error); + StartDone(sync_has_nodes ? OK : OK_FIRST_RUN, + local_merge_result, + syncer_merge_result); } void UIDataTypeController::AbortModelLoad() { @@ -211,15 +213,36 @@ void UIDataTypeController::AbortModelLoad() { type())); } -void UIDataTypeController::StartDone(StartResult result) { +void UIDataTypeController::StartDone( + StartResult start_result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + if (!IsSuccessfulResult(start_result)) { + if (IsUnrecoverableResult(start_result)) + RecordUnrecoverableError(FROM_HERE, "StartFailed"); + + StopModels(); + if (start_result == ASSOCIATION_FAILED) { + state_ = DISABLED; + } else { + state_ = NOT_RUNNING; + } + RecordStartFailure(start_result); + + if (shared_change_processor_.get()) { + shared_change_processor_->Disconnect(); + shared_change_processor_ = NULL; + } + } + // We have to release the callback before we call it, since it's possible // invoking the callback will trigger a call to Stop(), which will get // confused by the non-NULL start_callback_. StartCallback callback = start_callback_; start_callback_.Reset(); - callback.Run(result, syncer::SyncError()); + callback.Run(start_result, local_merge_result, syncer_merge_result); } void UIDataTypeController::Stop() { diff --git a/chrome/browser/sync/glue/ui_data_type_controller.h b/chrome/browser/sync/glue/ui_data_type_controller.h index 40dd418..dbd6f08 100644 --- a/chrome/browser/sync/glue/ui_data_type_controller.h +++ b/chrome/browser/sync/glue/ui_data_type_controller.h @@ -77,9 +77,10 @@ class UIDataTypeController : public DataTypeController { // DataTypeController interface. virtual void OnModelLoaded() OVERRIDE; - // Helper methods for cleaning up state and invoking the start callback. - virtual void StartFailed(StartResult result, const syncer::SyncError& error); - virtual void StartDone(StartResult result); + // Helper method for cleaning up state and invoking the start callback. + virtual void StartDone(StartResult result, + const syncer::SyncMergeResult& local_merge_result, + const syncer::SyncMergeResult& syncer_merge_result); // Record association time. virtual void RecordAssociationTime(base::TimeDelta time); diff --git a/chrome/browser/sync/glue/ui_data_type_controller_unittest.cc b/chrome/browser/sync/glue/ui_data_type_controller_unittest.cc index 042c72f..b1a5d55 100644 --- a/chrome/browser/sync/glue/ui_data_type_controller_unittest.cc +++ b/chrome/browser/sync/glue/ui_data_type_controller_unittest.cc @@ -113,7 +113,7 @@ class SyncUIDataTypeControllerTest : public testing::Test { // state. TEST_F(SyncUIDataTypeControllerTest, Start) { SetActivateExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); @@ -127,7 +127,7 @@ TEST_F(SyncUIDataTypeControllerTest, Start) { TEST_F(SyncUIDataTypeControllerTest, StartStop) { SetActivateExpectations(); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); @@ -144,7 +144,7 @@ TEST_F(SyncUIDataTypeControllerTest, StartStop) { TEST_F(SyncUIDataTypeControllerTest, StartStopFirstRun) { SetActivateExpectations(); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK_FIRST_RUN, _, _)); change_processor_->set_sync_model_has_user_created_nodes(false); EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state()); @@ -163,7 +163,7 @@ TEST_F(SyncUIDataTypeControllerTest, StartStopFirstRun) { TEST_F(SyncUIDataTypeControllerTest, StartAssociationFailed) { SetStopExpectations(); EXPECT_CALL(start_callback_, - Run(DataTypeController::ASSOCIATION_FAILED, _)); + Run(DataTypeController::ASSOCIATION_FAILED, _, _)); syncable_service_.set_merge_data_and_start_syncing_error( syncer::SyncError(FROM_HERE, "Error", type_)); @@ -183,7 +183,7 @@ TEST_F(SyncUIDataTypeControllerTest, StartAssociationFailed) { TEST_F(SyncUIDataTypeControllerTest, StartAssociationTriggersUnrecoverableError) { EXPECT_CALL(start_callback_, - Run(DataTypeController::UNRECOVERABLE_ERROR, _)); + Run(DataTypeController::UNRECOVERABLE_ERROR, _, _)); change_processor_->set_sync_model_has_user_created_nodes_success(false); EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state()); @@ -201,7 +201,7 @@ TEST_F(SyncUIDataTypeControllerTest, OnSingleDatatypeUnrecoverableError) { WillOnce(InvokeWithoutArgs(preference_dtc_.get(), &UIDataTypeController::Stop)); SetStopExpectations(); - EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _)); + EXPECT_CALL(start_callback_, Run(DataTypeController::OK, _, _)); EXPECT_EQ(DataTypeController::NOT_RUNNING, preference_dtc_->state()); EXPECT_FALSE(syncable_service_.syncing()); diff --git a/sync/api/sync_merge_result.cc b/sync/api/sync_merge_result.cc new file mode 100644 index 0000000..c2253b3 --- /dev/null +++ b/sync/api/sync_merge_result.cc @@ -0,0 +1,77 @@ +// 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/api/sync_merge_result.h" + +namespace syncer { + +SyncMergeResult::SyncMergeResult(ModelType type) + : model_type_(type), + num_items_before_association_(0), + num_items_after_association_(0), + num_items_added_(0), + num_items_deleted_(0), + num_items_modified_(0) { +} + +SyncMergeResult::~SyncMergeResult() { +} + +// Setters. +void SyncMergeResult::set_error(SyncError error) { + DCHECK_EQ(model_type_, error.type()); + error_ = error; +} + +void SyncMergeResult::set_num_items_before_association( + int num_items_before_association) { + num_items_before_association_ = num_items_before_association; +} + +void SyncMergeResult::set_num_items_after_association( + int num_items_after_association) { + num_items_after_association_ = num_items_after_association; +} + +void SyncMergeResult::set_num_items_added(int num_items_added) { + num_items_added_ = num_items_added; +} + +void SyncMergeResult::set_num_items_deleted(int num_items_deleted) { + num_items_deleted_ = num_items_deleted; +} + +void SyncMergeResult::set_num_items_modified(int num_items_modified) { + num_items_modified_ = num_items_modified; +} + +ModelType SyncMergeResult::model_type() const { + return model_type_; +} + +SyncError SyncMergeResult::error() const { + return error_; +} + +int SyncMergeResult::num_items_before_association() const { + return num_items_before_association_; +} + +int SyncMergeResult::num_items_after_association() const { + return num_items_after_association_; +} + +int SyncMergeResult::num_items_added() const { + return num_items_added_; +} + +int SyncMergeResult::num_items_deleted() const { + return num_items_deleted_; +} + +int SyncMergeResult::num_items_modified() const { + return num_items_modified_; +} + +} // namespace syncer diff --git a/sync/api/sync_merge_result.h b/sync/api/sync_merge_result.h new file mode 100644 index 0000000..41accfd --- /dev/null +++ b/sync/api/sync_merge_result.h @@ -0,0 +1,73 @@ +// 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. + +#ifndef SYNC_API_SYNC_MERGE_RESULT_H_ +#define SYNC_API_SYNC_MERGE_RESULT_H_ + +#include "sync/api/sync_error.h" +#include "sync/internal_api/public/base/model_type.h" + +namespace syncer { + +// A model-type-specific view of a sync merge. This class encapsulates the +// state before and after the merge as well as the deltas and any error that +// occurred. +// Note: This class only tracks one side of the merge. In other words, if built +// by the local SyncableService, all values correspond to the local state before +// and after merging, and the delta's applied to that state. Sync's change +// processor will create a separate merge result. +class SyncMergeResult { + public: + // Initialize an empty merge result for model type |type|. + explicit SyncMergeResult(ModelType type); + ~SyncMergeResult(); + + // Default copy and assign welcome. + + // Setters. + // Note: |error.IsSet()| must be true, and |error.type()| must match + // model_type_ + void set_error(SyncError error); + void set_num_items_before_association(int num_items_before_association); + void set_num_items_after_association(int num_items_after_association); + void set_num_items_added(int num_items_added); + void set_num_items_deleted(int num_items_deleted); + void set_num_items_modified(int num_items_modified); + + // Getters. + ModelType model_type() const; + SyncError error() const; + int num_items_before_association() const; + int num_items_after_association() const; + int num_items_added() const; + int num_items_deleted() const; + int num_items_modified() const; + + private: + // Make |this| into a copy of |other|. + void CopyFrom(const SyncMergeResult& other); + + // The datatype that was associated. + ModelType model_type_; + + // The error encountered during association. Unset if no error was + // encountered. + SyncError error_; + + // The state of the world before association. + int num_items_before_association_; + + // The state of the world after association. + int num_items_after_association_; + + // The changes that took place during association. In a correctly working + // system these should be the deltas between before and after. + int num_items_added_; + int num_items_deleted_; + int num_items_modified_; +}; + +} // namespace syncer + +#endif // SYNC_API_SYNC_MERGE_RESULT_H_ diff --git a/sync/api/sync_merge_result_unittest.cc b/sync/api/sync_merge_result_unittest.cc new file mode 100644 index 0000000..dddfa4c --- /dev/null +++ b/sync/api/sync_merge_result_unittest.cc @@ -0,0 +1,77 @@ +// 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/api/sync_merge_result.h" + +#include "base/location.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace syncer { + +namespace { + +typedef testing::Test SyncMergeResultTest; + +TEST_F(SyncMergeResultTest, Unset) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_FALSE(merge_result.error().IsSet()); + EXPECT_EQ(0, merge_result.num_items_before_association()); + EXPECT_EQ(0, merge_result.num_items_after_association()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(0, merge_result.num_items_modified()); +} + +TEST_F(SyncMergeResultTest, SetError) { + SyncError error(FROM_HERE, "message", BOOKMARKS); + SyncMergeResult merge_result(BOOKMARKS); + + merge_result.set_error(error); + EXPECT_TRUE(merge_result.error().IsSet()); + EXPECT_EQ(BOOKMARKS, merge_result.model_type()); +} + +TEST_F(SyncMergeResultTest, SetNumItemsBeforeAssociation) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_EQ(0, merge_result.num_items_before_association()); + + merge_result.set_num_items_before_association(10); + EXPECT_EQ(10, merge_result.num_items_before_association()); +} + +TEST_F(SyncMergeResultTest, SetNumItemsAfterAssociation) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_EQ(0, merge_result.num_items_after_association()); + + merge_result.set_num_items_after_association(10); + EXPECT_EQ(10, merge_result.num_items_after_association()); +} + +TEST_F(SyncMergeResultTest, SetNumItemsAdded) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_EQ(0, merge_result.num_items_added()); + + merge_result.set_num_items_added(10); + EXPECT_EQ(10, merge_result.num_items_added()); +} + +TEST_F(SyncMergeResultTest, SetNumItemsDeleted) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_EQ(0, merge_result.num_items_deleted()); + + merge_result.set_num_items_deleted(10); + EXPECT_EQ(10, merge_result.num_items_deleted()); +} + +TEST_F(SyncMergeResultTest, SetNumItemsModified) { + SyncMergeResult merge_result(BOOKMARKS); + EXPECT_EQ(0, merge_result.num_items_modified()); + + merge_result.set_num_items_modified(10); + EXPECT_EQ(10, merge_result.num_items_modified()); +} + +} // namespace + +} // namespace syncer diff --git a/sync/sync.gyp b/sync/sync.gyp index 6172b84..758ef4c 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -415,6 +415,8 @@ 'api/sync_error.cc', 'api/sync_error_factory.h', 'api/sync_error_factory.cc', + 'api/sync_merge_result.h', + 'api/sync_merge_result.cc', 'api/time.h', ], }, @@ -612,6 +614,7 @@ 'internal_api/public/base/ordinal_unittest.cc', 'internal_api/public/engine/model_safe_worker_unittest.cc', 'internal_api/public/util/immutable_unittest.cc', + 'internal_api/public/util/weak_handle_unittest.cc', 'engine/apply_control_data_updates_unittest.cc', 'engine/apply_updates_and_resolve_conflicts_command_unittest.cc', 'engine/backoff_delay_provider_unittest.cc', @@ -647,7 +650,6 @@ 'util/get_session_name_unittest.cc', 'util/nigori_unittest.cc', 'util/protobuf_unittest.cc', - 'internal_api/public/util/weak_handle_unittest.cc', ], 'conditions': [ ['OS == "ios" and coverage != 0', { @@ -832,6 +834,7 @@ 'sources': [ 'api/sync_change_unittest.cc', 'api/sync_error_unittest.cc', + 'api/sync_merge_result_unittest.cc', ], }, }, |