Bug 1326290 - Correct sequencing of layer transaction and StartAsyncScrollbarDrag messages. r=kats draft
authorBotond Ballo <botond@mozilla.com>
Wed, 04 Jan 2017 13:42:36 -0500
changeset 456022 3517e8c8a578a0bd257a80bb8cb81303d171bb6c
parent 454222 a6d29e9432f5f88a941c0ea5284cb082f34bd097
child 541101 1d92375c59ca0b1f3f97fb55e7d6478c7a09cb7f
push id40358
push userbballo@mozilla.com
push dateWed, 04 Jan 2017 18:44:15 +0000
reviewerskats
bugs1326290
milestone53.0a1
Bug 1326290 - Correct sequencing of layer transaction and StartAsyncScrollbarDrag messages. r=kats In cases where a mouse click that starts a scrollbar drag is also what layerizes the scroll frame, the StartAsyncScrollbarDrag message needs to arrive after the layer transaction. This patch ensures it does. MozReview-Commit-ID: A02qRb6yWxg
dom/ipc/TabChild.cpp
gfx/layers/apz/util/APZCCallbackHelper.cpp
gfx/layers/apz/util/APZCCallbackHelper.h
gfx/layers/apz/util/InputAPZContext.cpp
gfx/layers/apz/util/InputAPZContext.h
layout/xul/nsSliderFrame.cpp
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1590,24 +1590,28 @@ TabChild::RecvRealMouseButtonEvent(const
                                    const uint64_t& aInputBlockId)
 {
   // Mouse events like eMouseEnterIntoWidget, that are created in the parent
   // process EventStateManager code, have an input block id which they get from
   // the InputAPZContext in the parent process stack. However, they did not
   // actually go through the APZ code and so their mHandledByAPZ flag is false.
   // Since thos events didn't go through APZ, we don't need to send notifications
   // for them.
+  bool pendingLayerization = false;
   if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
     nsCOMPtr<nsIDocument> document(GetDocument());
-    APZCCallbackHelper::SendSetTargetAPZCNotification(
+    pendingLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification(
       mPuppetWidget, document, aEvent, aGuid, aInputBlockId);
   }
 
   nsEventStatus unused;
   InputAPZContext context(aGuid, aInputBlockId, unused);
+  if (pendingLayerization) {
+    context.SetPendingLayerization();
+  }
 
   WidgetMouseEvent localEvent(aEvent);
   localEvent.mWidget = mPuppetWidget;
   APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid,
       mPuppetWidget->GetDefaultScale());
   APZCCallbackHelper::DispatchWidgetEvent(localEvent);
 
   if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -740,34 +740,34 @@ SendSetTargetAPZCNotificationHelper(nsIW
   if (!waitForRefresh) {
     APZCCH_LOG("Sending target APZCs for input block %" PRIu64 "\n", aInputBlockId);
     aWidget->SetConfirmedTargetAPZC(aInputBlockId, aTargets);
   } else {
     APZCCH_LOG("Successfully registered post-refresh observer\n");
   }
 }
 
