Bug 1468545 - Don't process drag blocks that are still waiting for their drag metrics. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 14 Jun 2018 10:30:19 -0400
changeset 807417 3c1e374ea6377807ad171b39b05c7ebeebce7ea2
parent 807416 cd1e1e853e4a44b71df1e279f534d54430b2fbce
child 807418 ed0085e2988824af2f09c55bf94dd3c52af76db0
push id113099
push userkgupta@mozilla.com
push dateThu, 14 Jun 2018 14:31:06 +0000
reviewersbotond
bugs1468545
milestone62.0a1
Bug 1468545 - Don't process drag blocks that are still waiting for their drag metrics. r?botond MozReview-Commit-ID: 3ft551iBIDY
gfx/layers/apz/src/InputBlockState.cpp
gfx/layers/apz/src/InputBlockState.h
gfx/layers/apz/src/InputQueue.cpp
--- a/gfx/layers/apz/src/InputBlockState.cpp
+++ b/gfx/layers/apz/src/InputBlockState.cpp
@@ -309,41 +309,69 @@ DragBlockState::MarkMouseUpReceived()
 }
 
 void
 DragBlockState::SetInitialThumbPos(CSSCoord aThumbPos)
 {
   mInitialThumbPos = aThumbPos;
 }
 
-void
+bool
 DragBlockState::SetDragMetrics(const AsyncDragMetrics& aDragMetrics)
 {
-  mDragMetrics = aDragMetrics;
+  if (mDragMetrics) {
+    // The main thread might send metrics after we've already started the
+    // scrolling on the APZ side. In that case keep our existing metrics.
+    // As a sanity check we assert that the main thread found the same
+    // scrollbar target. Note that aDragMetrics.mScrollbarDragOffset might
+    // be different from mDragMetrics->mScrollbarDragOffset here bceause
+    // the main thread might have gotten the input event after APZ already
+    // handled more of the drag block, moved the scrollthumb, and triggered
+    // a repaint. So we should keep mDragMetrics in this scenario.
+    MOZ_ASSERT(aDragMetrics.mViewId == mDragMetrics->mViewId);
+    MOZ_ASSERT(aDragMetrics.mDirection == mDragMetrics->mDirection);
+    return false;
+  }
+  mDragMetrics = Some(aDragMetrics);
+  return true;
 }
 
 bool
 DragBlockState::SetContentResponse(bool aStartedAsyncScrollbarDrag)
 {
   // For mouse events we've repurposed the argument (which normally indicates
   // if content called preventDefault() on the event) to instead indicate if
   // the event started an async scrollbar drag.
   mStartedAsyncScrollbarDrag |= aStartedAsyncScrollbarDrag;
   return CancelableBlockState::SetContentResponse(false);
 }
 
+bool
+DragBlockState::IsReadyForHandling() const
+{
+  if (!CancelableBlockState::IsReadyForHandling()) {
+    return false;
+  }
+
+  if (mStartedAsyncScrollbarDrag && !mDragMetrics.isSome()) {
+    return false;
+  }
+
+  return true;
+}
+
 void
 DragBlockState::DispatchEvent(const InputData& aEvent) const
 {
   MouseInput mouseInput = aEvent.AsMouseInput();
   if (!mouseInput.TransformToLocal(mTransformToApzc)) {
     return;
   }
 
-  GetTargetApzc()->HandleDragEvent(mouseInput, mDragMetrics, mInitialThumbPos);
+  GetTargetApzc()->HandleDragEvent(mouseInput, mDragMetrics.refOr(AsyncDragMetrics()), mInitialThumbPos);
 }
 
 bool
 DragBlockState::MustStayActive()
 {
   return !mReceivedMouseUp;
 }
 
--- a/gfx/layers/apz/src/InputBlockState.h
+++ b/gfx/layers/apz/src/InputBlockState.h
@@ -317,22 +317,23 @@ public:
   bool HasReceivedMouseUp();
   void MarkMouseUpReceived();
 
   DragBlockState *AsDragBlock() override {
     return this;
   }
 
   void SetInitialThumbPos(CSSCoord aThumbPos);
-  void SetDragMetrics(const AsyncDragMetrics& aDragMetrics);
+  bool SetDragMetrics(const AsyncDragMetrics& aDragMetrics);
 
   bool SetContentResponse(bool aStartedAsyncScrollbarDrag) override;
+  bool IsReadyForHandling() const override;
   void DispatchEvent(const InputData& aEvent) const override;
 private:
-  AsyncDragMetrics mDragMetrics;
+  Maybe<AsyncDragMetrics> mDragMetrics;
   CSSCoord mInitialThumbPos;
   bool mStartedAsyncScrollbarDrag;
   bool mReceivedMouseUp;
 };
 
 /**
  * A single block of pan gesture events.
  */
--- a/gfx/layers/apz/src/InputQueue.cpp
+++ b/gfx/layers/apz/src/InputQueue.cpp
@@ -698,30 +698,32 @@ InputQueue::SetConfirmedTargetApzc(uint6
     NS_WARNING("input block is not a cancelable block");
   }
   if (success) {
     ProcessQueue();
   }
 }
 
 void
-InputQueue::ConfirmDragBlock(uint64_t aInputBlockId, const RefPtr<AsyncPanZoomController>& aTargetApzc,
-                                   const AsyncDragMetrics& aDragMetrics)
+InputQueue::ConfirmDragBlock(uint64_t aInputBlockId,
+                             const RefPtr<AsyncPanZoomController>& aTargetApzc,
+                             const AsyncDragMetrics& aDragMetrics)
 {
   APZThreadUtils::AssertOnControllerThread();
 
-  INPQ_LOG("got a target apzc; block=%" PRIu64 " guid=%s\n",
-    aInputBlockId, aTargetApzc ? Stringify(aTargetApzc->GetGuid()).c_str() : "");
+  INPQ_LOG("got a target apzc; block=%" PRIu64 " guid=%s dragtarget=%" PRIu64 "\n",
+    aInputBlockId, aTargetApzc ? Stringify(aTargetApzc->GetGuid()).c_str() : "",
+    aDragMetrics.mViewId);
   bool success = false;
   InputData* firstInput = nullptr;
   InputBlockState* inputBlock = FindBlockForId(aInputBlockId, &firstInput);
   if (inputBlock && inputBlock->AsDragBlock()) {
     DragBlockState* block = inputBlock->AsDragBlock();
-    block->SetDragMetrics(aDragMetrics);
-    success = block->SetConfirmedTargetApzc(aTargetApzc,
+    success = block->SetDragMetrics(aDragMetrics);
+    success |= block->SetConfirmedTargetApzc(aTargetApzc,
         InputBlockState::TargetConfirmationState::eConfirmed,
         firstInput,
         /* aForScrollbarDrag = */ true);
     block->RecordContentResponseTime();
   }
   if (success) {
     ProcessQueue();
   }