Bug 1494745 part 2 - Make nsRefreshDriver::AddRefreshObserver void since it's infallible. r=bz
authorMats Palmgren <mats@mozilla.com>
Sun, 14 Oct 2018 18:12:22 +0200
changeset 441154 e0debd3f26f6d4ff98c8ede64aa05111864c56d0
parent 441153 4ac8ee3c801972a1c47e8270dccc801612c3aa21
child 441155 7fa23df410e43bab5f4c264b7095fc36eee44f35
push id108916
push usermpalmgren@mozilla.com
push dateSun, 14 Oct 2018 16:12:35 +0000
treeherdermozilla-inbound@2b7fce8c4f92 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1494745
milestone64.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 1494745 part 2 - Make nsRefreshDriver::AddRefreshObserver void since it's infallible. r=bz nsRefreshDriver::ObserverArray is a nsTObserverArray which is infallible, so no need to check the return value from AppendElement (which a later patch in this series will remove).
dom/ipc/CoalescedMouseData.cpp
layout/base/PresShell.cpp
layout/base/nsRefreshDriver.cpp
layout/base/nsRefreshDriver.h
layout/generic/nsGfxScrollFrame.cpp
--- a/dom/ipc/CoalescedMouseData.cpp
+++ b/dom/ipc/CoalescedMouseData.cpp
@@ -83,19 +83,17 @@ CoalescedMouseMoveFlusher::StartObserver
   nsRefreshDriver* refreshDriver = GetRefreshDriver();
   if (mRefreshDriver && mRefreshDriver == refreshDriver) {
     // Nothing to do if we already added an observer and it's same refresh driver.
     return;
   }
   RemoveObserver();
   if (refreshDriver) {
     mRefreshDriver = refreshDriver;
-    DebugOnly<bool> success =
-      mRefreshDriver->AddRefreshObserver(this, FlushType::Event);
-    MOZ_ASSERT(success);
+    mRefreshDriver->AddRefreshObserver(this, FlushType::Event);
   }
 }
 
 void
 CoalescedMouseMoveFlusher::RemoveObserver()
 {
   if (mRefreshDriver) {
     mRefreshDriver->RemoveRefreshObserver(this, FlushType::Event);
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -5558,22 +5558,18 @@ void PresShell::SynthesizeMouseMove(bool
 
   if (mMouseLocation == nsPoint(NS_UNCONSTRAINEDSIZE, NS_UNCONSTRAINEDSIZE))
     return;
 
   if (!mSynthMouseMoveEvent.IsPending()) {
     RefPtr<nsSynthMouseMoveEvent> ev =
         new nsSynthMouseMoveEvent(this, aFromScroll);
 
-    if (!GetPresContext()->RefreshDriver()
-                         ->AddRefreshObserver(ev, FlushType::Display)) {
-      NS_WARNING("failed to dispatch nsSynthMouseMoveEvent");
-      return;
-    }
-
+    GetPresContext()->RefreshDriver()
+                    ->AddRefreshObserver(ev, FlushType::Display);
     mSynthMouseMoveEvent = std::move(ev);
   }
 }
 
 /**
  * Find the first floating view with a widget in a postorder traversal of the
  * view tree that contains the point. Thus more deeply nested floating views
  * are preferred over their ancestors, and floating views earlier in the
@@ -9411,18 +9407,21 @@ PresShell::Observe(nsISupports* aSubject
   return NS_ERROR_FAILURE;
 }
 
 bool
 nsIPresShell::AddRefreshObserver(nsARefreshObserver* aObserver,
                                  FlushType aFlushType)
 {
   nsPresContext* presContext = GetPresContext();
-  return presContext &&
-      presContext->RefreshDriver()->AddRefreshObserver(aObserver, aFlushType);
+  if (MOZ_UNLIKELY(!presContext)) {
+    return false;
+  }
+  presContext->RefreshDriver()->AddRefreshObserver(aObserver, aFlushType);
+  return true;
 }
 
 bool
 nsIPresShell::RemoveRefreshObserver(nsARefreshObserver* aObserver,
                                     FlushType aFlushType)
 {
   nsPresContext* presContext = GetPresContext();
   return presContext &&
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -1199,24 +1199,23 @@ nsRefreshDriver::MostRecentRefresh() con
   // RestyleManager::ProcessPendingRestyles().
   if (!ServoStyleSet::IsInServoTraversal()) {
     const_cast<nsRefreshDriver*>(this)->EnsureTimerStarted();
   }
 
   return mMostRecentRefresh;
 }
 
-bool
+void
 nsRefreshDriver::AddRefreshObserver(nsARefreshObserver* aObserver,
                                     FlushType aFlushType)
 {
   ObserverArray& array = ArrayFor(aFlushType);
-  bool success = array.AppendElement(aObserver) != nullptr;
+  array.AppendElement(aObserver);
   EnsureTimerStarted();
-  return success;
 }
 
 bool
 nsRefreshDriver::RemoveRefreshObserver(nsARefreshObserver* aObserver,
                                        FlushType aFlushType)
 {
   ObserverArray& array = ArrayFor(aFlushType);
   return array.RemoveElement(aObserver);
--- a/layout/base/nsRefreshDriver.h
+++ b/layout/base/nsRefreshDriver.h
@@ -115,36 +115,36 @@ public:
    * used by callers who want to start an animation now and want to know
    * what time to consider the start of the animation.  (This helps
    * ensure that multiple animations started during the same event off
    * the main event loop have the same start time.)
    */
   mozilla::TimeStamp MostRecentRefresh() const;
 
   /**
-   * Add / remove refresh observers.  Returns whether the operation
-   * succeeded.
+   * Add / remove refresh observers.
+   * RemoveRefreshObserver returns true if aObserver was found.
    *
    * The flush type affects:
    *   + the order in which the observers are notified (lowest flush
    *     type to highest, in order registered)
    *   + (in the future) which observers are suppressed when the display
    *     doesn't require current position data or isn't currently
    *     painting, and, correspondingly, which get notified when there
    *     is a flush during such suppression
    * and it must be FlushType::Style, FlushType::Layout, or FlushType::Display.
    *
    * The refresh driver does NOT own a reference to these observers;
    * they must remove themselves before they are destroyed.
    *
    * The observer will be called even if there is no other activity.
    */
-  bool AddRefreshObserver(nsARefreshObserver *aObserver,
+  void AddRefreshObserver(nsARefreshObserver* aObserver,
                           mozilla::FlushType aFlushType);
-  bool RemoveRefreshObserver(nsARefreshObserver *aObserver,
+  bool RemoveRefreshObserver(nsARefreshObserver* aObserver,
                              mozilla::FlushType aFlushType);
   /**
    * Add / remove an observer wants to know the time when the refresh driver
    * updated the most recent refresh time due to its active timer changes.
    */
   bool AddTimerAdjustmentObserver(nsATimerAdjustmentObserver *aObserver);
   bool RemoveTimerAdjustmentObserver(nsATimerAdjustmentObserver *aObserver);
 
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -1836,27 +1836,22 @@ public:
     // The callback may release "this".
     // We don't access members after returning, so no need for KungFuDeathGrip.
     ScrollFrameHelper::AsyncSmoothMSDScrollCallback(mCallee, deltaTime);
   }
 
   /*
    * Set a refresh observer for smooth scroll iterations (and start observing).
    * Should be used at most once during the lifetime of this object.
-   * Return value: true on success, false otherwise.
    */
-  bool SetRefreshObserver(ScrollFrameHelper *aCallee) {
+  void SetRefreshObserver(ScrollFrameHelper* aCallee) {
     NS_ASSERTION(aCallee && !mCallee, "AsyncSmoothMSDScroll::SetRefreshObserver - Invalid usage.");
 
-    if (!RefreshDriver(aCallee)->AddRefreshObserver(this, FlushType::Style)) {
-      return false;
-    }
-
+    RefreshDriver(aCallee)->AddRefreshObserver(this, FlushType::Style);
     mCallee = aCallee;
-    return true;
   }
 
   /**
    * The mCallee holds a strong ref to us since the refresh driver doesn't.
    * Our dtor and mCallee's Destroy() method both call RemoveObserver() -
    * whichever comes first removes us from the refresh driver.
    */
   void RemoveObserver() {
@@ -1949,30 +1944,25 @@ private:
 // The next section is observer/callback management
 // Bodies of WillRefresh and RefreshDriver contain ScrollFrameHelper specific code.
 public:
   NS_INLINE_DECL_REFCOUNTING(AsyncScroll, override)
 
   /*
    * Set a refresh observer for smooth scroll iterations (and start observing).
    * Should be used at most once during the lifetime of this object.
-   * Return value: true on success, false otherwise.
    */
-  bool SetRefreshObserver(ScrollFrameHelper *aCallee) {
+  void SetRefreshObserver(ScrollFrameHelper* aCallee) {
     NS_ASSERTION(aCallee && !mCallee, "AsyncScroll::SetRefreshObserver - Invalid usage.");
 
-    if (!RefreshDriver(aCallee)->AddRefreshObserver(this, FlushType::Style)) {
-      return false;
-    }
-
+    RefreshDriver(aCallee)->AddRefreshObserver(this, FlushType::Style);
     mCallee = aCallee;
     nsIPresShell* shell = mCallee->mOuter->PresShell();
     MOZ_ASSERT(shell);
     shell->SuppressDisplayport(true);
-    return true;
   }
 
   virtual void WillRefresh(mozilla::TimeStamp aTime) override {
     // The callback may release "this".
     // We don't access members after returning, so no need for KungFuDeathGrip.
     ScrollFrameHelper::AsyncScrollCallback(mCallee, aTime);
   }
 
@@ -2465,21 +2455,17 @@ ScrollFrameHelper::ScrollToWithOrigin(ns
           return;
         }
 
         mAsyncSmoothMSDScroll =
           new AsyncSmoothMSDScroll(GetScrollPosition(), mDestination,
                                    currentVelocity, GetScrollRangeForClamping(),
                                    now, presContext);
 
-        if (!mAsyncSmoothMSDScroll->SetRefreshObserver(this)) {
-          // Observer setup failed. Scroll the normal way.
-          CompleteAsyncScroll(range, aOrigin);
-          return;
-        }
+        mAsyncSmoothMSDScroll->SetRefreshObserver(this);
       } else {
         // A previous smooth MSD scroll is still in progress, so we just need to
         // update its range and destination.
         mAsyncSmoothMSDScroll->SetRange(GetScrollRangeForClamping());
         mAsyncSmoothMSDScroll->SetDestination(mDestination);
       }
 
       return;
@@ -2488,21 +2474,17 @@ ScrollFrameHelper::ScrollToWithOrigin(ns
         currentVelocity = mAsyncSmoothMSDScroll->GetVelocity();
         mAsyncSmoothMSDScroll = nullptr;
       }
     }
   }
 
   if (!mAsyncScroll) {
     mAsyncScroll = new AsyncScroll();
-    if (!mAsyncScroll->SetRefreshObserver(this)) {
-      // Observer setup failed. Scroll the normal way.
-      CompleteAsyncScroll(range, aOrigin);
-      return;
-    }
+    mAsyncScroll->SetRefreshObserver(this);
   }
 
   if (isSmoothScroll) {
     mAsyncScroll->InitSmoothScroll(now, GetScrollPosition(), mDestination,
                                    aOrigin, range, currentVelocity);
   } else {
     mAsyncScroll->Init(range);
   }