Propagate sync activation request to bridges

This struct includes authenticated_account_id, which several bridges are
interested in. This is safer and simpler to use compared to reading
from side channels, and prevents a runtime dependency cycle across some
factories.

TBR=estade@chromium.org,rockot@chromium.org

Bug: 850428,834042,854446
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib17a4503ff9d284e1a5f4e222e7eeb95b08a024f
Reviewed-on: https://chromium-review.googlesource.com/1111716
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: vitaliii <vitaliii@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570037}

chrome/browser/consent_auditor/consent_auditor_factory.cc

@@ -7,7 +7,6 @@
 #include "base/bind_helpers.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/user_event_service_factory.h"
 #include "chrome/common/channel_info.h"
 #include "components/browser_sync/profile_sync_service.h"
@@ -44,10 +43,6 @@ ConsentAuditorFactory::ConsentAuditorFactory()
           "ConsentAuditor",
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(browser_sync::UserEventServiceFactory::GetInstance());
-  // TODO(crbug.com/850428): This is missing
-  // DependsOn(ProfileSyncServiceFactory::GetInstance()), which we can't
-  // simply add because ProfileSyncServiceFactory itself depends on this
-  // factory.
 }
 
 ConsentAuditorFactory::~ConsentAuditorFactory() {}
@@ -66,16 +61,8 @@ KeyedService* ConsentAuditorFactory::BuildServiceInstanceFor(
             syncer::USER_CONSENTS,
             base::BindRepeating(&syncer::ReportUnrecoverableError,
                                 chrome::GetChannel()));
-    syncer::SyncService* sync_service =
-        ProfileSyncServiceFactory::GetForProfile(profile);
     bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>(
-        std::move(store_factory), std::move(change_processor),
-        /*authenticated_account_id_callback=*/
-        base::BindRepeating(
-            [](syncer::SyncService* sync_service) {
-              return sync_service->GetAuthenticatedAccountInfo().account_id;
-            },
-            base::Unretained(sync_service)));
+        std::move(store_factory), std::move(change_processor));
   }
   // TODO(vitaliii): Don't create UserEventService when it won't be used.
   return new consent_auditor::ConsentAuditor(

chrome/browser/extensions/api/sessions/sessions_apitest.cc

@@ -32,7 +32,7 @@
 #include "components/sync/base/hash_util.h"
 #include "components/sync/device_info/local_device_info_provider_mock.h"
 #include "components/sync/driver/sync_api_component_factory_mock.h"
-#include "components/sync/engine/data_type_activation_request.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/test/engine/mock_model_type_worker.h"
 #include "components/sync_sessions/session_store.h"
 #include "extensions/browser/api_test_utils.h"

chrome/browser/sync/user_event_service_factory.cc

@@ -67,7 +67,7 @@ KeyedService* UserEventServiceFactory::BuildServiceInstanceFor(
                               chrome::GetChannel()));
   auto bridge = std::make_unique<syncer::UserEventSyncBridge>(
       std::move(store_factory), std::move(change_processor),
-      sync_service->GetGlobalIdMapper(), sync_service);
+      sync_service->GetGlobalIdMapper());
   return new syncer::UserEventServiceImpl(sync_service, std::move(bridge));
 }
 

components/autofill/core/browser/webdata/autocomplete_sync_bridge_unittest.cc

@@ -25,8 +25,8 @@
 #include "components/autofill/core/browser/webdata/autofill_webdata_backend.h"
 #include "components/autofill/core/browser/webdata/autofill_webdata_backend_impl.h"
 #include "components/sync/base/hash_util.h"
-#include "components/sync/engine/data_type_activation_request.h"
 #include "components/sync/model/data_batch.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "components/sync/model/model_error.h"

components/consent_auditor/consent_sync_bridge_impl.cc

