Bug 1318720 - Prevent chaining idle callbacks in the same period. r=smaug
authorAndreas Farre <farre@mozilla.com>
Mon, 27 Feb 2017 15:00:24 +0100
changeset 348679 9ba17600e3c9ce1cd42eec7e95287293b7af0026
parent 348678 c3831ae7c9ffb3129955a8db0916efb03b0ea910
child 348680 58d628a5c367c6c6f1b092988b6b3ee4054bf1f5
push id31533
push userkwierso@gmail.com
push dateTue, 21 Mar 2017 23:08:53 +0000
treeherdermozilla-central@8744e9f8eb99 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1318720
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1318720 - Prevent chaining idle callbacks in the same period. r=smaug MozReview-Commit-ID: H3lvpNi9Lx9
dom/base/nsGlobalWindow.cpp
dom/base/nsGlobalWindow.h
testing/web-platform/meta/html/webappapis/idle-callbacks/callback-idle-periods.html.ini
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -523,43 +523,100 @@ DialogValueHolder::Get(JSContext* aCx, J
   if (aSubject->Subsumes(mOrigin)) {
     aError = nsContentUtils::XPConnect()->VariantToJS(aCx, aScope,
                                                       mValue, aResult);
   } else {
     aResult.setUndefined();
   }
 }
 
+class IdleRequestExecutor;
+
+class IdleRequestExecutorTimeoutHandler final : public TimeoutHandler
+{
+public:
+  explicit IdleRequestExecutorTimeoutHandler(IdleRequestExecutor* aExecutor)
+    : mExecutor(aExecutor)
+  {
+  }
+
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(IdleRequestExecutorTimeoutHandler,
+                                           TimeoutHandler)
+
+  nsresult Call() override;
+
+private:
+  ~IdleRequestExecutorTimeoutHandler() {}
+  RefPtr<IdleRequestExecutor> mExecutor;
+};
+
+NS_IMPL_CYCLE_COLLECTION_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler, mExecutor)
+
+NS_IMPL_ADDREF_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler)
+NS_IMPL_RELEASE_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler)
+
+NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IdleRequestExecutorTimeoutHandler)
+NS_INTERFACE_MAP_END_INHERITING(TimeoutHandler)
+
+
 class IdleRequestExecutor final : public nsIRunnable
                                 , public nsICancelableRunnable
                                 , public nsIIncrementalRunnable
 {
 public:
   explicit IdleRequestExecutor(nsGlobalWindow* aWindow)
     : mDispatched(false)
     , mDeadline(TimeStamp::Now())
     , mWindow(aWindow)
   {
     MOZ_DIAGNOSTIC_ASSERT(mWindow);
     MOZ_DIAGNOSTIC_ASSERT(mWindow->IsInnerWindow());
+
+    mIdlePeriodLimit = { mDeadline, mWindow->LastIdleRequestHandle() };
   }
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(IdleRequestExecutor, nsIRunnable)
 
   NS_DECL_NSIRUNNABLE
   nsresult Cancel() override;
   void SetDeadline(TimeStamp aDeadline) override;
 
-  void MaybeDispatch();
+  bool IsCancelled() const { return !mWindow; }
+  // Checks if aRequest shouldn't execute in the current idle period
+  // since it has been queued from a chained call to
+  // requestIdleCallback from within a running idle callback.
+  bool IneligibleForCurrentIdlePeriod(IdleRequest* aRequest) const
+  {
+    return aRequest->Handle() >= mIdlePeriodLimit.mLastRequestIdInIdlePeriod &&
+           TimeStamp::Now() <= mIdlePeriodLimit.mEndOfIdlePeriod;
+  }
+
+  void MaybeUpdateIdlePeriodLimit();
+
+  // Maybe dispatch the IdleRequestExecutor. MabyeDispatch will
+  // schedule a throttled dispatch if the associated window is in the
+  // background or if given a time to wait until dispatching.
+  void MaybeDispatch(TimeStamp aDelayUntil = TimeStamp());
+  void ScheduleDispatch();
 private:
+  struct IdlePeriodLimit
+  {
+    TimeStamp mEndOfIdlePeriod;
+    uint32_t mLastRequestIdInIdlePeriod;
+  };
+
+  void ThrottledDispatch(uint32_t aDelay);
+
   ~IdleRequestExecutor() {}
 
   bool mDispatched;
   TimeStamp mDeadline;
+  IdlePeriodLimit mIdlePeriodLimit;
   RefPtr<nsGlobalWindow> mWindow;
 };
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(IdleRequestExecutor)
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(IdleRequestExecutor)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(IdleRequestExecutor)
 
@@ -608,92 +665,88 @@ IdleRequestExecutor::SetDeadline(TimeSta
   if (!mWindow) {
     return;
   }
 
   mDeadline = aDeadline;
 }
 
 void
