Ensure raw_ptrs are initialized in components/sync/

Details in https://crbug.com/1473266#c2.

Bug: 1473266
Change-Id: Idaa3d5a3adc03bb7f87d824eaa08436fd4512808
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4783447
Auto-Submit: Victor Vianna <victorvianna@google.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Jood Hajeer <jood@google.com>
Cr-Commit-Position: refs/heads/main@{#1184613}

components/sync/base/unique_position_unittest.cc

@@ -575,7 +575,7 @@ class IndexedLessThan {
   }
 
  private:
-  raw_ptr<const T> values_;
+  const raw_ptr<const T> values_;
   LessThan less_than_;
 };
 

components/sync/engine/cancelation_signal_unittest.cc

@@ -42,7 +42,7 @@ class BlockingTask : public CancelationSignal::Observer {
  private:
   base::WaitableEvent event_;
   base::Thread exec_thread_;
-  raw_ptr<CancelationSignal> cancel_signal_;
+  const raw_ptr<CancelationSignal> cancel_signal_;
   bool was_started_ = false;
 };
 

components/sync/engine/commit_processor.h

@@ -73,7 +73,7 @@ class CommitProcessor {
   const ModelTypeSet commit_types_;
 
   // A map of 'commit contributors', one for each enabled type.
-  raw_ptr<CommitContributorMap> commit_contributor_map_;
+  const raw_ptr<CommitContributorMap> commit_contributor_map_;
   GatheringPhase phase_;
 };
 

components/sync/engine/cycle/sync_cycle_context.h

@@ -151,7 +151,7 @@ class SyncCycleContext {
   // client behavior on server side.
   const raw_ptr<DebugInfoGetter, DanglingUntriaged> debug_info_getter_;
 
-  raw_ptr<ModelTypeRegistry> model_type_registry_;
+  const raw_ptr<ModelTypeRegistry> model_type_registry_;
 
   // Satus information to be sent up to the server.
   sync_pb::ClientStatus client_status_;

components/sync/engine/get_updates_processor.h

@@ -83,14 +83,11 @@ class GetUpdatesProcessor {
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, InvalidResponse);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, MoreToDownloadResponse);
   FRIEND_TEST_ALL_PREFIXES(GetUpdatesProcessorTest, NormalResponseTest);
-  FRIEND_TEST_ALL_PREFIXES(DownloadUpdatesDebugInfoTest,
-                           VerifyCopyClientDebugInfo_Empty);
-  FRIEND_TEST_ALL_PREFIXES(DownloadUpdatesDebugInfoTest, VerifyCopyOverwrites);
 
   // A map of 'update handlers', one for each enabled type.
   // This must be kept in sync with the routing info.  Our temporary solution to
   // that problem is to initialize this map in set_routing_info().
-  raw_ptr<UpdateHandlerMap> update_handler_map_;
+  const raw_ptr<UpdateHandlerMap> update_handler_map_;
 
   // Whether last GetUpdatesResponse has non-zero `changes_remaining`.
   bool has_more_updates_to_download_ = false;

components/sync/engine/get_updates_processor_unittest.cc

@@ -32,18 +32,13 @@ namespace {
 }  // namespace
 
 // A test fixture for tests exercising download updates functions.
