Bug 1864907 - Part 1: Change the behavior of the cooldown mechansim of cookie banner clicking. r=pbz

This patch changes the cooldown mechansim. The original behavior is stop
executing cookie banner clicking after loading the site in the browsing
session. We modify it to that the cooldown mechanism takes effect after
loading the site several times in the browsing session..

Differential Revision: https://phabricator.services.mozilla.com/D193906

modules/libpref/init/StaticPrefList.yaml

@@ -2039,11 +2039,14 @@
   value: 31536000
   mirror: always
 
-# Whether the banner clicking code only get executed once when the site is
-# loaded in the browsing session.
-- name: cookiebanners.bannerClicking.executeOnce
-  type: bool
-  value: true
+# How many times per site and site load to check for cookie banners after which
+# the mechanism is considered on cooldown for the site in the current browsing
+# session. If the threshold is set to zero, banner clicking won't be considered
+# as being on cooldown regardless of how many times the site is loaded. The
+# maximum value for the retry is 255, any value over than that will be capped.
+- name: cookiebanners.bannerClicking.maxTriesPerSiteAndSession
+  type: uint32_t
+  value: 3
   mirror: always
 
 #---------------------------------------------------------------------------

toolkit/components/cookiebanners/CookieBannerParent.sys.mjs

@@ -24,9 +24,9 @@ XPCOMUtils.defineLazyPreferenceGetter(
 );
 XPCOMUtils.defineLazyPreferenceGetter(
   lazy,
-  "executeOnce",
-  "cookiebanners.bannerClicking.executeOnce",
-  true
+  "maxTriesPerSiteAndSession",
+  "cookiebanners.bannerClicking.maxTriesPerSiteAndSession",
+  3
 );
 
 ChromeUtils.defineLazyGetter(lazy, "CookieBannerL10n", () => {
@@ -200,12 +200,12 @@ export class CookieBannerParent extends JSWindowActorParent {
       }
     }
 
-    // Check if we previously executed banner clicking for the site before. If
-    // the pref instructs to always execute banner clicking, we will set it to
+    // Check if we previously executed banner clicking for the site. If the pref
+    // instructs to always execute banner clicking, we will set it to
     // false.
     let hasExecuted = false;
-    if (lazy.executeOnce) {
-      hasExecuted = Services.cookieBanners.hasExecutedForSite(
+    if (lazy.maxTriesPerSiteAndSession > 0) {
+      hasExecuted = Services.cookieBanners.shouldStopBannerClickingForSite(
         domain,
         this.#isTopLevel,
         this.#isPrivateBrowsing

toolkit/components/cookiebanners/nsCookieBannerService.cpp

@@ -914,14 +914,26 @@ nsCookieBannerService::RemoveAllDomainPrefs(const bool aIsPrivate) {
 }
 
 NS_IMETHODIMP
-nsCookieBannerService::HasExecutedForSite(const nsACString& aSite,
-                                          const bool aIsTopLevel,
-                                          const bool aIsPrivate,
-                                          bool* aHasExecuted) {
+nsCookieBannerService::ShouldStopBannerClickingForSite(const nsACString& aSite,
+                                                       const bool aIsTopLevel,
+                                                       const bool aIsPrivate,
+                                                       bool* aShouldStop) {
   if (!mIsInitialized) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
+  uint8_t threshold =
+      StaticPrefs::cookiebanners_bannerClicking_maxTriesPerSiteAndSession();
+
+  // Don't stop banner clicking if the pref is set to zero.
+  if (threshold == 0) {
+    *aShouldStop = false;
+    return NS_OK;
+  }
+
+  // Ensure we won't use an overflowed threshold.
+  threshold = std::min(threshold, std::numeric_limits<uint8_t>::max());
+
   auto entry = mExecutedDataForSites.MaybeGet(aSite);
 
   if (!entry) {
@@ -929,15 +941,17 @@ nsCookieBannerService::HasExecutedForSite(const nsACString& aSite,
   }
 
   auto& data = entry.ref();
+  uint8_t cnt = 0;
 
   if (aIsPrivate) {
-    *aHasExecuted = aIsTopLevel ? data.hasExecutedInTopPrivate
-                                : data.hasExecutedInFramePrivate;
+    cnt = aIsTopLevel ? data.countExecutedInTopPrivate
+                      : data.countExecutedInFramePrivate;
   } else {
-    *aHasExecuted =
-        aIsTopLevel ? data.hasExecutedInTop : data.hasExecutedInFrame;
+    cnt = aIsTopLevel ? data.countExecutedInTop : data.countExecutedInFrame;
   }
 
+  *aShouldStop = cnt >= threshold;
+
   return NS_OK;
 }
 
@@ -952,21 +966,29 @@ nsCookieBannerService::MarkSiteExecuted(const nsACString& aSite,
   }
 
   auto& data = mExecutedDataForSites.LookupOrInsert(aSite);
+  uint8_t* count = nullptr;
 
   if (aIsPrivate) {
     if (aIsTopLevel) {
-      data.hasExecutedInTopPrivate = true;
+      count = &data.countExecutedInTopPrivate;
     } else {
-      data.hasExecutedInFramePrivate = true;
+      count = &data.countExecutedInFramePrivate;
     }
   } else {
     if (aIsTopLevel) {
-      data.hasExecutedInTop = true;
+      count = &data.countExecutedInTop;
     } else {
-      data.hasExecutedInFrame = true;
+      count = &data.countExecutedInFrame;
     }
   }
 
+  MOZ_ASSERT(count);
+
+  // Ensure we never overflow.
+  if (*count < std::numeric_limits<uint8_t>::max()) {
+    (*count) += 1;
+  }
+
   return NS_OK;
 }
 
@@ -986,16 +1008,16 @@ nsCookieBannerService::RemoveExecutedRecordForSite(const nsACString& aSite,
   auto data = entry.Data();
 
   if (aIsPrivate) {
-    data.hasExecutedInTopPrivate = false;
-    data.hasExecutedInFramePrivate = false;
+    data.countExecutedInTopPrivate = 0;
+    data.countExecutedInFramePrivate = 0;
   } else {
-    data.hasExecutedInTop = false;
-    data.hasExecutedInFrame = false;
+    data.countExecutedInTop = 0;
+    data.countExecutedInFrame = 0;
   }
 
   // We can remove the entry if there is no flag set after removal.
-  if (!data.hasExecutedInTop && !data.hasExecutedInFrame &&
-      !data.hasExecutedInTopPrivate && !data.hasExecutedInFramePrivate) {
+  if (!data.countExecutedInTop && !data.countExecutedInFrame &&
+      !data.countExecutedInTopPrivate && !data.countExecutedInFramePrivate) {
     entry.Remove();
   }
 
@@ -1012,16 +1034,16 @@ nsCookieBannerService::RemoveAllExecutedRecords(const bool aIsPrivate) {
     auto& data = iter.Data();
     // Clear the flags.
     if (aIsPrivate) {
-      data.hasExecutedInTopPrivate = false;
-      data.hasExecutedInFramePrivate = false;
+      data.countExecutedInTopPrivate = 0;
+      data.countExecutedInFramePrivate = 0;
     } else {
-      data.hasExecutedInTop = false;
-      data.hasExecutedInFrame = false;
+      data.countExecutedInTop = 0;
+      data.countExecutedInFrame = 0;
     }
 
     // Remove the entry if there is no flag set.
-    if (!data.hasExecutedInTop && !data.hasExecutedInFrame &&
-        !data.hasExecutedInTopPrivate && !data.hasExecutedInFramePrivate) {
+    if (!data.countExecutedInTop && !data.countExecutedInFrame &&
+        !data.countExecutedInTopPrivate && !data.countExecutedInFramePrivate) {
       iter.Remove();
     }
   }

toolkit/components/cookiebanners/nsCookieBannerService.h

@@ -139,15 +139,15 @@ class nsCookieBannerService final : public nsIObserver,
   // context.
   typedef struct ExecutedData {
     ExecutedData()
-        : hasExecutedInTop(false),
-          hasExecutedInFrame(false),
-          hasExecutedInTopPrivate(false),
-          hasExecutedInFramePrivate(false) {}
+        : countExecutedInTop(0),
+          countExecutedInFrame(0),
+          countExecutedInTopPrivate(0),
+          countExecutedInFramePrivate(0) {}
 
-    bool hasExecutedInTop;
-    bool hasExecutedInFrame;
-    bool hasExecutedInTopPrivate;
-    bool hasExecutedInFramePrivate;
+    uint8_t countExecutedInTop;
+    uint8_t countExecutedInFrame;
+    uint8_t countExecutedInTopPrivate;
+    uint8_t countExecutedInFramePrivate;
   } ExecutedData;
 
   // Map of the sites (eTLD+1) that we have executed the cookie banner handling

toolkit/components/cookiebanners/nsICookieBannerService.idl

@@ -127,12 +127,12 @@ interface nsICookieBannerService : nsISupports {
   void removeAllDomainPrefs(in boolean aIsPrivate);
 
   /**
-   * Return if we have run the cookie banner handling code for the given site
+   * Return true if we should stop cookie banner clicking for the given site in
    * this session.
    */
-  boolean hasExecutedForSite(in ACString aSite,
-                             in bool aIsTopLevel,
-                             in bool aIsPrivate);
+  boolean shouldStopBannerClickingForSite(in ACString aSite,
+                                          in bool aIsTopLevel,
+                                          in bool aIsPrivate);
 
   /**
    * Mark that the cookie banner handling code was executed for the given site