-IdleRequestExecutor::MaybeDispatch()
+IdleRequestExecutor::MaybeUpdateIdlePeriodLimit()
+{
+  if (TimeStamp::Now() > mIdlePeriodLimit.mEndOfIdlePeriod) {
+    mIdlePeriodLimit = { mDeadline, mWindow->LastIdleRequestHandle() };
+  }
+}
+
+void
+IdleRequestExecutor::MaybeDispatch(TimeStamp aDelayUntil)
 {
   // If we've already dispatched the executor we don't want to do it
   // again. Also, if we've called IdleRequestExecutor::Cancel mWindow
   // will be null, which indicates that we shouldn't dispatch this
   // executor either.
   if (mDispatched || !mWindow) {
     return;
   }
 
   mDispatched = true;
+
+  nsPIDOMWindowOuter* outer = mWindow->GetOuterWindow();
+  if (outer && outer->AsOuter()->IsBackground()) {
+    // Set a timeout handler with a timeout of 0 ms to throttle idle
+    // callback requests coming from a backround window using
+    // background timeout throttling.
+    ThrottledDispatch(0);
+  } else if (aDelayUntil) {
+    ThrottledDispatch(
+      static_cast<uint32_t>((aDelayUntil - TimeStamp::Now()).ToMilliseconds()));
+  } else {
+    ScheduleDispatch();
+  }
+}
+
+void
+IdleRequestExecutor::ScheduleDispatch()
+{
+  MOZ_ASSERT(mWindow);
   RefPtr<IdleRequestExecutor> request = this;
   NS_IdleDispatchToCurrentThread(request.forget());
 }
 