-class GetUpdatesProcessorTest : public ::testing::Test {
+class GetUpdatesProcessorBaseTest : public ::testing::Test {
  public:
-  GetUpdatesProcessorTest() = default;
+  GetUpdatesProcessorBaseTest() = default;
 
-  GetUpdatesProcessorTest(const GetUpdatesProcessorTest&) = delete;
-  GetUpdatesProcessorTest& operator=(const GetUpdatesProcessorTest&) = delete;
-
-  void SetUp() override {
-    autofill_handler_ = AddUpdateHandler(AUTOFILL);
-    bookmarks_handler_ = AddUpdateHandler(BOOKMARKS);
-    preferences_handler_ = AddUpdateHandler(PREFERENCES);
-  }
+  GetUpdatesProcessorBaseTest(const GetUpdatesProcessorBaseTest&) = delete;
+  GetUpdatesProcessorBaseTest& operator=(const GetUpdatesProcessorBaseTest&) =
+      delete;
 
   ModelTypeSet enabled_types() { return enabled_types_; }
 
@@ -82,23 +77,30 @@ class GetUpdatesProcessorTest : public ::testing::Test {
     return handler_ptr;
   }
 
-  MockUpdateHandler* GetBookmarksHandler() { return bookmarks_handler_; }
-
-  MockUpdateHandler* GetAutofillHandler() { return autofill_handler_; }
-
-  MockUpdateHandler* GetPreferencesHandler() { return preferences_handler_; }
-
   const base::TimeTicks kTestStartTime = base::TimeTicks::Now();
 
  private:
   ModelTypeSet enabled_types_;
   std::set<std::unique_ptr<MockUpdateHandler>> update_handlers_;
   UpdateHandlerMap update_handler_map_;
   std::unique_ptr<GetUpdatesProcessor> get_updates_processor_;
+};
+
+class GetUpdatesProcessorTest : public GetUpdatesProcessorBaseTest {
+ public:
+  MockUpdateHandler* GetBookmarksHandler() { return bookmarks_handler_; }
+
+  MockUpdateHandler* GetAutofillHandler() { return autofill_handler_; }
 
-  raw_ptr<MockUpdateHandler> bookmarks_handler_;
-  raw_ptr<MockUpdateHandler> autofill_handler_;
-  raw_ptr<MockUpdateHandler> preferences_handler_;
+  MockUpdateHandler* GetPreferencesHandler() { return preferences_handler_; }
+
+ private:
+  const raw_ptr<MockUpdateHandler> bookmarks_handler_ =
+      AddUpdateHandler(BOOKMARKS);
+  const raw_ptr<MockUpdateHandler> autofill_handler_ =
+      AddUpdateHandler(AUTOFILL);
+  const raw_ptr<MockUpdateHandler> preferences_handler_ =
+      AddUpdateHandler(PREFERENCES);
 };
 
 // Basic test to make sure nudges are expressed properly in the request.
@@ -407,25 +409,22 @@ TEST_F(GetUpdatesProcessorTest, NormalResponseTest) {
 //
 // Maintains two enabled types, but requests that updates be applied for only
 // one of them.
-class GetUpdatesProcessorApplyUpdatesTest : public GetUpdatesProcessorTest {
+class GetUpdatesProcessorApplyUpdatesTest : public GetUpdatesProcessorBaseTest {
  public:
   GetUpdatesProcessorApplyUpdatesTest() = default;
   ~GetUpdatesProcessorApplyUpdatesTest() override = default;
 
-  void SetUp() override {
-    bookmarks_handler_ = AddUpdateHandler(BOOKMARKS);
-    autofill_handler_ = AddUpdateHandler(AUTOFILL);
-  }
-
   ModelTypeSet GetGuTypes() { return {AUTOFILL}; }
 
   MockUpdateHandler* GetNonAppliedHandler() { return bookmarks_handler_; }
 
   MockUpdateHandler* GetAppliedHandler() { return autofill_handler_; }
 
  private:
-  raw_ptr<MockUpdateHandler> bookmarks_handler_;
-  raw_ptr<MockUpdateHandler> autofill_handler_;
+  const raw_ptr<MockUpdateHandler> bookmarks_handler_ =
+      AddUpdateHandler(BOOKMARKS);
+  const raw_ptr<MockUpdateHandler> autofill_handler_ =
+      AddUpdateHandler(AUTOFILL);
 };
 
 // Verify that a normal cycle applies updates to the specified types.
@@ -478,20 +477,4 @@ TEST_F(GetUpdatesProcessorApplyUpdatesTest, Poll) {
   EXPECT_EQ(1, GetAppliedHandler()->GetApplyUpdatesCount());
 }
 
-class DownloadUpdatesDebugInfoTest : public ::testing::Test {
- public:
-  DownloadUpdatesDebugInfoTest() = default;
-  ~DownloadUpdatesDebugInfoTest() override = default;
-
-  StatusController* status() { return &status_; }
-
-  DebugInfoGetter* debug_info_getter() { return &debug_info_getter_; }
-
-  void AddDebugEvent() { debug_info_getter_.AddDebugEvent(); }
-
- private:
-  StatusController status_;
-  MockDebugInfoGetter debug_info_getter_;
-};
-
 }  // namespace syncer

components/sync/engine/loopback_server/loopback_server.cc

@@ -241,8 +241,7 @@ LoopbackServer::LoopbackServer(const base::FilePath& persistent_file)
       writer_(
           persistent_file_,
           base::ThreadPool::CreateSequencedTaskRunner(
-              {base::MayBlock(), base::TaskShutdownBehavior::BLOCK_SHUTDOWN})),
-      observer_for_tests_(nullptr) {
+              {base::MayBlock(), base::TaskShutdownBehavior::BLOCK_SHUTDOWN})) {
   DCHECK(!persistent_file_.empty());
   Init();
 }

components/sync/engine/loopback_server/loopback_server.h

@@ -265,7 +265,7 @@ class LoopbackServer : public base::ImportantFileWriter::DataSerializer {
   SEQUENCE_CHECKER(sequence_checker_);
 
   // Used to observe the completion of commit messages for the sake of testing.
-  raw_ptr<ObserverForTests> observer_for_tests_;
+  raw_ptr<ObserverForTests> observer_for_tests_ = nullptr;
 
   // Response type override callback used in tests.
   ResponseTypeProvider response_type_override_;

components/sync/engine/net/http_bridge_unittest.cc

@@ -146,7 +146,7 @@ class ShuntedHttpBridge : public HttpBridge {
     OnURLLoadCompleteInternal(200, net::OK, GURL("http://www.google.com"),
                               std::make_unique<std::string>("success!"));
   }
-  raw_ptr<MAYBE_SyncHttpBridgeTest> test_;
+  const raw_ptr<MAYBE_SyncHttpBridgeTest> test_;
   bool never_finishes_;
 };
 

components/sync/engine/sync_manager.h

@@ -90,18 +90,18 @@ class SyncManager {
     std::unique_ptr<SyncEncryptionHandler::Observer> encryption_observer_proxy;
 
     // Must outlive SyncManager.
-    raw_ptr<ExtensionsActivity> extensions_activity;
+    raw_ptr<ExtensionsActivity> extensions_activity = nullptr;
 
     std::unique_ptr<EngineComponentsFactory> engine_components_factory;
 
     // Must outlive SyncManager.
-    raw_ptr<SyncEncryptionHandler> encryption_handler;
+    raw_ptr<SyncEncryptionHandler> encryption_handler = nullptr;
 
     // Carries shutdown requests across threads and will be used to cut short
     // any network I/O and tell the syncer to exit early.
     //
     // Must outlive SyncManager.
-    raw_ptr<CancelationSignal> cancelation_signal;
+    raw_ptr<CancelationSignal> cancelation_signal = nullptr;
 
     // Define the polling interval. Must not be zero.
     base::TimeDelta poll_interval;

components/sync/engine/sync_manager_factory.h

@@ -33,7 +33,7 @@ class SyncManagerFactory {
       const std::string& name);
 
  private:
-  raw_ptr<network::NetworkConnectionTracker> network_connection_tracker_;
+  const raw_ptr<network::NetworkConnectionTracker> network_connection_tracker_;
 };
 
 }  // namespace syncer