-void
+bool
 APZCCallbackHelper::SendSetTargetAPZCNotification(nsIWidget* aWidget,
                                                   nsIDocument* aDocument,
                                                   const WidgetGUIEvent& aEvent,
                                                   const ScrollableLayerGuid& aGuid,
                                                   uint64_t aInputBlockId)
 {
   if (!aWidget || !aDocument) {
-    return;
+    return false;
   }
   if (aInputBlockId == sLastTargetAPZCNotificationInputBlock) {
     // We have already confirmed the target APZC for a previous event of this
     // input block. If we activated a scroll frame for this input block,
     // sending another target APZC confirmation would be harmful, as it might
     // race the original confirmation (which needs to go through a layers
     // transaction).
     APZCCH_LOG("Not resending target APZC confirmation for input block %" PRIu64 "\n", aInputBlockId);
-    return;
+    return false;
   }
   sLastTargetAPZCNotificationInputBlock = aInputBlockId;
   if (nsIPresShell* shell = aDocument->GetShell()) {
     if (nsIFrame* rootFrame = shell->GetRootFrame()) {
       rootFrame = UpdateRootFrameForTouchTargetDocument(rootFrame);
 
       bool waitForRefresh = false;
       nsTArray<ScrollableLayerGuid> targets;
@@ -789,18 +789,21 @@ APZCCallbackHelper::SendSetTargetAPZCNot
       if (!targets.IsEmpty()) {
         SendSetTargetAPZCNotificationHelper(
           aWidget,
           shell,
           aInputBlockId,
           Move(targets),
           waitForRefresh);
       }
+
+      return waitForRefresh;
     }
   }
+  return false;
 }
 
 void
 APZCCallbackHelper::SendSetAllowedTouchBehaviorNotification(
         nsIWidget* aWidget,
         nsIDocument* aDocument,
         const WidgetTouchEvent& aEvent,
         uint64_t aInputBlockId,
--- a/gfx/layers/apz/util/APZCCallbackHelper.h
+++ b/gfx/layers/apz/util/APZCCallbackHelper.h
@@ -133,18 +133,22 @@ public:
     /* Perform hit-testing on the touch points of |aEvent| to determine
      * which scrollable frames they target. If any of these frames don't have
      * a displayport, set one.
      *
      * If any displayports need to be set, the actual notification to APZ is
      * sent to the compositor, which will then post a message back to APZ's
      * controller thread. Otherwise, the provided widget's SetConfirmedTargetAPZC
      * method is invoked immediately.
+     *
+     * Returns true if any displayports need to be set. (A caller may be
+     * interested to know this, because they may need to delay certain actions
+     * until after the displayport comes into effect.)
      */
-    static void SendSetTargetAPZCNotification(nsIWidget* aWidget,
+    static bool SendSetTargetAPZCNotification(nsIWidget* aWidget,
                                               nsIDocument* aDocument,
                                               const WidgetGUIEvent& aEvent,
                                               const ScrollableLayerGuid& aGuid,
                                               uint64_t aInputBlockId);
 
     /* Figure out the allowed touch behaviors of each touch point in |aEvent|
      * and send that information to the provided callback. */
     static void SendSetAllowedTouchBehaviorNotification(nsIWidget* aWidget,
--- a/gfx/layers/apz/util/InputAPZContext.cpp
+++ b/gfx/layers/apz/util/InputAPZContext.cpp
@@ -7,16 +7,17 @@
 
 namespace mozilla {
 namespace layers {
 
 ScrollableLayerGuid InputAPZContext::sGuid;
 uint64_t InputAPZContext::sBlockId = 0;
 nsEventStatus InputAPZContext::sApzResponse = nsEventStatus_eIgnore;
 bool InputAPZContext::sRoutedToChildProcess = false;
+bool InputAPZContext::sPendingLayerization = false;
 
 /*static*/ ScrollableLayerGuid
 InputAPZContext::GetTargetLayerGuid()
 {
   return sGuid;
 }
 
 /*static*/ uint64_t
@@ -32,38 +33,53 @@ InputAPZContext::GetApzResponse()
 }
 
 /*static*/ void
 InputAPZContext::SetRoutedToChildProcess()
 {
   sRoutedToChildProcess = true;
 }
 
+/*static*/ void
+InputAPZContext::SetPendingLayerization()
+{
+  sPendingLayerization = true;
+}
+
 InputAPZContext::InputAPZContext(const ScrollableLayerGuid& aGuid,
                                  const uint64_t& aBlockId,
                                  const nsEventStatus& aApzResponse)
   : mOldGuid(sGuid)
   , mOldBlockId(sBlockId)
   , mOldApzResponse(sApzResponse)
   , mOldRoutedToChildProcess(sRoutedToChildProcess)
+  , mOldPendingLayerization(sPendingLayerization)
 {
   sGuid = aGuid;
   sBlockId = aBlockId;
   sApzResponse = aApzResponse;
   sRoutedToChildProcess = false;
+  sPendingLayerization = false;
 }
 
 InputAPZContext::~InputAPZContext()
 {
   sGuid = mOldGuid;
   sBlockId = mOldBlockId;
   sApzResponse = mOldApzResponse;
   sRoutedToChildProcess = mOldRoutedToChildProcess;
+  sPendingLayerization = mOldPendingLayerization;
 }
 
-bool
+/*static*/ bool
 InputAPZContext::WasRoutedToChildProcess()
 {
   return sRoutedToChildProcess;
 }
 
+/*static*/ bool
+InputAPZContext::HavePendingLayerization()
+{
+  return sPendingLayerization;
+}
+
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/apz/util/InputAPZContext.h
+++ b/gfx/layers/apz/util/InputAPZContext.h
@@ -18,33 +18,37 @@ namespace layers {
 // that has been processed by APZ directly from a widget.
 class MOZ_STACK_CLASS InputAPZContext
 {
 private:
   static ScrollableLayerGuid sGuid;
   static uint64_t sBlockId;
   static nsEventStatus sApzResponse;
   static bool sRoutedToChildProcess;
+  static bool sPendingLayerization;
 
 public:
   static ScrollableLayerGuid GetTargetLayerGuid();
   static uint64_t GetInputBlockId();
   static nsEventStatus GetApzResponse();
   static void SetRoutedToChildProcess();
+  static void SetPendingLayerization();
 
   InputAPZContext(const ScrollableLayerGuid& aGuid,
                   const uint64_t& aBlockId,
                   const nsEventStatus& aApzResponse);
   ~InputAPZContext();
 
-  bool WasRoutedToChildProcess();
+  static bool WasRoutedToChildProcess();
+  static bool HavePendingLayerization();
 
 private:
   ScrollableLayerGuid mOldGuid;
   uint64_t mOldBlockId;
   nsEventStatus mOldApzResponse;
   bool mOldRoutedToChildProcess;
+  bool mOldPendingLayerization;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif /* mozilla_layers_InputAPZContext_h */
--- a/layout/xul/nsSliderFrame.cpp
+++ b/layout/xul/nsSliderFrame.cpp
@@ -32,16 +32,17 @@
 #include "nsScrollbarFrame.h"
 #include "nsRepeatService.h"
 #include "nsBoxLayoutState.h"
 #include "nsSprocketLayout.h"
 #include "nsIServiceManager.h"
 #include "nsContentUtils.h"
 #include "nsLayoutUtils.h"
 #include "nsDisplayList.h"
+#include "nsRefreshDriver.h"            // for nsAPostRefreshObserver
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT
 #include "mozilla/Preferences.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/layers/APZCCallbackHelper.h"
 #include "mozilla/layers/AsyncDragMetrics.h"
 #include "mozilla/layers/InputAPZContext.h"
@@ -924,16 +925,53 @@ nsSliderMediator::HandleEvent(nsIDOMEven
 {
   // Only process the event if the thumb is not being dragged.
   if (mSlider && !mSlider->isDraggingThumb())
     return mSlider->StartDrag(aEvent);
 
   return NS_OK;
 }
 
+class AsyncScrollbarDragStarter : public nsAPostRefreshObserver {
+public:
+  AsyncScrollbarDragStarter(nsIPresShell* aPresShell,
+                            nsIWidget* aWidget,
+                            const AsyncDragMetrics& aDragMetrics)
+    : mPresShell(aPresShell)
+    , mWidget(aWidget)
+    , mDragMetrics(aDragMetrics)
+  {
+  }
+  virtual ~AsyncScrollbarDragStarter() {}
+
+  void DidRefresh() override {
+    if (!mPresShell) {
+      MOZ_ASSERT_UNREACHABLE("Post-refresh observer fired again after failed attempt at unregistering it");
+      return;
+    }
+
+    mWidget->StartAsyncScrollbarDrag(mDragMetrics);
+
+    if (!mPresShell->RemovePostRefreshObserver(this)) {
+      MOZ_ASSERT_UNREACHABLE("Unable to unregister post-refresh observer! Leaking it instead of leaving garbage registered");
+      // Graceful handling, just in case...
+      mPresShell = nullptr;
+      mWidget = nullptr;
+      return;
+    }
+
+    delete this;
+  }
+
+private:
+  RefPtr<nsIPresShell> mPresShell;
+  RefPtr<nsIWidget> mWidget;
+  AsyncDragMetrics mDragMetrics;
+};
+
 bool
 nsSliderFrame::StartAPZDrag(WidgetGUIEvent* aEvent)
 {
   if (!aEvent->mFlags.mHandledByAPZ) {
     return false;
   }
 
   if (!gfxPlatform::GetPlatform()->SupportsApzDragInput()) {
@@ -969,32 +1007,41 @@ nsSliderFrame::StartAPZDrag(WidgetGUIEve
   nsRect sliderTrack;
   GetXULClientRect(sliderTrack);
 
   // This rect is the range in which the scroll thumb can slide in.
   sliderTrack = sliderTrack + GetRect().TopLeft() + scrollbarBox->GetPosition() -
                 scrollFrameAsScrollable->GetScrollPortRect().TopLeft();
   CSSRect sliderTrackCSS = CSSRect::FromAppUnits(sliderTrack);
 
+  nsIPresShell* shell = PresContext()->PresShell();
   uint64_t inputblockId = InputAPZContext::GetInputBlockId();
-  uint32_t presShellId = PresContext()->PresShell()->GetPresShellId();
+  uint32_t presShellId = shell->GetPresShellId();
   AsyncDragMetrics dragMetrics(scrollTargetId, presShellId, inputblockId,
                                NSAppUnitsToFloatPixels(mDragStart,
                                  float(AppUnitsPerCSSPixel())),
                                sliderTrackCSS,
                                IsXULHorizontal() ? AsyncDragMetrics::HORIZONTAL :
                                                    AsyncDragMetrics::VERTICAL);
 
   if (!nsLayoutUtils::HasDisplayPort(scrollableContent)) {
     return false;
   }
 
   // When we start an APZ drag, we wont get mouse events for the drag.
   // APZ will consume them all and only notify us of the new scroll position.
-  this->GetNearestWidget()->StartAsyncScrollbarDrag(dragMetrics);
+  bool waitForRefresh = InputAPZContext::HavePendingLayerization();
+  nsIWidget* widget = this->GetNearestWidget();
+  if (waitForRefresh) {
+    waitForRefresh = shell->AddPostRefreshObserver(
+        new AsyncScrollbarDragStarter(shell, widget, dragMetrics));
+  }
+  if (!waitForRefresh) {
+    widget->StartAsyncScrollbarDrag(dragMetrics);
+  }
   return true;
 }
 
 nsresult
 nsSliderFrame::StartDrag(nsIDOMEvent* aEvent)
 {
 #ifdef DEBUG_SLIDER
   printf("Begin dragging\n");