-class IdleRequestExecutorTimeoutHandler final : public TimeoutHandler
-{
-public:
-  explicit IdleRequestExecutorTimeoutHandler(IdleRequestExecutor* aExecutor)
-    : mExecutor(aExecutor)
-  {
-  }
-
-  NS_DECL_ISUPPORTS_INHERITED
-  NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(IdleRequestExecutorTimeoutHandler,
-                                           TimeoutHandler)
-
-  nsresult Call() override
-  {
-    mExecutor->MaybeDispatch();
-    return NS_OK;
-  }
-private:
-  ~IdleRequestExecutorTimeoutHandler() {}
-  RefPtr<IdleRequestExecutor> mExecutor;
-};
-
-NS_IMPL_CYCLE_COLLECTION_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler, mExecutor)
-
-NS_IMPL_ADDREF_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler)
-NS_IMPL_RELEASE_INHERITED(IdleRequestExecutorTimeoutHandler, TimeoutHandler)
-
-NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IdleRequestExecutorTimeoutHandler)
-  NS_INTERFACE_MAP_ENTRY(nsITimeoutHandler)
-NS_INTERFACE_MAP_END_INHERITING(TimeoutHandler)
+void
+IdleRequestExecutor::ThrottledDispatch(uint32_t aDelay)
+{
+  MOZ_ASSERT(mWindow);
+
+  nsCOMPtr<nsITimeoutHandler> handler =
+    new IdleRequestExecutorTimeoutHandler(this);
+  int32_t dummy;
+  mWindow->AsInner()->TimeoutManager().SetTimeout(
+    handler, aDelay, false, Timeout::Reason::eIdleCallbackTimeout, &dummy);
+}
+
+nsresult
+IdleRequestExecutorTimeoutHandler::Call()
+{
+  if (!mExecutor->IsCancelled()) {
+    mExecutor->ScheduleDispatch();
+  }
+  return NS_OK;
+}
 
 void
 nsGlobalWindow::ScheduleIdleRequestDispatch()
 {
   AssertIsOnMainThread();
 
-  if (mIdleRequestCallbacks.isEmpty()) {
-    if (mIdleRequestExecutor) {
-      mIdleRequestExecutor->Cancel();
-      mIdleRequestExecutor = nullptr;
-    }
-
-    return;
-  }
-
   if (!mIdleRequestExecutor) {
     mIdleRequestExecutor = new IdleRequestExecutor(this);
   }
 
-  nsPIDOMWindowOuter* outer = GetOuterWindow();
-  if (outer && outer->AsOuter()->IsBackground()) {
-    nsCOMPtr<nsITimeoutHandler> handler = new IdleRequestExecutorTimeoutHandler(mIdleRequestExecutor);
-    int32_t dummy;
-    // Set a timeout handler with a timeout of 0 ms to throttle idle
-    // callback requests coming from a backround window using
-    // background timeout throttling.
-    mTimeoutManager->SetTimeout(handler, 0, false,
-                                Timeout::Reason::eIdleCallbackTimeout, &dummy);
-    return;
-  }
-
   mIdleRequestExecutor->MaybeDispatch();
 }
 
 void
 nsGlobalWindow::SuspendIdleRequests()
 {
   if (mIdleRequestExecutor) {
     mIdleRequestExecutor->Cancel();
@@ -749,25 +802,34 @@ nsGlobalWindow::ExecuteIdleRequest(TimeS
   RefPtr<IdleRequest> request = mIdleRequestCallbacks.getFirst();
 
   if (!request) {
     // There are no more idle requests, so stop scheduling idle
     // request callbacks.
     return NS_OK;
   }
 
+  // If the request that we're trying to execute has been queued
+  // during the current idle period, then dispatch it again at the end
+  // of the idle period.
+  if (mIdleRequestExecutor->IneligibleForCurrentIdlePeriod(request)) {
+    mIdleRequestExecutor->MaybeDispatch(aDeadline);
+    return NS_OK;
+  }
+
   DOMHighResTimeStamp deadline = 0.0;
 
   if (Performance* perf = GetPerformance()) {
     deadline = perf->GetDOMTiming()->TimeStampToDOMHighRes(aDeadline);
   }
 
+  mIdleRequestExecutor->MaybeUpdateIdlePeriodLimit();
   nsresult result = RunIdleRequest(request, deadline, false);
 
-  ScheduleIdleRequestDispatch();
+  mIdleRequestExecutor->MaybeDispatch();
   return result;
 }
 
 class IdleRequestTimeoutHandler final : public TimeoutHandler
 {
 public:
   IdleRequestTimeoutHandler(JSContext* aCx,
                             IdleRequest* aIdleRequest,
@@ -798,17 +860,16 @@ NS_IMPL_CYCLE_COLLECTION_INHERITED(IdleR
                                    TimeoutHandler,
                                    mIdleRequest,
                                    mWindow)
 
 NS_IMPL_ADDREF_INHERITED(IdleRequestTimeoutHandler, TimeoutHandler)
 NS_IMPL_RELEASE_INHERITED(IdleRequestTimeoutHandler, TimeoutHandler)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IdleRequestTimeoutHandler)
-  NS_INTERFACE_MAP_ENTRY(nsITimeoutHandler)
 NS_INTERFACE_MAP_END_INHERITING(TimeoutHandler)
 
 uint32_t
 nsGlobalWindow::RequestIdleCallback(JSContext* aCx,
                                     IdleRequestCallback& aCallback,
                                     const IdleRequestOptions& aOptions,
                                     ErrorResult& aError)
 {
@@ -830,25 +891,20 @@ nsGlobalWindow::RequestIdleCallback(JSCo
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return 0;
     }
 
     request->SetTimeoutHandle(timeoutHandle);
   }
 
-  // If the list of idle callback requests is not empty it means that
-  // we've already dispatched the first idle request. If we're
-  // suspended we should only queue the idle callback and not schedule
-  // it to run, that will be done in ResumeIdleRequest.
-  bool needsScheduling = !IsSuspended() && mIdleRequestCallbacks.isEmpty();
   // mIdleRequestCallbacks now owns request
   InsertIdleCallback(request);
 
-  if (needsScheduling) {
+  if (!IsSuspended()) {
     ScheduleIdleRequestDispatch();
   }
 
   return handle;
 }
 
 void
 nsGlobalWindow::CancelIdleCallback(uint32_t aHandle)
--- a/dom/base/nsGlobalWindow.h
+++ b/dom/base/nsGlobalWindow.h
@@ -1819,17 +1819,17 @@ public:
 
   virtual nsIEventTarget*
   EventTargetFor(mozilla::TaskCategory aCategory) const override;
 
   virtual mozilla::AbstractThread*
   AbstractMainThreadFor(mozilla::TaskCategory aCategory) override;
 
   void DisableIdleCallbackRequests();
-  uint32_t IdleRequestHandle() const { return mIdleRequestCallbackCounter; }
+  uint32_t LastIdleRequestHandle() const { return mIdleRequestCallbackCounter - 1; }
   nsresult RunIdleRequest(mozilla::dom::IdleRequest* aRequest,
                           DOMHighResTimeStamp aDeadline, bool aDidTimeout);
   nsresult ExecuteIdleRequest(TimeStamp aDeadline);
   void ScheduleIdleRequestDispatch();
   void SuspendIdleRequests();
   void ResumeIdleRequests();
 
   typedef mozilla::LinkedList<mozilla::dom::IdleRequest> IdleRequests;
deleted file mode 100644
--- a/testing/web-platform/meta/html/webappapis/idle-callbacks/callback-idle-periods.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[callback-idle-periods.html]
-  type: testharness
-  [Check that if an idle callback calls requestIdleCallback the new callback doesn't run in the current idle period.]
-    expected: FAIL
-