Bug 1681030 - Fix some regressions from bug 1645528, r=emilio

This fixes a bunch of regressions:
 - a wrong calculation in `GetIdleDeadlineHint()`, leading to pageload
regressions.
 - in certain situations we'd use `StartupRefreshDriverTimer` instead
of `VsyncRefreshDriverTimer` when initializing timers early
 - unnecessary use of `BrowserChild` on backends that don't opt for
per-browser-child vsync - i.e. all but Wayland.

This is partly done by reverting to pre-1645528 behaviour, although
with some code simplifications.

FTR: I also played with some more radical changes, but given the
complexity of the code involved I found the regression potential too
big. Thus this is the most conservative solution I could come up with.

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

dom/ipc/BrowserChild.cpp

@@ -553,8 +553,6 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
   NS_ENSURE_SUCCESS(rv, rv);
 #endif
 
-  InitVsyncChild();
-
   // We've all set up, make sure our visibility state is consistent. This is
   // important for OOP iframes, which start off as hidden.
   UpdateVisibility();
@@ -562,20 +560,6 @@ nsresult BrowserChild::Init(mozIDOMWindowProxy* aParent,
   return NS_OK;
 }
 
-void BrowserChild::InitVsyncChild() {
-#if defined(MOZ_WAYLAND)
-  if (!IsWaylandDisabled()) {
-    PVsyncChild* actor = SendPVsyncConstructor();
-    mVsyncChild = static_cast<VsyncChild*>(actor);
-  } else
-#endif
-  {
-    PBackgroundChild* actorChild =
-        BackgroundChild::GetOrCreateForCurrentThread();
-    mVsyncChild = static_cast<VsyncChild*>(actorChild->SendPVsyncConstructor());
-  }
-}
-
 void BrowserChild::NotifyTabContextUpdated() {
   nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation());
   MOZ_ASSERT(docShell);
@@ -2188,7 +2172,18 @@ bool BrowserChild::DeallocPVsyncChild(PVsyncChild* aActor) {
   return true;
 }
 
-RefPtr<VsyncChild> BrowserChild::GetVsyncChild() { return mVsyncChild; }
+RefPtr<VsyncChild> BrowserChild::GetVsyncChild() {
+  // Initializing mVsyncChild here turns on per-BrowserChild Vsync for a
+  // given platform. Note: this only makes sense if nsWindow returns a
+  // window-specific VsyncSource.
+#if defined(MOZ_WAYLAND)
+  if (!IsWaylandDisabled() && !mVsyncChild) {
+    PVsyncChild* actor = SendPVsyncConstructor();
+    mVsyncChild = static_cast<VsyncChild*>(actor);
+  }
+#endif
+  return mVsyncChild;
+}
 
 mozilla::ipc::IPCResult BrowserChild::RecvActivateFrameEvent(
     const nsString& aType, const bool& capture) {

dom/ipc/BrowserChild.h

@@ -755,8 +755,6 @@ class BrowserChild final : public nsMessageManagerScriptExecutor,
       const mozilla::layers::CompositorOptions& aCompositorOptions);
   void InitAPZState();
 
-  void InitVsyncChild();
-
   void DestroyWindow();
 
   void ApplyParentShowInfo(const ParentShowInfo&);

layout/base/nsRefreshDriver.cpp

