[Sync] Notify observers on the initial load of DeviceInfos

OnDeviceInfoChange() should be called on each change of the device info
list. Some observers expect that this callback will be invoked on each
change, even after loading from the persistent storage when the list
hasn't been actually changed on browser startup.

This CL slightly changes this behaviour. Observers will be notified when
the metadata is fully loaded and the local device hasn't been changed.

Bug: 1023457
Change-Id: I8012e5dda272bab7d218d9d51dca6ca5af5053ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2633623
Commit-Queue: Rushan Suleymanov <rushans@google.com>
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846049}

components/sync_device_info/device_info_sync_bridge.cc

@@ -613,7 +613,13 @@ void DeviceInfoSyncBridge::OnReadAllMetadata(
   // we save the new one to prefs.
   device_info_prefs_->AddLocalCacheGuid(local_cache_guid_);
   ExpireOldEntries();
-  ReconcileLocalAndStored();
+  if (!ReconcileLocalAndStored()) {
+    // If the device info list has not been changed, notify observers explicitly
+    // that the list of devices has been successfully loaded from the storage.
+    // Otherwise, all observers should already have been notified during
+    // ReconcileLocalAndStored().
+    NotifyObservers();
+  }
 }
 
 void DeviceInfoSyncBridge::OnCommit(

components/sync_device_info/device_info_sync_bridge_unittest.cc

@@ -1482,6 +1482,29 @@ TEST_F(DeviceInfoSyncBridgeTest,
               IsNull());
 }
 
+TEST_F(DeviceInfoSyncBridgeTest, ShouldInvokeCallbackOnReadAllMetadata) {
+  const DeviceInfoSpecifics specifics = CreateLocalDeviceSpecifics();
+  const ModelTypeState model_type_state = StateWithEncryption("ekn");
+  WriteToStoreWithMetadata({specifics}, model_type_state);
+
+  InitializeBridge();
+  // Wait until the metadata is loaded.
+  base::RunLoop run_loop;
+  EXPECT_CALL(*processor(), IsTrackingMetadata).WillOnce(Return(true));
+  EXPECT_CALL(*processor(), ModelReadyToSync)
+      .WillOnce([&run_loop](std::unique_ptr<MetadataBatch> batch) {
+        run_loop.Quit();
+      });
+  run_loop.Run();
+
+  // Check that the bridge won't call SendLocalData() during initial merge.
+  EXPECT_CALL(*processor(), Put).Times(0);
+
+  // Check that the bridge has notified observers even if the local data hasn't
+  // been changed.
+  EXPECT_EQ(1, change_count());
+}
+
 }  // namespace
 
 }  // namespace syncer

components/sync_device_info/device_info_tracker.h

@@ -21,6 +21,17 @@ class DeviceInfoTracker {
   // Observer class for listening to device info changes.
   class Observer {
    public:
+    // Called on any change in the device info list. If sync is enabled, it is
+    // guaranteed that the method will be called at least once after browser
+    // startup. There are several possible cases:
+    // 1. The list has been changed during remote update (initial merge or
+    // incremental).
+    // 2. The list has been cleaned up when sync is stopped.
+    // 3. The local device info has been changed and committed to the server.
+    // 4. The list has been just loaded after browser startup from the
+    // persistent storage. If the list is empty (e.g. due to mismatching cache
+    // GUID or this is the first browser startup), it will be updated later
+    // during the initial merge.
     virtual void OnDeviceInfoChange() = 0;
   };