Fix potential sync loops due to broadcasting empty updates

We thought there was no reason to avoid broadcasting notifications when
an empty list of remote updates is received, but it turns out that at
least one client (iOS RecentTabsMediator) relied on the fact that such
events would be ignored (not broadcasted via
SyncServiceObserver::OnForeignSessionUpdated()).

To avoid behavioral changes with the pre-USS implementation, which
dropped such empty updates in an earlier stage (I believe in
GenericChangeProcessor::CommitChangesFromSyncModel()), we adopt similar
logic for USS (SessionSyncBridge).

Bug: 854049
Change-Id: I6d3b6123b7fb15bb0e5bfeccdc0a6d9170427fe4
Reviewed-on: https://chromium-review.googlesource.com/1106143Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568444}

components/sync_sessions/session_sync_bridge.cc

@@ -261,9 +261,10 @@ base::Optional<syncer::ModelError> SessionSyncBridge::ApplySyncChanges(
       ->TransferChangesTo(batch->GetMetadataChangeList());
   SessionStore::WriteBatch::Commit(std::move(batch));
 
-  // This might overtrigger because we don't check if the batch is empty, but
-  // observers should handle these events well so we don't bother detecting.
+  if (!entity_changes.empty()) {
     foreign_sessions_updated_callback_.Run();
+  }
+
   return base::nullopt;
 }
 

components/sync_sessions/session_sync_bridge_unittest.cc

@@ -1384,6 +1384,20 @@ TEST_F(SessionSyncBridgeTest, ShouldIgnoreLocalSessionDeletionFromUI) {
               NotNull());
 }
 
+// Verifies that receiving an empty update list does not broadcast a foreign
+// session change via the corresponding callback.
+TEST_F(SessionSyncBridgeTest, ShouldNotBroadcastUpdatesIfEmpty) {
+  InitializeBridge();
+  StartSyncing();
+
+  EXPECT_CALL(mock_foreign_sessions_updated_callback(), Run()).Times(0);
+
+  // Mimic receiving an empty list of remote updates.
+  sync_pb::ModelTypeState state;
+  state.set_initial_sync_done(true);
+  real_processor()->OnUpdateReceived(state, {});
+}
+
 TEST_F(SessionSyncBridgeTest, ShouldDoGarbageCollection) {
   // We construct two identical sessions, one modified recently, one modified
   // more than |kStaleSessionThreshold| ago (14 days ago).