Update the floc computation scheduling pattern

Before: Schedule the computation events using a RepeatingTimer and use a
session number to address potential race conditions.

After: Use a flag |floc_computation_in_progress_| to guard the process
from ComputeFloc to OnComputeFlocCompleted. Incoming computation will be
ignored if there’s one ongoing. After each OnComputeFlocCompleted, a
ComputeFloc event will be scheduled 24 hours later / override the
existing scheduled event.

Why: The new pattern would be more suitable for a future plan where more
computation triggers are introduced (e.g. kHistoryDelete). In that case,
the caller can simply call
ComputeFloc(ComputeFlocTrigger::kHistoryDelete) without needing to reset
the timer.


Bug: 1062736
Change-Id: Ic128262272c71be3bc8998bb192ff7c7b615faa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359394
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799547}

chrome/browser/federated_learning/floc_id_provider_browsertest.cc

@@ -278,7 +278,7 @@ IN_PROC_BROWSER_TEST_F(FlocIdProviderWithCustomizedServicesBrowserTest,
 
   EXPECT_EQ(GetFlocId().ToDebugHeaderValue(), FlocId().ToDebugHeaderValue());
 
-  // Turn on sync-history to trigger the start of the 1st floc session.
+  // Turn on sync-history to trigger the 1st floc computation.
   sync_service()->SetActiveDataTypes(syncer::ModelTypeSet::All());
   sync_service()->FireStateChanged();
 
@@ -313,7 +313,7 @@ IN_PROC_BROWSER_TEST_F(FlocIdProviderWithCustomizedServicesBrowserTest,
 
   EXPECT_EQ(GetFlocId().ToDebugHeaderValue(), FlocId().ToDebugHeaderValue());
 
-  // Turn on sync-history to trigger the start of the 1st floc session.
+  // Turn on sync-history to trigger the 1st floc computation.
   sync_service()->SetActiveDataTypes(syncer::ModelTypeSet::All());
   sync_service()->FireStateChanged();
 

chrome/browser/federated_learning/floc_id_provider_impl.cc

@@ -25,7 +25,7 @@ namespace federated_learning {
 namespace {
 
 constexpr size_t kMinHistoryDomainSizeToReportFlocId = 1;
-constexpr base::TimeDelta kFlocSessionRenewInterval =
+constexpr base::TimeDelta kFlocScheduledUpdateInterval =
     base::TimeDelta::FromDays(1);
 constexpr int kQueryHistoryWindowInDays = 7;
 
@@ -48,9 +48,7 @@ FlocIdProviderImpl::FlocIdProviderImpl(
 
 FlocIdProviderImpl::~FlocIdProviderImpl() = default;
 
-void FlocIdProviderImpl::NotifyFlocIdUpdated() {
-  DCHECK(floc_session_count_ > 0);
-
+void FlocIdProviderImpl::NotifyFlocUpdated(ComputeFlocTrigger trigger) {
   if (!base::FeatureList::IsEnabled(features::kFlocIdComputedEventLogging))
     return;
 
@@ -60,9 +58,9 @@ void FlocIdProviderImpl::NotifyFlocIdUpdated() {
       specifics->mutable_floc_id_computed_event();
 
   sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger event_trigger =
-      (floc_session_count_ == 1u)
-          ? sync_pb::UserEventSpecifics::FlocIdComputed::NEW
-          : sync_pb::UserEventSpecifics::FlocIdComputed::REFRESHED;
+      (trigger == ComputeFlocTrigger::kBrowserStart)
+          ? sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_NEW
+          : sync_pb::UserEventSpecifics_FlocIdComputed_EventTrigger_REFRESHED;
 
   floc_id_computed_event->set_event_trigger(event_trigger);
 
@@ -86,7 +84,7 @@ bool FlocIdProviderImpl::AreThirdPartyCookiesAllowed() {
 }
 
 void FlocIdProviderImpl::IsSwaaNacAccountEnabled(
-    CanComputeFlocIdCallback callback) {
+    CanComputeFlocCallback callback) {
   net::PartialNetworkTrafficAnnotationTag partial_traffic_annotation =
       net::DefinePartialNetworkTrafficAnnotation(
           "floc_id_provider_impl", "floc_remote_permission_service",
@@ -127,29 +125,59 @@ void FlocIdProviderImpl::Shutdown() {
 }
 
 void FlocIdProviderImpl::OnStateChanged(syncer::SyncService* sync_service) {
-  if (floc_session_count_ > 0)
+  if (first_floc_computation_triggered_)
     return;
 
   if (!IsSyncHistoryEnabled())
     return;
 
-  CalculateFloc();
+  ComputeFloc(ComputeFlocTrigger::kBrowserStart);
+}
+
+void FlocIdProviderImpl::ComputeFloc(ComputeFlocTrigger trigger) {
+  DCHECK_NE(trigger == ComputeFlocTrigger::kBrowserStart,
+            first_floc_computation_triggered_);
+
+  DCHECK(trigger != ComputeFlocTrigger::kBrowserStart ||
+         !floc_computation_in_progress_);
+
+  // Skip computing as long as there's one still in progress.
+  if (floc_computation_in_progress_)
+    return;
+
+  floc_computation_in_progress_ = true;
+  first_floc_computation_triggered_ = true;
+
+  auto compute_floc_completed_callback =
+      base::BindOnce(&FlocIdProviderImpl::OnComputeFlocCompleted,
+                     weak_ptr_factory_.GetWeakPtr(), trigger);
 
-  floc_session_start_timer_.Start(
-      FROM_HERE, kFlocSessionRenewInterval,
-      base::BindRepeating(&FlocIdProviderImpl::CalculateFloc,
-                          weak_ptr_factory_.GetWeakPtr()));
+  CheckCanComputeFloc(
+      base::BindOnce(&FlocIdProviderImpl::OnCheckCanComputeFlocCompleted,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     std::move(compute_floc_completed_callback)));
 }
 
-void FlocIdProviderImpl::CalculateFloc() {
-  floc_session_count_ += 1;
-  CheckCanComputeFlocId(
-      base::BindOnce(&FlocIdProviderImpl::OnCheckCanComputeFlocIdCompleted,
-                     weak_ptr_factory_.GetWeakPtr()));
+void FlocIdProviderImpl::OnComputeFlocCompleted(ComputeFlocTrigger trigger,
+                                                FlocId floc_id) {
+  DCHECK(floc_computation_in_progress_);
+  floc_computation_in_progress_ = false;
+
+  if (floc_id_ != floc_id) {
+    floc_id_ = floc_id;
+    NotifyFlocUpdated(trigger);
+  }
+
+  // Abandon the scheduled task if any, and schedule a new compute-floc task
+  // that is |kFlocScheduledUpdateInterval| from now.
+  compute_floc_timer_.Start(
+      FROM_HERE, kFlocScheduledUpdateInterval,
+      base::BindOnce(&FlocIdProviderImpl::ComputeFloc,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     ComputeFlocTrigger::kScheduledUpdate));
 }
 
-void FlocIdProviderImpl::CheckCanComputeFlocId(
-    CanComputeFlocIdCallback callback) {
+void FlocIdProviderImpl::CheckCanComputeFloc(CanComputeFlocCallback callback) {
   if (!IsSyncHistoryEnabled() || !AreThirdPartyCookiesAllowed()) {
     std::move(callback).Run(false);
     return;
@@ -158,19 +186,17 @@ void FlocIdProviderImpl::CheckCanComputeFlocId(
   IsSwaaNacAccountEnabled(std::move(callback));
 }
 
-void FlocIdProviderImpl::OnCheckCanComputeFlocIdCompleted(
+void FlocIdProviderImpl::OnCheckCanComputeFlocCompleted(
+    ComputeFlocCompletedCallback callback,
     bool can_compute_floc) {
   if (!can_compute_floc) {
-    if (floc_id_.IsValid()) {
-      floc_id_ = FlocId();
-      NotifyFlocIdUpdated();
-    }
+    std::move(callback).Run(FlocId());
     return;
   }
 
   GetRecentlyVisitedURLs(
       base::BindOnce(&FlocIdProviderImpl::OnGetRecentlyVisitedURLsCompleted,
-                     weak_ptr_factory_.GetWeakPtr(), floc_session_count_));
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void FlocIdProviderImpl::GetRecentlyVisitedURLs(
@@ -184,12 +210,8 @@ void FlocIdProviderImpl::GetRecentlyVisitedURLs(
 }
 
 void FlocIdProviderImpl::OnGetRecentlyVisitedURLsCompleted(
-    size_t floc_session_count,
+    ComputeFlocCompletedCallback callback,
     history::QueryResults results) {
-  DCHECK_LE(floc_session_count, floc_session_count_);
-  if (floc_session_count < floc_session_count_)
-    return;
-
   std::unordered_set<std::string> domains;
   for (const history::URLResult& url_result : results) {
     if (!url_result.publicly_routable())
@@ -204,10 +226,7 @@ void FlocIdProviderImpl::OnGetRecentlyVisitedURLsCompleted(
                        ? FlocId::CreateFromHistory(domains)
                        : FlocId();
 
-  if (floc_id_ != floc_id) {
-    floc_id_ = floc_id;
-    NotifyFlocIdUpdated();
-  }
+  std::move(callback).Run(floc_id);
 }
 
 }  // namespace federated_learning

chrome/browser/federated_learning/floc_id_provider_impl.h

@@ -25,19 +25,31 @@ namespace federated_learning {
 
 class FlocRemotePermissionService;
 
-// A service that regularly computes the floc id and logs it in a user event.
+// A service that regularly computes the floc id and logs it in a user event. A
+// computed floc can be in either a valid or invalid state, based on whether all
+// the prerequisites are met:
+// 1) Sync & sync-history are enabled.
+// 2) 3rd party cookies are NOT blocked.
+// 3) Supplemental Web and App Activity is enabled.
+// 4) Supplemental Ad Personalization is enabled.
+// 5) The account type is NOT a child account.
 //
-// A floc session starts when sync & sync-history is first enabled. We validate
-// the following conditions in sequence before computing the floc id: user
-// doesn’t block 3rd party cookies; all 3 of swaa & nac & account_type are
-// enabled; the user has visited at least 1 domain with publicly routable ip in
-// the past 7 days. Every 24 hours, it'll go over the conditions above again
-// (including the sync & sync-history check), and reset or recompute the id
-// accordingly.
+// When all the prerequisites are met, the floc will be computed by sim-hashing
+// navigation URL domains in the last 7 days; otherwise, an invalid floc will be
+// given.
+//
+// The floc will be first computed after sync & sync-history are enabled. After
+// each computation, another computation will be scheduled 24 hours later.
 class FlocIdProviderImpl : public FlocIdProvider,
                            public syncer::SyncServiceObserver {
  public:
-  using CanComputeFlocIdCallback = base::OnceCallback<void(bool)>;
+  enum class ComputeFlocTrigger {
+    kBrowserStart,
+    kScheduledUpdate,
+  };
+
+  using CanComputeFlocCallback = base::OnceCallback<void(bool)>;
+  using ComputeFlocCompletedCallback = base::OnceCallback<void(FlocId)>;
   using GetRecentlyVisitedURLsCallback =
       history::HistoryService::QueryHistoryCallback;
 
@@ -53,10 +65,10 @@ class FlocIdProviderImpl : public FlocIdProvider,
 
  protected:
   // protected virtual for testing.
-  virtual void NotifyFlocIdUpdated();
+  virtual void NotifyFlocUpdated(ComputeFlocTrigger trigger);
   virtual bool IsSyncHistoryEnabled();
   virtual bool AreThirdPartyCookiesAllowed();
-  virtual void IsSwaaNacAccountEnabled(CanComputeFlocIdCallback callback);
+  virtual void IsSwaaNacAccountEnabled(CanComputeFlocCallback callback);
 
  private:
   friend class FlocIdProviderUnitTest;
@@ -68,17 +80,20 @@ class FlocIdProviderImpl : public FlocIdProvider,
   // syncer::SyncServiceObserver:
   void OnStateChanged(syncer::SyncService* sync_service) override;
 
-  void CalculateFloc();
+  void ComputeFloc(ComputeFlocTrigger trigger);
+  void OnComputeFlocCompleted(ComputeFlocTrigger trigger, FlocId floc_id);
 
-  void CheckCanComputeFlocId(CanComputeFlocIdCallback callback);
-  void OnCheckCanComputeFlocIdCompleted(bool can_compute_floc);
+  void CheckCanComputeFloc(CanComputeFlocCallback callback);
+  void OnCheckCanComputeFlocCompleted(ComputeFlocCompletedCallback callback,
+                                      bool can_compute_floc);
 
   void GetRecentlyVisitedURLs(GetRecentlyVisitedURLsCallback callback);
-  void OnGetRecentlyVisitedURLsCompleted(size_t floc_session_count,
+  void OnGetRecentlyVisitedURLsCompleted(ComputeFlocCompletedCallback callback,
                                          history::QueryResults results);
 
   FlocId floc_id_;
-  size_t floc_session_count_ = 0;
+  bool floc_computation_in_progress_ = false;
+  bool first_floc_computation_triggered_ = false;
 
   syncer::SyncService* sync_service_;
   scoped_refptr<content_settings::CookieSettings> cookie_settings_;
@@ -89,8 +104,8 @@ class FlocIdProviderImpl : public FlocIdProvider,
   // Used for the async tasks querying the HistoryService.
   base::CancelableTaskTracker history_task_tracker_;
 
-  // The timer used to start the floc session at regular intervals.
-  base::RepeatingTimer floc_session_start_timer_;
+  // The timer used to schedule a floc computation.
+  base::OneShotTimer compute_floc_timer_;
 
   base::WeakPtrFactory<FlocIdProviderImpl> weak_ptr_factory_{this};
 };