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 489486 e0debd3f26f6d4ff98c8ede64aa05111864c56d0
parent 489485 4ac8ee3c801972a1c47e8270dccc801612c3aa21
child 489487 7fa23df410e43bab5f4c264b7095fc36eee44f35
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersbz
bugs1494745
milestone64.0a1
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);
   }