@@ -981,7 +981,6 @@ static StaticRefPtr<InactiveRefreshDriverTimer> sThrottledRateTimer;
 
 void nsRefreshDriver::CreateVsyncRefreshTimer() {
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(!mOwnTimer);
 
   PodArrayZero(sJankLevels);
 
@@ -989,29 +988,52 @@ void nsRefreshDriver::CreateVsyncRefreshTimer() {
     return;
   }
 
-  // If available, we fetch the widget-specific vsync source.
-  nsPresContext* pc = GetPresContext();
-  nsIWidget* widget = pc->GetRootWidget();
-  if (widget) {
-    if (RefPtr<gfx::VsyncSource> localVsyncSource = widget->GetVsyncSource()) {
-      mOwnTimer = new VsyncRefreshDriverTimer(localVsyncSource);
-      sRegularRateTimerList->AppendElement(mOwnTimer.get());
-      return;
-    }
-    if (BrowserChild* browserChild = widget->GetOwningBrowserChild()) {
-      if (RefPtr<VsyncChild> localVsyncSource = browserChild->GetVsyncChild()) {
+  if (!mOwnTimer) {
+    // If available, we fetch the widget-specific vsync source.
+    nsPresContext* pc = GetPresContext();
+    nsIWidget* widget = pc->GetRootWidget();
+    if (widget) {
+      if (RefPtr<gfx::VsyncSource> localVsyncSource =
+              widget->GetVsyncSource()) {
         mOwnTimer = new VsyncRefreshDriverTimer(localVsyncSource);
         sRegularRateTimerList->AppendElement(mOwnTimer.get());
         return;
       }
+      if (BrowserChild* browserChild = widget->GetOwningBrowserChild()) {
+        if (RefPtr<VsyncChild> localVsyncSource =
+                browserChild->GetVsyncChild()) {
+          mOwnTimer = new VsyncRefreshDriverTimer(localVsyncSource);
+          sRegularRateTimerList->AppendElement(mOwnTimer.get());
+          return;
+        }
+      }
     }
   }
-  if (!sRegularRateTimer && XRE_IsParentProcess()) {
-    // Make sure all vsync systems are ready.
-    gfxPlatform::GetPlatform();
-    // In parent process, we can create the VsyncRefreshDriverTimer directly.
-    sRegularRateTimer = new VsyncRefreshDriverTimer();
-    sRegularRateTimerList->AppendElement(sRegularRateTimer);
+  if (!sRegularRateTimer) {
+    if (XRE_IsParentProcess()) {
+      // Make sure all vsync systems are ready.
+      gfxPlatform::GetPlatform();
+      // In parent process, we can create the VsyncRefreshDriverTimer directly.
+      sRegularRateTimer = new VsyncRefreshDriverTimer();
+    } else {
+      PBackgroundChild* actorChild =
+          BackgroundChild::GetOrCreateForCurrentThread();
+      if (NS_WARN_IF(!actorChild)) {
+        return;
+      }
+
+      dom::PVsyncChild* actor = actorChild->SendPVsyncConstructor();
+      if (NS_WARN_IF(!actor)) {
+        return;
+      }
+
+      dom::VsyncChild* child = static_cast<dom::VsyncChild*>(actor);
+
+      RefPtr<RefreshDriverTimer> vsyncRefreshDriverTimer =
+          new VsyncRefreshDriverTimer(child);
+
+      sRegularRateTimer = std::move(vsyncRefreshDriverTimer);
+    }
   }
 }
 
@@ -1101,7 +1123,6 @@ RefreshDriverTimer* nsRefreshDriver::ChooseTimer() {
   if (!sRegularRateTimer) {
     double rate = GetRegularTimerInterval();
     sRegularRateTimer = new StartupRefreshDriverTimer(rate);
-    sRegularRateTimerList->AppendElement(sRegularRateTimer);
   }
 
   return sRegularRateTimer;
@@ -2713,30 +2734,47 @@ TimeStamp nsRefreshDriver::GetIdleDeadlineHint(TimeStamp aDefault) {
   // resulting from a tick on the sRegularRateTimer counts as being
   // busy but tasks resulting from a tick on sThrottledRateTimer
   // counts as being idle.
-  TimeStamp hint = aDefault;
+  if (sRegularRateTimer) {
+    return sRegularRateTimer->GetIdleDeadlineHint(aDefault);
+  }
+
+  // The following calculation is only used on platform using per-BrowserChild
+  // Vsync. This is hard to properly map on static calls such as this -
+  // optimally we'd only want to query the timers that are relevant for the
+  // caller, not all in this process. Further more, in this scenario we often
+  // hit cases where timers would return their fallback value that is aDefault,
+  // giving us a much higher value than intended.
+  // For now we use a somewhat simplistic approach that in many situations
+  // gives us similar behaviour to what we would get using sRegularRateTimer:
+  // use the highest result that is still lower than the aDefault fallback.
+  TimeStamp hint = TimeStamp();
   if (sRegularRateTimerList) {
     for (RefreshDriverTimer* timer : *sRegularRateTimerList) {
       TimeStamp newHint = timer->GetIdleDeadlineHint(aDefault);
-      if (newHint > hint) {
+      if (newHint < aDefault && (hint.IsNull() || newHint > hint)) {
         hint = newHint;
       }
     }
   }
-  return hint;
+
+  return hint.IsNull() ? aDefault : hint;
 }
 
 /* static */
 Maybe<TimeStamp> nsRefreshDriver::GetNextTickHint() {
   MOZ_ASSERT(NS_IsMainThread());
 
+  if (sRegularRateTimer) {
+    return sRegularRateTimer->GetNextTickHint();
+  }
+
   Maybe<TimeStamp> hint = Nothing();
   if (sRegularRateTimerList) {
     for (RefreshDriverTimer* timer : *sRegularRateTimerList) {
-      Maybe<TimeStamp> newHint = timer->GetNextTickHint();
-      if ((newHint.isSome() && hint.isNothing()) ||
-          (newHint.isSome() && hint.isSome() &&
-           newHint.value() > hint.value())) {
-        hint = newHint;
+      if (Maybe<TimeStamp> newHint = timer->GetNextTickHint()) {
+        if (!hint || newHint.value() < hint.value()) {
+          hint = newHint;
+        }
       }
     }
   }