From d2188669d75c4adebf43bffa5a40e822c86067cc Mon Sep 17 00:00:00 2001
From: Marc Treib <[email protected]>
Date: Mon, 27 Jul 2026 06:35:34 -0700
Subject: [PATCH] Fix use-after-free in DeviceStatisticsTracker

DeviceStatisticsTracker promises (via a comment on Start() that its
callback is not run anymore if the tracker is destroyed. Before this CL,
there were some edge cases where this was not true, since the callback
was run as a posted task.

This CL fixes this by running the callback through a class method, bound
to a weak ptr.

Fixed: 539341100
Change-Id: Ie95a230ddceea391bdf0e7aadbdf9c8e6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8154099
Reviewed-by: Justin Cohen <[email protected]>
Commit-Queue: Marc Treib <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1668589}
---

diff --git a/components/sync/service/device_statistics_tracker.cc b/components/sync/service/device_statistics_tracker.cc
index ae37196..0058d69 100644
--- a/components/sync/service/device_statistics_tracker.cc
+++ b/components/sync/service/device_statistics_tracker.cc
@@ -228,8 +228,12 @@
       !std::ranges::contains(accounts, primary_account_)) {
     // The primary account must have been removed between the constructor and
     // now, or something's wrong with the IdentityManager.
+    // Note: This is plumbed through a class method to ensure that the callback
+    // isn't run anymore if `this` gets destroyed first.
     base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
-        FROM_HERE, std::move(callback_));
+        FROM_HERE,
+        base::BindOnce(&DeviceStatisticsTracker::RunCallback,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback_)));
     return;
   }
 
@@ -247,8 +251,12 @@
     RecordOverallDevicesOutcome();
     RecordOverallPlatformsOutcome();
 
+    // Note: This is plumbed through a class method to ensure that the callback
+    // isn't run anymore if `this` gets destroyed first.
     base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
-        FROM_HERE, std::move(callback_));
+        FROM_HERE,
+        base::BindOnce(&DeviceStatisticsTracker::RunCallback,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback_)));
     return;
   }
 
@@ -757,4 +765,8 @@
   return deduped_devices;
 }
 
+void DeviceStatisticsTracker::RunCallback(base::OnceClosure callback) {
+  std::move(callback).Run();
+}
+
 }  // namespace syncer
diff --git a/components/sync/service/device_statistics_tracker.h b/components/sync/service/device_statistics_tracker.h
index 0ce7777..667ce12 100644
--- a/components/sync/service/device_statistics_tracker.h
+++ b/components/sync/service/device_statistics_tracker.h
@@ -14,6 +14,7 @@
 #include "base/containers/flat_set.h"
 #include "base/functional/callback.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/types/expected.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "google_apis/gaia/gaia_id.h"
@@ -260,6 +261,8 @@
       const std::vector<sync_pb::SyncEntity>& entities,
       const base::flat_set<std::string>& current_device_cache_guids);
 
+  void RunCallback(base::OnceClosure callback);
+
   const raw_ptr<signin::IdentityManager> identity_manager_;
 
   const GURL sync_server_url_;
@@ -293,6 +296,8 @@
   // devices (which may be empty).
   base::flat_map<GaiaId, base::expected<std::vector<DeviceData>, RequestFailed>>
       other_devices_by_gaia_;
+
+  base::WeakPtrFactory<DeviceStatisticsTracker> weak_ptr_factory_{this};
 };
 
 }  // namespace syncer