components/sync/engine/sync_manager_impl.cc

@@ -85,11 +85,7 @@ GURL MakeConnectionURL(const GURL& sync_server, const std::string& client_id) {
 SyncManagerImpl::SyncManagerImpl(
     const std::string& name,
     network::NetworkConnectionTracker* network_connection_tracker)
-    : name_(name),
-      network_connection_tracker_(network_connection_tracker),
-      initialized_(false),
-      observing_network_connectivity_changes_(false),
-      sync_encryption_handler_(nullptr) {}
+    : name_(name), network_connection_tracker_(network_connection_tracker) {}
 
 SyncManagerImpl::~SyncManagerImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/sync/engine/sync_manager_impl.h

@@ -130,7 +130,7 @@ class SyncManagerImpl
 
   const std::string name_;
 
-  raw_ptr<network::NetworkConnectionTracker> network_connection_tracker_;
+  const raw_ptr<network::NetworkConnectionTracker> network_connection_tracker_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 
@@ -157,16 +157,16 @@ class SyncManagerImpl
   std::unique_ptr<SyncStatusTracker> sync_status_tracker_;
 
   // Set to true once Init has been called.
-  bool initialized_;
+  bool initialized_ = false;
 
-  bool observing_network_connectivity_changes_;
+  bool observing_network_connectivity_changes_ = false;
 
   // This is for keeping track of client events to send to the server.
   DebugInfoEventListener debug_info_event_listener_;
 
   ProtocolEventBuffer protocol_event_buffer_;
 
-  raw_ptr<SyncEncryptionHandler> sync_encryption_handler_;
+  raw_ptr<SyncEncryptionHandler> sync_encryption_handler_ = nullptr;
 
   std::unique_ptr<SyncEncryptionHandler::Observer> encryption_observer_proxy_;
 

components/sync/engine/sync_scheduler_impl.h

@@ -273,7 +273,7 @@ class SyncSchedulerImpl : public SyncScheduler {
   // Invoked to run through the sync cycle.
   const std::unique_ptr<Syncer> syncer_;
 
-  raw_ptr<SyncCycleContext> cycle_context_;
+  const raw_ptr<SyncCycleContext> cycle_context_;
 
   // TryJob might get called for multiple reasons. It should only call
   // DoPollSyncCycleJob after some time since the last attempt.

components/sync/model/client_tag_based_model_type_processor.cc

@@ -57,7 +57,7 @@ size_t CountDuplicateClientTags(const EntityMetadataMap& metadata_map) {
 ClientTagBasedModelTypeProcessor::ClientTagBasedModelTypeProcessor(
     ModelType type,
     const base::RepeatingClosure& dump_stack)
-    : type_(type), bridge_(nullptr), dump_stack_(dump_stack) {
+    : type_(type), dump_stack_(dump_stack) {
   ResetState(CLEAR_METADATA);
 }
 

components/sync/model/client_tag_based_model_type_processor.h

@@ -257,7 +257,7 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
 
   // ModelTypeSyncBridge linked to this processor. The bridge owns this
   // processor instance so the pointer should never become invalid.
-  raw_ptr<ModelTypeSyncBridge, DanglingUntriaged> bridge_;
+  raw_ptr<ModelTypeSyncBridge, DanglingUntriaged> bridge_ = nullptr;
 
   // Function to capture and upload a stack trace when an error occurs.
   const base::RepeatingClosure dump_stack_;

components/sync/model/sync_metadata_store_change_list.h

@@ -56,7 +56,7 @@ class SyncMetadataStoreChangeList : public MetadataChangeList {
   void SetError(ModelError error);
 
   // The metadata store to store metadata in; always outlives |this|.
-  raw_ptr<SyncMetadataStore> store_;
+  const raw_ptr<SyncMetadataStore> store_;
 
   // The sync model type for this metadata.
   ModelType type_;

components/sync/nigori/nigori_model_type_processor.cc

@@ -34,7 +34,7 @@ const char kRawNigoriClientTagHash[] = "NigoriClientTagHash";
 
 }  // namespace
 
-NigoriModelTypeProcessor::NigoriModelTypeProcessor() : bridge_(nullptr) {}
+NigoriModelTypeProcessor::NigoriModelTypeProcessor() = default;
 
 NigoriModelTypeProcessor::~NigoriModelTypeProcessor() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

components/sync/nigori/nigori_model_type_processor.h

@@ -90,7 +90,7 @@ class NigoriModelTypeProcessor : public ModelTypeProcessor,
 
   // The bridge owns this processor instance so the pointer should never become
   // invalid.
-  raw_ptr<NigoriSyncBridge> bridge_;
+  raw_ptr<NigoriSyncBridge> bridge_ = nullptr;
 
   // The model type metadata (progress marker, initial sync done, etc).
   sync_pb::ModelTypeState model_type_state_;

components/sync/nigori/nigori_model_type_processor_unittest.cc

@@ -107,10 +107,7 @@ class MockNigoriSyncBridge : public NigoriSyncBridge {
 
 class NigoriModelTypeProcessorTest : public testing::Test {
  public:
-  NigoriModelTypeProcessorTest() {
-    mock_commit_queue_ = std::make_unique<testing::NiceMock<MockCommitQueue>>();
-    mock_commit_queue_ptr_ = mock_commit_queue_.get();
-  }
+  NigoriModelTypeProcessorTest() = default;
 
   void SimulateModelReadyToSync(bool initial_sync_done, int server_version) {
     NigoriMetadataBatch nigori_metadata_batch;
@@ -172,8 +169,10 @@ class NigoriModelTypeProcessorTest : public testing::Test {
 
  private:
   testing::NiceMock<MockNigoriSyncBridge> mock_nigori_sync_bridge_;
-  std::unique_ptr<testing::NiceMock<MockCommitQueue>> mock_commit_queue_;
-  raw_ptr<MockCommitQueue, DanglingUntriaged> mock_commit_queue_ptr_;
+  std::unique_ptr<testing::NiceMock<MockCommitQueue>> mock_commit_queue_ =
+      std::make_unique<testing::NiceMock<MockCommitQueue>>();
+  raw_ptr<MockCommitQueue, DanglingUntriaged> mock_commit_queue_ptr_ =
+      mock_commit_queue_.get();
   NigoriModelTypeProcessor processor_;
 };
 

components/sync/nigori/nigori_sync_bridge_impl_unittest.cc

@@ -254,8 +254,9 @@ class NigoriSyncBridgeImplTest : public testing::Test {
  private:
   std::unique_ptr<NigoriSyncBridgeImpl> bridge_;
   // Ownership transferred to |bridge_|.
-  raw_ptr<testing::NiceMock<MockNigoriLocalChangeProcessor>> processor_;
-  raw_ptr<testing::NiceMock<MockNigoriStorage>> storage_;
+  raw_ptr<testing::NiceMock<MockNigoriLocalChangeProcessor>> processor_ =
+      nullptr;
+  raw_ptr<testing::NiceMock<MockNigoriStorage>> storage_ = nullptr;
   testing::NiceMock<MockObserver> observer_;
 };
 

components/sync/protocol/proto_value_conversions.cc

@@ -311,7 +311,7 @@ class ToValueVisitor {
   }
 
   const ProtoValueConversionOptions options_;
-  raw_ptr<base::Value::Dict> value_;
+  const raw_ptr<base::Value::Dict> value_;
 };
 
 }  // namespace

components/sync/service/backend_migrator.h

@@ -80,7 +80,7 @@ class BackendMigrator {
   void OnConfigureDoneImpl(const DataTypeManager::ConfigureResult& result);
 
   const std::string name_;
-  raw_ptr<DataTypeManager, DanglingUntriaged> manager_;
+  const raw_ptr<DataTypeManager, DanglingUntriaged> manager_;
 
   const base::RepeatingClosure reconfigure_callback_;
   const base::RepeatingClosure migration_done_callback_;

components/sync/service/data_type_manager_impl.h

@@ -185,7 +185,7 @@ class DataTypeManagerImpl : public DataTypeManager,
 
   // The encryption handler lets the DataTypeManager know the state of sync
   // datatype encryption.
-  raw_ptr<const DataTypeEncryptionHandler> encryption_handler_;
+  const raw_ptr<const DataTypeEncryptionHandler> encryption_handler_;
 
   base::WeakPtrFactory<DataTypeManagerImpl> weak_ptr_factory_{this};
 };

components/sync/service/glue/sync_engine_impl_unittest.cc

@@ -121,7 +121,7 @@ class FakeSyncManagerFactory : public SyncManagerFactory {
   ModelTypeSet initial_sync_ended_types_;
   ModelTypeSet progress_marker_types_;
   ModelTypeSet configure_fail_types_;
-  raw_ptr<FakeSyncManager*> fake_manager_;
+  const raw_ptr<FakeSyncManager*> fake_manager_;
 };
 
 class MockActiveDevicesProvider : public ActiveDevicesProvider {