@@ -16,6 +16,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/mutable_data_batch.h"
@@ -64,12 +65,8 @@ std::unique_ptr<EntityData> CopyToEntityData(
 
 ConsentSyncBridgeImpl::ConsentSyncBridgeImpl(
     OnceModelTypeStoreFactory store_factory,
-    std::unique_ptr<ModelTypeChangeProcessor> change_processor,
-    base::RepeatingCallback<std::string()> authenticated_account_id_callback)
-    : ModelTypeSyncBridge(std::move(change_processor)),
-      authenticated_account_id_callback_(authenticated_account_id_callback),
-      is_sync_starting_or_started_(false) {
-  DCHECK(authenticated_account_id_callback_);
+    std::unique_ptr<ModelTypeChangeProcessor> change_processor)
+    : ModelTypeSyncBridge(std::move(change_processor)) {
   std::move(store_factory)
       .Run(USER_CONSENTS, base::BindOnce(&ConsentSyncBridgeImpl::OnStoreCreated,
                                          base::AsWeakPtr(this)));
@@ -130,13 +127,13 @@ std::string ConsentSyncBridgeImpl::GetStorageKey(
   return GetStorageKeyFromSpecifics(entity_data.specifics.user_consent());
 }
 
-void ConsentSyncBridgeImpl::OnSyncStarting() {
-#if !defined(OS_IOS)  // https://crbug.com/834042
-  DCHECK(!authenticated_account_id_callback_.Run().empty());
-#endif  // !defined(OS_IOS)
-  DCHECK(!is_sync_starting_or_started_);
+void ConsentSyncBridgeImpl::OnSyncStarting(
+    const DataTypeActivationRequest& request) {
+  DCHECK(!request.authenticated_account_id.empty());
+  DCHECK(syncing_account_id_.empty());
+
+  syncing_account_id_ = request.authenticated_account_id;
 
-  is_sync_starting_or_started_ = true;
   if (store_ && change_processor()->IsTrackingMetadata()) {
     ReadAllDataAndResubmit();
   }
@@ -148,7 +145,7 @@ ConsentSyncBridgeImpl::ApplyStopSyncChanges(
   // Sync can only be stopped after initialization.
   DCHECK(deferred_consents_while_initializing_.empty());
 
-  is_sync_starting_or_started_ = false;
+  syncing_account_id_.clear();
 
   if (delete_metadata_change_list) {
     // Preserve all consents in the store, but delete their metadata, because it
@@ -169,7 +166,7 @@ ConsentSyncBridgeImpl::ApplyStopSyncChanges(
 }
 
 void ConsentSyncBridgeImpl::ReadAllDataAndResubmit() {
-  DCHECK(is_sync_starting_or_started_);
+  DCHECK(!syncing_account_id_.empty());
   DCHECK(change_processor()->IsTrackingMetadata());
   DCHECK(store_);
   store_->ReadAllData(base::BindOnce(
@@ -179,7 +176,7 @@ void ConsentSyncBridgeImpl::ReadAllDataAndResubmit() {
 void ConsentSyncBridgeImpl::OnReadAllDataToResubmit(
     const base::Optional<ModelError>& error,
     std::unique_ptr<RecordList> data_records) {
-  if (!is_sync_starting_or_started_) {
+  if (syncing_account_id_.empty()) {
     // Meanwhile the sync has been disabled. We will try next time.
     return;
   }
@@ -193,7 +190,7 @@ void ConsentSyncBridgeImpl::OnReadAllDataToResubmit(
   for (const Record& r : *data_records) {
     auto specifics = std::make_unique<UserConsentSpecifics>();
     if (specifics->ParseFromString(r.value) &&
-        specifics->account_id() == authenticated_account_id_callback_.Run()) {
+        specifics->account_id() == syncing_account_id_) {
       RecordConsentImpl(std::move(specifics));
     }
   }
@@ -201,6 +198,8 @@ void ConsentSyncBridgeImpl::OnReadAllDataToResubmit(
 
 void ConsentSyncBridgeImpl::RecordConsent(
     std::unique_ptr<UserConsentSpecifics> specifics) {
+  // TODO(vitaliii): Sanity-check specifics->account_id() against
+  // syncing_account_id_, maybe DCHECK.
   DCHECK(!specifics->account_id().empty());
   if (change_processor()->IsTrackingMetadata()) {
     RecordConsentImpl(std::move(specifics));
@@ -262,7 +261,7 @@ void ConsentSyncBridgeImpl::OnReadAllMetadata(
   } else {
     change_processor()->ModelReadyToSync(std::move(metadata_batch));
     DCHECK(change_processor()->IsTrackingMetadata());
-    if (is_sync_starting_or_started_) {
+    if (!syncing_account_id_.empty()) {
       ReadAllDataAndResubmit();
     }
     ProcessQueuedEvents();

components/consent_auditor/consent_sync_bridge_impl.h

@@ -11,7 +11,6 @@
 #include <string>
 #include <vector>
 
-#include "base/callback.h"
 #include "base/macros.h"
 #include "base/optional.h"
 #include "components/consent_auditor/consent_sync_bridge.h"
@@ -26,12 +25,11 @@ class ConsentSyncBridgeImpl : public ConsentSyncBridge,
  public:
   ConsentSyncBridgeImpl(
       OnceModelTypeStoreFactory store_factory,
-      std::unique_ptr<ModelTypeChangeProcessor> change_processor,
-      base::RepeatingCallback<std::string()> authenticated_account_id_callback);
+      std::unique_ptr<ModelTypeChangeProcessor> change_processor);
   ~ConsentSyncBridgeImpl() override;
 
   // ModelTypeSyncBridge implementation.
-  void OnSyncStarting() override;
+  void OnSyncStarting(const DataTypeActivationRequest& request) override;
   std::unique_ptr<MetadataChangeList> CreateMetadataChangeList() override;
   base::Optional<ModelError> MergeSyncData(
       std::unique_ptr<MetadataChangeList> metadata_change_list,
@@ -89,9 +87,8 @@ class ConsentSyncBridgeImpl : public ConsentSyncBridge,
   std::vector<std::unique_ptr<sync_pb::UserConsentSpecifics>>
       deferred_consents_while_initializing_;
 
-  base::RepeatingCallback<std::string()> authenticated_account_id_callback_;
-
-  bool is_sync_starting_or_started_;
+  // Empty if sync not running.
+  std::string syncing_account_id_;
 
   DISALLOW_COPY_AND_ASSIGN(ConsentSyncBridgeImpl);
 };

components/consent_auditor/consent_sync_bridge_impl_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
 #include "components/sync/model/data_batch.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "components/sync/protocol/sync.pb.h"
@@ -67,13 +68,19 @@ std::unique_ptr<UserConsentSpecifics> SpecificsUniquePtr(
       CreateSpecifics(client_consent_time_usec));
 }
 
+DataTypeActivationRequest CreateActivationRequest(
+    const std::string& account_id) {
+  DataTypeActivationRequest request;
+  request.authenticated_account_id = account_id;
+  return request;
+}
+
 class ConsentSyncBridgeImplTest : public testing::Test {
  protected:
   ConsentSyncBridgeImplTest() {
     bridge_ = std::make_unique<ConsentSyncBridgeImpl>(
         ModelTypeStoreTestUtil::FactoryForInMemoryStoreForTest(),
-        mock_processor_.CreateForwardingProcessor(),
-        GetAuthenticatedAccountIdCallback());
+        mock_processor_.CreateForwardingProcessor());
     ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
   }
 
@@ -83,21 +90,6 @@ class ConsentSyncBridgeImplTest : public testing::Test {
     return bridge()->GetStorageKey(entity_data);
   }
 
-  void SetAuthenticatedAccountId(const std::string& new_id) {
-    authenticated_account_id_ = new_id;
-  }
-
-  std::string GetAuthenticatedAccountId() const {
-    return authenticated_account_id_;
-  }
-
-  base::RepeatingCallback<std::string()> GetAuthenticatedAccountIdCallback()
-      const {
-    return base::BindRepeating(
-        &ConsentSyncBridgeImplTest::GetAuthenticatedAccountId,
-        base::Unretained(this));
-  }
-
   ConsentSyncBridgeImpl* bridge() { return bridge_.get(); }
   MockModelTypeChangeProcessor* processor() { return &mock_processor_; }
 
@@ -154,8 +146,6 @@ class ConsentSyncBridgeImplTest : public testing::Test {
   std::unique_ptr<ConsentSyncBridgeImpl> bridge_;
   testing::NiceMock<MockModelTypeChangeProcessor> mock_processor_;
   base::MessageLoop message_loop_;
-
-  std::string authenticated_account_id_;
 };
 
 TEST_F(ConsentSyncBridgeImplTest, ShouldCallModelReadyToSyncOnStartup) {
@@ -272,8 +262,7 @@ TEST_F(ConsentSyncBridgeImplTest,
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(),
-      GetAuthenticatedAccountIdCallback());
+      processor()->CreateForwardingProcessor());
 
   // Record consent before the store is initialized.
   late_init_bridge.RecordConsent(
@@ -287,8 +276,7 @@ TEST_F(ConsentSyncBridgeImplTest,
            ModelTypeStoreTestUtil::CreateInMemoryStoreForTest(store_init_type));
   base::RunLoop().RunUntilIdle();
 
-  SetAuthenticatedAccountId("account_id");
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   // Record consent after initializaiton is done.
   late_init_bridge.RecordConsent(
@@ -323,12 +311,11 @@ TEST_F(ConsentSyncBridgeImplTest,
   ASSERT_THAT(GetAllData(), SizeIs(1));
 
   // Reenable sync.
-  SetAuthenticatedAccountId("account_id");
   ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
       .WillOnce(WithArg<0>(SaveArg<0>(&storage_key)));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("account_id"));
 
   // The bridge may asynchronously query the store to choose what to resubmit.
   base::RunLoop().RunUntilIdle();
@@ -356,13 +343,11 @@ TEST_F(ConsentSyncBridgeImplTest,
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(),
-      GetAuthenticatedAccountIdCallback());
+      processor()->CreateForwardingProcessor());
 
   // Sync is active, but the store is not ready yet.
-  SetAuthenticatedAccountId("account_id");
   EXPECT_CALL(*processor(), ModelReadyToSync(_)).Times(0);
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   // Initialize the store.
   std::unique_ptr<ModelTypeStore> store =
@@ -414,8 +399,7 @@ TEST_F(ConsentSyncBridgeImplTest,
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(),
-      GetAuthenticatedAccountIdCallback());
+      processor()->CreateForwardingProcessor());
 
   // Initialize the store.
   std::unique_ptr<ModelTypeStore> store =
@@ -440,8 +424,7 @@ TEST_F(ConsentSyncBridgeImplTest,
            ModelTypeStoreTestUtil::CreateInMemoryStoreForTest(store_init_type));
   base::RunLoop().RunUntilIdle();
 
-  SetAuthenticatedAccountId("account_id");
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
@@ -456,7 +439,7 @@ TEST_F(ConsentSyncBridgeImplTest,
 
 TEST_F(ConsentSyncBridgeImplTest,
        ShouldResubmitPersistedConsentOnlyIfSameAccount) {
-  SetAuthenticatedAccountId("first_account");
+  // This consent is being recorded while sync is stopped.
   UserConsentSpecifics user_consent_specifics(
       CreateSpecifics(/*client_consent_time_usec=*/2u));
   user_consent_specifics.set_account_id("first_account");
@@ -474,13 +457,11 @@ TEST_F(ConsentSyncBridgeImplTest,
               ElementsAre(Pair(_, MatchesUserConsent(user_consent_specifics))));
 
   // A new user signs in and enables sync.
-  SetAuthenticatedAccountId("second_account");
-
   // The previous account consent should not be resubmited, because the new sync
   // account is different.
   EXPECT_CALL(*processor(), Put(_, _, _)).Times(0);
   ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("second_account"));
   base::RunLoop().RunUntilIdle();
 
   EXPECT_THAT(
@@ -488,13 +469,10 @@ TEST_F(ConsentSyncBridgeImplTest,
       Eq(ModelTypeSyncBridge::StopSyncResponse::kModelStillReadyToSync));
   base::RunLoop().RunUntilIdle();
 
-  // The previous user signs in again and enables sync.
-  SetAuthenticatedAccountId("first_account");
-
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
       .WillOnce(WithArg<0>(SaveArg<0>(&storage_key)));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("first_account"));
   // The bridge may asynchronously query the store to choose what to resubmit.
   base::RunLoop().RunUntilIdle();
 

components/sync/BUILD.gn

@@ -173,8 +173,6 @@ jumbo_static_library("sync") {
     "engine/cycle/type_debug_info_observer.h",
     "engine/cycle/update_counters.cc",
     "engine/cycle/update_counters.h",
-    "engine/data_type_activation_request.cc",
-    "engine/data_type_activation_request.h",
     "engine/data_type_activation_response.cc",
     "engine/data_type_activation_response.h",
     "engine/data_type_association_stats.cc",
@@ -389,6 +387,8 @@ jumbo_static_library("sync") {
     "model/conflict_resolution.cc",
     "model/conflict_resolution.h",
     "model/data_batch.h",
+    "model/data_type_activation_request.cc",
+    "model/data_type_activation_request.h",
     "model/data_type_error_handler.h",
     "model/data_type_error_handler_impl.cc",
     "model/data_type_error_handler_impl.h",

components/sync/driver/model_type_controller.cc

@@ -15,9 +15,9 @@
 #include "components/sync/device_info/local_device_info_provider.h"
 #include "components/sync/driver/sync_client.h"
 #include "components/sync/driver/sync_service.h"
-#include "components/sync/engine/data_type_activation_request.h"
 #include "components/sync/engine/data_type_activation_response.h"
 #include "components/sync/engine/model_type_configurer.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/data_type_error_handler_impl.h"
 #include "components/sync/model/model_type_change_processor.h"
 #include "components/sync/model/sync_merge_result.h"

components/sync/engine/data_type_activation_request.cc → components/sync/model/data_type_activation_request.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/sync/engine/data_type_activation_request.h"
+#include "components/sync/model/data_type_activation_request.h"
 
 namespace syncer {
 
@@ -16,4 +16,10 @@ DataTypeActivationRequest::DataTypeActivationRequest(
 
 DataTypeActivationRequest::~DataTypeActivationRequest() = default;
 
+DataTypeActivationRequest& DataTypeActivationRequest::operator=(
+    const DataTypeActivationRequest& request) = default;
+
+DataTypeActivationRequest& DataTypeActivationRequest::operator=(
+    DataTypeActivationRequest&& request) = default;
+
 }  // namespace syncer

components/sync/engine/data_type_activation_request.h → components/sync/model/data_type_activation_request.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_SYNC_ENGINE_DATA_TYPE_ACTIVATION_REQUEST_H_
-#define COMPONENTS_SYNC_ENGINE_DATA_TYPE_ACTIVATION_REQUEST_H_
+#ifndef COMPONENTS_SYNC_MODEL_DATA_TYPE_ACTIVATION_REQUEST_H_
+#define COMPONENTS_SYNC_MODEL_DATA_TYPE_ACTIVATION_REQUEST_H_
 
 #include <string>
 
@@ -19,6 +19,10 @@ struct DataTypeActivationRequest {
   DataTypeActivationRequest(DataTypeActivationRequest&& request);
   ~DataTypeActivationRequest();
 
+  DataTypeActivationRequest& operator=(
+      const DataTypeActivationRequest& request);
+  DataTypeActivationRequest& operator=(DataTypeActivationRequest&& request);
+
   ModelErrorHandler error_handler;
   std::string authenticated_account_id;
   std::string cache_guid;
@@ -26,4 +30,4 @@ struct DataTypeActivationRequest {
 
 }  // namespace syncer
 
-#endif  // COMPONENTS_SYNC_ENGINE_DATA_TYPE_ACTIVATION_REQUEST_H_
+#endif  // COMPONENTS_SYNC_MODEL_DATA_TYPE_ACTIVATION_REQUEST_H_

components/sync/model/model_type_sync_bridge.cc

@@ -21,7 +21,8 @@ ModelTypeSyncBridge::ModelTypeSyncBridge(
 
 ModelTypeSyncBridge::~ModelTypeSyncBridge() {}
 
-void ModelTypeSyncBridge::OnSyncStarting() {}
+void ModelTypeSyncBridge::OnSyncStarting(
+    const DataTypeActivationRequest& request) {}
 
 bool ModelTypeSyncBridge::SupportsGetStorageKey() const {
   return true;

components/sync/model/model_type_sync_bridge.h

@@ -19,6 +19,7 @@ namespace syncer {
 
 class ConflictResolution;
 class DataBatch;
+struct DataTypeActivationRequest;
 struct EntityData;
 class MetadataChangeList;
 class ModelError;
@@ -49,7 +50,7 @@ class ModelTypeSyncBridge : public base::SupportsWeakPtr<ModelTypeSyncBridge> {
 
   // Called by the processor as a notification that sync has been started by the
   // ModelTypeController.
-  virtual void OnSyncStarting();
+  virtual void OnSyncStarting(const DataTypeActivationRequest& request);
 
   // Creates an object used to communicate changes in the sync metadata to the
   // model type store.

components/sync/model_impl/client_tag_based_model_type_processor.cc

@@ -16,7 +16,6 @@
 #include "components/sync/base/hash_util.h"
 #include "components/sync/base/time.h"
 #include "components/sync/engine/commit_queue.h"
-#include "components/sync/engine/data_type_activation_request.h"
 #include "components/sync/engine/data_type_activation_response.h"
 #include "components/sync/engine/model_type_processor_proxy.h"
 #include "components/sync/model_impl/processor_entity_tracker.h"
@@ -80,12 +79,13 @@ void ClientTagBasedModelTypeProcessor::OnSyncStarting(
   DCHECK(start_callback);
   DVLOG(1) << "Sync is starting for " << ModelTypeToString(type_);
 
+  start_callback_ = std::move(start_callback);
+  activation_request_ = request;
+
   // Notify the bridge sync is starting before calling the |start_callback_|
   // which in turn creates the worker.
-  bridge_->OnSyncStarting();
+  bridge_->OnSyncStarting(request);
 
-  error_handler_ = request.error_handler;
-  start_callback_ = std::move(start_callback);
   ConnectIfReady();
 }
 
@@ -146,7 +146,7 @@ void ClientTagBasedModelTypeProcessor::ConnectIfReady() {
     return;
 
   if (model_error_) {
-    error_handler_.Run(model_error_.value());
+    activation_request_.error_handler.Run(model_error_.value());
   } else {
     auto activation_response = std::make_unique<DataTypeActivationResponse>();
     activation_response->model_type_state = model_type_state_;
@@ -241,10 +241,10 @@ void ClientTagBasedModelTypeProcessor::ReportError(const ModelError& error) {
   if (start_callback_) {
     // Tell sync about the error instead of connecting.
     ConnectIfReady();
-  } else if (error_handler_) {
+  } else if (activation_request_.error_handler) {
     // Connecting was already initiated; just tell sync about the error instead
     // of going through ConnectIfReady().
-    error_handler_.Run(error);
+    activation_request_.error_handler.Run(error);
   }
 }
 
@@ -1054,7 +1054,7 @@ void ClientTagBasedModelTypeProcessor::ResetState(
   worker_.reset();
   model_error_.reset();
   start_callback_ = StartCallback();
-  error_handler_ = ModelErrorHandler();
+  activation_request_ = DataTypeActivationRequest();
   cached_gc_directive_version_ = 0;
   cached_gc_directive_aged_out_day_ = base::Time::FromDoubleT(0);
 

components/sync/model_impl/client_tag_based_model_type_processor.h

@@ -21,6 +21,7 @@
 #include "components/sync/engine/non_blocking_sync_common.h"
 #include "components/sync/model/conflict_resolution.h"
 #include "components/sync/model/data_batch.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/metadata_change_list.h"
 #include "components/sync/model/model_error.h"
@@ -253,9 +254,8 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
   // Stores the start callback in between OnSyncStarting() and ReadyToConnect().
   StartCallback start_callback_;
 
-  // The callback used for informing sync of errors; will be non-null after
-  // OnSyncStarting has been called.
-  ModelErrorHandler error_handler_;
+  // The request context passed in as part of OnSyncStarting().
+  DataTypeActivationRequest activation_request_;
 
   // Reference to the CommitQueue.
   //

components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc

@@ -16,8 +16,8 @@
 #include "base/run_loop.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/time.h"
-#include "components/sync/engine/data_type_activation_request.h"
 #include "components/sync/engine/data_type_activation_response.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/fake_model_type_sync_bridge.h"
 #include "components/sync/test/engine/mock_model_type_worker.h"
 #include "testing/gtest/include/gtest/gtest.h"

components/sync/user_events/user_event_service_impl_unittest.cc

@@ -112,7 +112,7 @@ class UserEventServiceImplTest : public testing::Test {
   std::unique_ptr<UserEventSyncBridge> MakeBridge() {
     return std::make_unique<UserEventSyncBridge>(
         ModelTypeStoreTestUtil::FactoryForInMemoryStoreForTest(),
-        mock_processor_.CreateForwardingProcessor(), &mapper_, &sync_service_);
+        mock_processor_.CreateForwardingProcessor(), &mapper_);
   }
 
   TestSyncService* sync_service() { return &sync_service_; }

components/sync/user_events/user_event_sync_bridge.cc

@@ -17,6 +17,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "components/signin/core/browser/account_info.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/mutable_data_batch.h"
@@ -72,14 +73,10 @@ std::unique_ptr<EntityData> CopyToEntityData(
 UserEventSyncBridge::UserEventSyncBridge(
     OnceModelTypeStoreFactory store_factory,
     std::unique_ptr<ModelTypeChangeProcessor> change_processor,
-    GlobalIdMapper* global_id_mapper,
-    SyncService* sync_service)
+    GlobalIdMapper* global_id_mapper)
     : ModelTypeSyncBridge(std::move(change_processor)),
-      global_id_mapper_(global_id_mapper),
-      sync_service_(sync_service),
-      is_sync_starting_or_started_(false) {
+      global_id_mapper_(global_id_mapper) {
   DCHECK(global_id_mapper_);
-  DCHECK(sync_service_);
   std::move(store_factory)
       .Run(USER_EVENTS, base::BindOnce(&UserEventSyncBridge::OnStoreCreated,
                                        base::AsWeakPtr(this)));
@@ -154,13 +151,13 @@ std::string UserEventSyncBridge::GetStorageKey(const EntityData& entity_data) {
   return GetStorageKeyFromSpecifics(entity_data.specifics.user_event());
 }
 
-void UserEventSyncBridge::OnSyncStarting() {
-#if !defined(OS_IOS)  // https://crbug.com/834042
-  DCHECK(!GetAuthenticatedAccountId().empty());
-#endif  // !defined(OS_IOS)
-  DCHECK(!is_sync_starting_or_started_);
+void UserEventSyncBridge::OnSyncStarting(
+    const DataTypeActivationRequest& request) {
+  DCHECK(!request.authenticated_account_id.empty());
+  DCHECK(syncing_account_id_.empty());
+
+  syncing_account_id_ = request.authenticated_account_id;
 
-  is_sync_starting_or_started_ = true;
   if (store_ && change_processor()->IsTrackingMetadata()) {
     ReadAllDataAndResubmit();
   }
@@ -171,7 +168,7 @@ ModelTypeSyncBridge::StopSyncResponse UserEventSyncBridge::ApplyStopSyncChanges(
   // Sync can only be stopped after initialization.
   DCHECK(deferred_user_events_while_initializing_.empty());
 
-  is_sync_starting_or_started_ = false;
+  syncing_account_id_.clear();
 
   if (delete_metadata_change_list) {
     // Delete everything except user consents. With DICE the signout may happen
@@ -215,7 +212,7 @@ void UserEventSyncBridge::OnReadAllDataToDelete(
 }
 
 void UserEventSyncBridge::ReadAllDataAndResubmit() {
-  DCHECK(is_sync_starting_or_started_);
+  DCHECK(!syncing_account_id_.empty());
   DCHECK(change_processor()->IsTrackingMetadata());
   DCHECK(store_);
   store_->ReadAllData(base::BindOnce(
@@ -225,7 +222,7 @@ void UserEventSyncBridge::ReadAllDataAndResubmit() {
 void UserEventSyncBridge::OnReadAllDataToResubmit(
     const base::Optional<ModelError>& error,
     std::unique_ptr<RecordList> data_records) {
-  if (!is_sync_starting_or_started_) {
+  if (syncing_account_id_.empty()) {
     // Meanwhile the sync has been disabled. We will try next time.
     return;
   }
@@ -241,7 +238,7 @@ void UserEventSyncBridge::OnReadAllDataToResubmit(
     if (specifics->ParseFromString(r.value) &&
         specifics->event_case() ==
             UserEventSpecifics::EventCase::kUserConsent &&
-        specifics->user_consent().account_id() == GetAuthenticatedAccountId()) {
+        specifics->user_consent().account_id() == syncing_account_id_) {
       RecordUserEventImpl(std::move(specifics));
     }
   }
@@ -249,6 +246,8 @@ void UserEventSyncBridge::OnReadAllDataToResubmit(
 
 void UserEventSyncBridge::RecordUserEvent(
     std::unique_ptr<UserEventSpecifics> specifics) {
+  // TODO(vitaliii): Sanity-check specifics->user_consent().account_id() against
+  // syncing_account_id_, maybe DCHECK.
   DCHECK(!specifics->has_user_consent() ||
          !specifics->user_consent().account_id().empty());
   if (change_processor()->IsTrackingMetadata()) {
@@ -324,7 +323,7 @@ void UserEventSyncBridge::OnReadAllMetadata(
   } else {
     change_processor()->ModelReadyToSync(std::move(metadata_batch));
     DCHECK(change_processor()->IsTrackingMetadata());
-    if (is_sync_starting_or_started_) {
+    if (!syncing_account_id_.empty()) {
       ReadAllDataAndResubmit();
     }
     ProcessQueuedEvents();
@@ -391,8 +390,4 @@ void UserEventSyncBridge::HandleGlobalIdChange(int64_t old_global_id,
   }
 }
 
-std::string UserEventSyncBridge::GetAuthenticatedAccountId() const {
-  return sync_service_->GetAuthenticatedAccountInfo().account_id;
-}
-
 }  // namespace syncer

components/sync/user_events/user_event_sync_bridge.h

@@ -14,7 +14,6 @@
 
 #include "base/macros.h"
 #include "base/optional.h"
-#include "components/sync/driver/sync_service.h"
 #include "components/sync/model/model_type_change_processor.h"
 #include "components/sync/model/model_type_store.h"
 #include "components/sync/model/model_type_sync_bridge.h"
@@ -27,12 +26,11 @@ class UserEventSyncBridge : public ModelTypeSyncBridge {
   UserEventSyncBridge(
       OnceModelTypeStoreFactory store_factory,
       std::unique_ptr<ModelTypeChangeProcessor> change_processor,
-      GlobalIdMapper* global_id_mapper,
-      SyncService* sync_service);
+      GlobalIdMapper* global_id_mapper);
   ~UserEventSyncBridge() override;
 
   // ModelTypeSyncBridge implementation.
-  void OnSyncStarting() override;
+  void OnSyncStarting(const DataTypeActivationRequest& request) override;
   std::unique_ptr<MetadataChangeList> CreateMetadataChangeList() override;
   base::Optional<ModelError> MergeSyncData(
       std::unique_ptr<MetadataChangeList> metadata_change_list,
@@ -82,8 +80,6 @@ class UserEventSyncBridge : public ModelTypeSyncBridge {
 
   void HandleGlobalIdChange(int64_t old_global_id, int64_t new_global_id);
 
-  std::string GetAuthenticatedAccountId() const;
-
   // Persistent storage for in flight events. Should remain quite small, as we
   // delete upon commit confirmation.
   std::unique_ptr<ModelTypeStore> store_;
@@ -98,9 +94,9 @@ class UserEventSyncBridge : public ModelTypeSyncBridge {
       in_flight_nav_linked_events_;
 
   GlobalIdMapper* global_id_mapper_;
-  SyncService* sync_service_;
 
-  bool is_sync_starting_or_started_;
+  // Empty if sync not running.
+  std::string syncing_account_id_;
 
   DISALLOW_COPY_AND_ASSIGN(UserEventSyncBridge);
 };

components/sync/user_events/user_event_sync_bridge_unittest.cc

@@ -13,8 +13,8 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/test/bind_test_util.h"
-#include "components/sync/driver/fake_sync_service.h"
 #include "components/sync/model/data_batch.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
 #include "components/sync/model/model_type_store_test_util.h"
 #include "components/sync/protocol/sync.pb.h"
@@ -69,6 +69,13 @@ UserEventSpecifics CreateSpecifics(int64_t event_time_usec,
   return specifics;
 }
 
+DataTypeActivationRequest CreateActivationRequest(
+    const std::string& account_id) {
+  DataTypeActivationRequest request;
+  request.authenticated_account_id = account_id;
+  return request;
+}
+
 std::unique_ptr<UserEventSpecifics> SpecificsUniquePtr(int64_t event_time_usec,
                                                        int64_t navigation_id,
                                                        uint64_t session_id) {
@@ -102,8 +109,7 @@ class UserEventSyncBridgeTest : public testing::Test {
   UserEventSyncBridgeTest() {
     bridge_ = std::make_unique<UserEventSyncBridge>(
         ModelTypeStoreTestUtil::FactoryForInMemoryStoreForTest(),
-        mock_processor_.CreateForwardingProcessor(), &test_global_id_mapper_,
-        &fake_sync_service_);
+        mock_processor_.CreateForwardingProcessor(), &test_global_id_mapper_);
     ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
   }
 
@@ -115,7 +121,6 @@ class UserEventSyncBridgeTest : public testing::Test {
 
   UserEventSyncBridge* bridge() { return bridge_.get(); }
   MockModelTypeChangeProcessor* processor() { return &mock_processor_; }
-  FakeSyncService* sync_service() { return &fake_sync_service_; }
   TestGlobalIdMapper* mapper() { return &test_global_id_mapper_; }
 
   std::map<std::string, sync_pb::EntitySpecifics> GetAllData() {
@@ -171,7 +176,6 @@ class UserEventSyncBridgeTest : public testing::Test {
   std::unique_ptr<UserEventSyncBridge> bridge_;
   testing::NiceMock<MockModelTypeChangeProcessor> mock_processor_;
   TestGlobalIdMapper test_global_id_mapper_;
-  FakeSyncService fake_sync_service_;
   base::MessageLoop message_loop_;
 };
 
@@ -374,7 +378,7 @@ TEST_F(UserEventSyncBridgeTest, RecordWithLateInitializedStore) {
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(), mapper(), sync_service());
+      processor()->CreateForwardingProcessor(), mapper());
 
   // Record events before the store is created. Only the consent will be
   // buffered, the other event is dropped.
@@ -391,10 +395,7 @@ TEST_F(UserEventSyncBridgeTest, RecordWithLateInitializedStore) {
            ModelTypeStoreTestUtil::CreateInMemoryStoreForTest(store_init_type));
   base::RunLoop().RunUntilIdle();
 
-  AccountInfo info;
-  info.account_id = "account_id";
-  sync_service()->SetAuthenticatedAccountInfo(info);
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   // Record events after metadata is ready.
   late_init_bridge.RecordUserEvent(
@@ -425,14 +426,11 @@ TEST_F(UserEventSyncBridgeTest,
   ASSERT_THAT(GetAllData(), SizeIs(1));
 
   // Reenable sync.
-  AccountInfo info;
-  info.account_id = "account_id";
-  sync_service()->SetAuthenticatedAccountInfo(info);
   ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
       .WillOnce(WithArg<0>(SaveArg<0>(&storage_key)));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("account_id"));
 
   // The bridge may asynchronously query the store to choose what to resubmit.
   base::RunLoop().RunUntilIdle();
@@ -457,14 +455,11 @@ TEST_F(UserEventSyncBridgeTest,
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(), mapper(), sync_service());
+      processor()->CreateForwardingProcessor(), mapper());
 
   // Sync is active, but the store is not ready yet.
-  AccountInfo info;
-  info.account_id = "account_id";
-  sync_service()->SetAuthenticatedAccountInfo(info);
   EXPECT_CALL(*processor(), ModelReadyToSync(_)).Times(0);
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   // Initialize the store.
   std::unique_ptr<ModelTypeStore> store =
@@ -513,7 +508,7 @@ TEST_F(UserEventSyncBridgeTest,
             store_init_type = type;
             store_init_callback = std::move(callback);
           }),
-      processor()->CreateForwardingProcessor(), mapper(), sync_service());
+      processor()->CreateForwardingProcessor(), mapper());
 
   // Initialize the store.
   std::unique_ptr<ModelTypeStore> store =
@@ -538,10 +533,7 @@ TEST_F(UserEventSyncBridgeTest,
            ModelTypeStoreTestUtil::CreateInMemoryStoreForTest(store_init_type));
   base::RunLoop().RunUntilIdle();
 
-  AccountInfo info;
-  info.account_id = "account_id";
-  sync_service()->SetAuthenticatedAccountInfo(info);
-  late_init_bridge.OnSyncStarting();
+  late_init_bridge.OnSyncStarting(CreateActivationRequest("account_id"));
 
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
@@ -553,9 +545,7 @@ TEST_F(UserEventSyncBridgeTest,
 }
 
 TEST_F(UserEventSyncBridgeTest, ShouldSubmitPersistedConsentOnlyIfSameAccount) {
-  AccountInfo info;
-  info.account_id = "first_account";
-  sync_service()->SetAuthenticatedAccountInfo(info);
+  // This consent is being recorded while sync is stopped.
   UserEventSpecifics user_event_specifics(CreateSpecifics(2u, 2u, 2u));
   auto* consent = user_event_specifics.mutable_user_consent();
   consent->set_account_id("first_account");
@@ -571,27 +561,21 @@ TEST_F(UserEventSyncBridgeTest, ShouldSubmitPersistedConsentOnlyIfSameAccount) {
               ElementsAre(Pair(_, MatchesUserEvent(user_event_specifics))));
 
   // A new user signs in and enables sync.
-  info.account_id = "second_account";
-  sync_service()->SetAuthenticatedAccountInfo(info);
-
   // The previous account consent should not be resubmited, because the new sync
   // account is different.
   EXPECT_CALL(*processor(), Put(_, _, _)).Times(0);
   ON_CALL(*processor(), IsTrackingMetadata()).WillByDefault(Return(true));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("second_account"));
   base::RunLoop().RunUntilIdle();
 
   bridge()->ApplyStopSyncChanges(WriteBatch::CreateMetadataChangeList());
   base::RunLoop().RunUntilIdle();
 
   // The previous user signs in again and enables sync.
-  info.account_id = "first_account";
-  sync_service()->SetAuthenticatedAccountInfo(info);
-
   std::string storage_key;
   EXPECT_CALL(*processor(), Put(_, _, _))
       .WillOnce(WithArg<0>(SaveArg<0>(&storage_key)));
-  bridge()->OnSyncStarting();
+  bridge()->OnSyncStarting(CreateActivationRequest("first_account"));
   // The bridge may asynchronously query the store to choose what to resubmit.
   base::RunLoop().RunUntilIdle();
 

components/sync_sessions/session_sync_bridge.cc

@@ -337,7 +337,8 @@ void SessionSyncBridge::OnFaviconVisited(const GURL& page_url,
   favicon_cache_.OnFaviconVisited(page_url, favicon_url);
 }
 
-void SessionSyncBridge::OnSyncStarting() {
+void SessionSyncBridge::OnSyncStarting(
+    const syncer::DataTypeActivationRequest& request) {
   DCHECK(!syncing_);
 
   session_store_factory_.Run(base::BindOnce(

components/sync_sessions/session_sync_bridge.h

@@ -60,7 +60,8 @@ class SessionSyncBridge : public AbstractSessionsSyncManager,
   syncer::ModelTypeSyncBridge* GetModelTypeSyncBridge() override;
 
   // ModelTypeSyncBridge implementation.
-  void OnSyncStarting() override;
+  void OnSyncStarting(
+      const syncer::DataTypeActivationRequest& request) override;
   std::unique_ptr<syncer::MetadataChangeList> CreateMetadataChangeList()
       override;
   base::Optional<syncer::ModelError> MergeSyncData(

components/sync_sessions/session_sync_bridge_unittest.cc

@@ -19,8 +19,8 @@
 #include "components/sync/base/hash_util.h"
 #include "components/sync/base/sync_prefs.h"
 #include "components/sync/device_info/local_device_info_provider_mock.h"
-#include "components/sync/engine/data_type_activation_request.h"
 #include "components/sync/model/data_batch.h"
+#include "components/sync/model/data_type_activation_request.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/metadata_change_list.h"
 #include "components/sync/model/mock_model_type_change_processor.h"

ios/chrome/browser/consent_auditor/consent_auditor_factory.cc

@@ -21,7 +21,6 @@
 #include "ios/chrome/browser/application_context.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/sync/ios_user_event_service_factory.h"
-#include "ios/chrome/browser/sync/profile_sync_service_factory.h"
 #include "ios/chrome/common/channel_info.h"
 #include "ios/web/public/browser_state.h"
 
@@ -49,10 +48,6 @@ ConsentAuditorFactory::ConsentAuditorFactory()
     : BrowserStateKeyedServiceFactory(
           "ConsentAuditor",
           BrowserStateDependencyManager::GetInstance()) {
-  // TODO(crbug.com/850428): This is missing
-  // DependsOn(ProfileSyncServiceFactory::GetInstance()), which we can't
-  // simply add because ProfileSyncServiceFactory itself depends on this
-  // factory.
   DependsOn(IOSUserEventServiceFactory::GetInstance());
 }
 
@@ -77,17 +72,8 @@ std::unique_ptr<KeyedService> ConsentAuditorFactory::BuildServiceInstanceFor(
             syncer::USER_CONSENTS,
             base::BindRepeating(&syncer::ReportUnrecoverableError,
                                 ::GetChannel()));
-    syncer::SyncService* sync_service =
-        ProfileSyncServiceFactory::GetForBrowserState(
-            ios::ChromeBrowserState::FromBrowserState(browser_state));
     bridge = std::make_unique<syncer::ConsentSyncBridgeImpl>(
-        std::move(store_factory), std::move(change_processor),
-        /*authenticated_account_id_callback=*/
-        base::BindRepeating(
-            [](syncer::SyncService* sync_service) {
-              return sync_service->GetAuthenticatedAccountInfo().account_id;
-            },
-            sync_service));
+        std::move(store_factory), std::move(change_processor));
   }
 
   return std::make_unique<consent_auditor::ConsentAuditor>(

ios/chrome/browser/sync/ios_user_event_service_factory.cc

@@ -60,7 +60,7 @@ IOSUserEventServiceFactory::BuildServiceInstanceFor(
       std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
           syncer::USER_EVENTS, /*dump_stack=*/base::BindRepeating(
               &syncer::ReportUnrecoverableError, ::GetChannel())),
-      sync_service->GetGlobalIdMapper(), sync_service);
+      sync_service->GetGlobalIdMapper());
   return std::make_unique<syncer::UserEventServiceImpl>(sync_service,
                                                         std::move(bridge));
 }