Backed out 4 changesets (bug 1865148, bug 1913613) for causing perma mochistest failures @ test_event_listener_leaks.html

Backed out changeset 47562b188068 (bug 1865148)
Backed out changeset 926b62d4264a (bug 1913613)
Backed out changeset 556574628458 (bug 1913613)
Backed out changeset 81190e44c586 (bug 1913613)

dom/base/IdleRequest.cpp

@@ -40,7 +40,7 @@ void IdleRequest::SetTimeoutHandle(int32_t aHandle) {
   mTimeoutHandle = Some(aHandle);
 }
 
-int32_t IdleRequest::GetTimeoutHandle() const {
+uint32_t IdleRequest::GetTimeoutHandle() const {
   MOZ_DIAGNOSTIC_ASSERT(mTimeoutHandle.isSome());
   return mTimeoutHandle.value();
 }

dom/base/IdleRequest.h

@@ -32,7 +32,7 @@ class IdleRequest final : public LinkedListElement<RefPtr<IdleRequest>> {
 
   void SetTimeoutHandle(int32_t aHandle);
   bool HasTimeout() const { return mTimeoutHandle.isSome(); }
-  int32_t GetTimeoutHandle() const;
+  uint32_t GetTimeoutHandle() const;
 
   uint32_t Handle() const { return mHandle; }
 

dom/base/Timeout.h

@@ -38,7 +38,7 @@ class Timeout final : protected LinkedListElement<RefPtr<Timeout>> {
   };
 
   struct TimeoutIdAndReason {
-    int32_t mId;
+    uint32_t mId;
     Reason mReason;
   };
 
@@ -157,7 +157,7 @@ class Timeout final : protected LinkedListElement<RefPtr<Timeout>> {
   UniquePtr<ProfileChunkedBuffer> mCause;
 
   // Returned as value of setTimeout()
-  int32_t mTimeoutId;
+  uint32_t mTimeoutId;
 
   // Identifies which firing level this Timeout is being processed in
   // when sync loops trigger nested firing.

dom/base/TimeoutManager.cpp

@@ -447,34 +447,16 @@ TimeoutManager::~TimeoutManager() {
           ("TimeoutManager %p destroyed\n", this));
 }
 
-int32_t TimeoutManager::GetTimeoutId(Timeout::Reason aReason) {
-  int32_t timeoutId;
-  do {
-    switch (aReason) {
-      case Timeout::Reason::eIdleCallbackTimeout:
-        timeoutId = mIdleCallbackTimeoutCounter;
-        if (mIdleCallbackTimeoutCounter ==
-            std::numeric_limits<int32_t>::max()) {
-          mIdleCallbackTimeoutCounter = 1;
-        } else {
-          ++mIdleCallbackTimeoutCounter;
-        }
-        break;
-      case Timeout::Reason::eTimeoutOrInterval:
-        timeoutId = mTimeoutIdCounter;
-        if (mTimeoutIdCounter == std::numeric_limits<int32_t>::max()) {
-          mTimeoutIdCounter = 1;
-        } else {
-          ++mTimeoutIdCounter;
-        }
-        break;
-      case Timeout::Reason::eDelayedWebTaskTimeout:
-      default:
-        return -1;  // no cancellation support
-    }
-  } while (mTimeouts.GetTimeout(timeoutId, aReason));
-
-  return timeoutId;
+uint32_t TimeoutManager::GetTimeoutId(Timeout::Reason aReason) {
+  switch (aReason) {
+    case Timeout::Reason::eIdleCallbackTimeout:
+      return ++mIdleCallbackTimeoutCounter;
+    case Timeout::Reason::eTimeoutOrInterval:
+      return ++mTimeoutIdCounter;
+    case Timeout::Reason::eDelayedWebTaskTimeout:
+    default:
+      return std::numeric_limits<uint32_t>::max();  // no cancellation support
+  }
 }
 
 bool TimeoutManager::IsRunningTimeout() const { return mRunningTimeout; }
@@ -595,11 +577,12 @@ bool TimeoutManager::ClearTimeoutInternal(int32_t aTimerId,
                  aReason == Timeout::Reason::eIdleCallbackTimeout,
              "This timeout reason doesn't support cancellation.");
 
+  uint32_t timerId = (uint32_t)aTimerId;
   Timeouts& timeouts = aIsIdle ? mIdleTimeouts : mTimeouts;
   RefPtr<TimeoutExecutor>& executor = aIsIdle ? mIdleExecutor : mExecutor;
   bool deferredDeletion = false;
 
-  Timeout* timeout = timeouts.GetTimeout(aTimerId, aReason);
+  Timeout* timeout = timeouts.GetTimeout(timerId, aReason);
   if (!timeout) {
     return false;
   }

dom/base/TimeoutManager.h

@@ -55,7 +55,7 @@ class TimeoutManager final {
                   bool aProcessIdle);
 
   void ClearAllTimeouts();
-  int32_t GetTimeoutId(mozilla::dom::Timeout::Reason aReason);
+  uint32_t GetTimeoutId(mozilla::dom::Timeout::Reason aReason);
 
   TimeDuration CalculateDelay(Timeout* aTimeout) const;
 
@@ -189,7 +189,7 @@ class TimeoutManager final {
       return false;
     }
 
-    Timeout* GetTimeout(int32_t aTimeoutId, Timeout::Reason aReason) {
+    Timeout* GetTimeout(uint32_t aTimeoutId, Timeout::Reason aReason) {
       Timeout::TimeoutIdAndReason key = {aTimeoutId, aReason};
       return mTimeouts->Get(key);
     }
@@ -222,7 +222,7 @@ class TimeoutManager final {
   RefPtr<TimeoutExecutor> mIdleExecutor;
   // The list of timeouts coming from non-tracking scripts.
   Timeouts mTimeouts;
-  int32_t mTimeoutIdCounter;
+  uint32_t mTimeoutIdCounter;
   uint32_t mNextFiringId;
 #ifdef DEBUG
   int64_t mFiringIndex;
@@ -236,7 +236,7 @@ class TimeoutManager final {
   Timeouts mIdleTimeouts;
 
   // The current idle request callback timeout handle
-  int32_t mIdleCallbackTimeoutCounter;
+  uint32_t mIdleCallbackTimeoutCounter;
 
   nsCOMPtr<nsITimer> mThrottleTimeoutsTimer;
   mozilla::TimeStamp mLastBudgetUpdate;

toolkit/components/cookiebanners/CookieBannerChild.sys.mjs

@@ -8,7 +8,6 @@ import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
 const lazy = {};
 
 ChromeUtils.defineESModuleGetters(lazy, {
-  setTimeout: "resource://gre/modules/Timer.sys.mjs",
   clearTimeout: "resource://gre/modules/Timer.sys.mjs",
   setInterval: "resource://gre/modules/Timer.sys.mjs",
   clearInterval: "resource://gre/modules/Timer.sys.mjs",
@@ -75,9 +74,8 @@ export class CookieBannerChild extends JSWindowActorChild {
   #isEnabledCached = null;
   #isTopLevel;
   #clickRules;
-  #givenUp = false;
+  #observerCleanUp;
   #observerCleanUpTimer;
-  #hasActiveObserver = false;
   // Indicates whether the page "load" event occurred.
   #didLoad = false;
   // Indicates whether we are using global rules to handle the banner.
@@ -321,7 +319,7 @@ export class CookieBannerChild extends JSWindowActorChild {
 
     lazy.logConsole.debug("Observed 'load' event", {
       href: this.document?.location.href,
-      hasActiveObserver: this.#hasActiveObserver,
+      hasActiveObserver: !!this.#observerCleanUp,
       observerCleanupTimer: this.#observerCleanUpTimer,
     });
 
@@ -349,11 +347,9 @@ export class CookieBannerChild extends JSWindowActorChild {
         "#startOrResetCleanupTimer: Cancelling existing cleanup timeout",
         {
           didLoad: this.#didLoad,
-          id: this.#observerCleanUpTimer,
         }
       );
       lazy.clearTimeout(this.#observerCleanUpTimer);
-      this.#observerCleanUpTimer = null;
     }
 
     let durationMS = this.#didLoad
@@ -364,27 +360,29 @@ export class CookieBannerChild extends JSWindowActorChild {
       {
         durationMS,
         didLoad: this.#didLoad,
+        hasObserverCleanup: !!this.#observerCleanUp,
       }
     );
 
-    this.#observerCleanUpTimer = lazy.setTimeout(() => {
+    this.#observerCleanUpTimer = this.contentWindow?.setTimeout(() => {
       lazy.logConsole.debug(
         "#startOrResetCleanupTimer: Cleanup timeout triggered",
         {
           durationMS,
           didLoad: this.#didLoad,
+          hasObserverCleanup: !!this.#observerCleanUp,
         }
       );
       this.#observerCleanUpTimer = null;
-      this.#givenUp = true;
+      this.#observerCleanUp?.();
     }, durationMS);
   }
 
   didDestroy() {
     this.#reportTelemetry();
 
-    // Cause the observer and polling function to be cleaned up.
-    this.#givenUp = true;
+    // Clean up the observer and timer if needed.
+    this.#observerCleanUp?.();
   }
 
   #reportTelemetry() {
@@ -411,7 +409,7 @@ export class CookieBannerChild extends JSWindowActorChild {
     } = this.#telemetryStatus;
 
     // Check if we got interrupted during an observe.
-    if (this.#hasActiveObserver && !success) {
+    if (this.#observerCleanUp && !success) {
       failReason = "actor_destroyed";
     }
 
@@ -590,12 +588,12 @@ export class CookieBannerChild extends JSWindowActorChild {
    * check function or null if the function times out.
    */
   #promiseObserve(checkFn) {
-    if (this.#hasActiveObserver) {
+    if (this.#observerCleanUp) {
       throw new Error(
         "The promiseObserve is called before previous one resolves."
       );
     }
-    this.#hasActiveObserver = true;
+    lazy.logConsole.debug("#promiseObserve", { didLoad: this.#didLoad });
 
     return new Promise(resolve => {
       let win = this.contentWindow;
@@ -619,15 +617,6 @@ export class CookieBannerChild extends JSWindowActorChild {
 
       // Start polling checkFn.
       let intervalFn = () => {
-        lazy.logConsole.debug(
-          "#promiseObserve interval function",
-          this.document?.location.href
-        );
-
-        if (this.#givenUp) {
-          cleanup(null);
-        }
-
         // Nothing changed since last run, skip running checkFn.
         if (!sawMutation) {
           return;
@@ -649,7 +638,6 @@ export class CookieBannerChild extends JSWindowActorChild {
           observer,
           cleanupTimeoutId: this.#observerCleanUpTimer,
           pollIntervalId,
-          href: this.document?.location.href,
         });
 
         // Unregister the observer.
@@ -664,9 +652,21 @@ export class CookieBannerChild extends JSWindowActorChild {
           pollIntervalId = null;
         }
 
-        this.#hasActiveObserver = false;
+        // Clear the cleanup timeout. This can happen when the actor gets
+        // destroyed before the cleanup timeout itself fires.
+        if (this.#observerCleanUpTimer) {
+          lazy.clearTimeout(this.#observerCleanUpTimer);
+        }
+
+        this.#observerCleanUp = null;
         resolve(result);
       };
+
+      // The clean up function to clean unfinished observer and timer on timeout
+      // or when the actor destroys.
+      this.#observerCleanUp = () => {
+        cleanup(null);
+      };
     });
   }