Bug 1437694 - Gracefully recover from hit testing bugs affecting scrollbar dragging. r=kats
The recovery is only enabled for release builds, to avoid papering over
such hit testing bugs. On nightly builds, a diagnostic assert is issued.
MozReview-Commit-ID: Aos0j0jv6Lb
--- a/gfx/layers/apz/src/InputBlockState.cpp
+++ b/gfx/layers/apz/src/InputBlockState.cpp
@@ -34,28 +34,49 @@ InputBlockState::InputBlockState(const R
// We should never be constructed with a nullptr target.
MOZ_ASSERT(mTargetApzc);
mOverscrollHandoffChain = mTargetApzc->BuildOverscrollHandoffChain();
}
bool
InputBlockState::SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput)
+ InputData* aFirstInput,
+ bool aForScrollbarDrag)
{
MOZ_ASSERT(aState == TargetConfirmationState::eConfirmed
|| aState == TargetConfirmationState::eTimedOut);
if (mTargetConfirmed == TargetConfirmationState::eTimedOut &&
aState == TargetConfirmationState::eConfirmed) {
// The main thread finally responded. We had already timed out the
// confirmation, but we want to update the state internally so that we
// can record the time for telemetry purposes.
mTargetConfirmed = TargetConfirmationState::eTimedOutAndMainThreadResponded;
}
+ // Sometimes, bugs in compositor hit testing can lead to APZ confirming
+ // a different target than the main thread. If this happens for a drag
+ // block created for a scrollbar drag, the consequences can be fairly
+ // user-unfriendly, such as the scrollbar not being draggable at all,
+ // or it scrolling the contents of the wrong scrollframe. In Nightly
+ // builds, we issue a diagnostic assert in this situation, so that the
+ // underlying compositor hit testing bug can be fixed. In release builds,
+ // however, we just silently accept the main thread's confirmed target,
+ // which will produce the expected behaviour (apart from drag events
+ // received so far being dropped).
+ if (AsDragBlock() && aForScrollbarDrag &&
+ mTargetConfirmed == TargetConfirmationState::eConfirmed &&
+ aState == TargetConfirmationState::eConfirmed &&
+ mTargetApzc && aTargetApzc &&
+ mTargetApzc->GetGuid() != aTargetApzc->GetGuid()) {
+ MOZ_DIAGNOSTIC_ASSERT(false, "APZ and main thread confirmed scrollbar drag block with different targets");
+ UpdateTargetApzc(aTargetApzc);
+ return true;
+ }
+
if (mTargetConfirmed != TargetConfirmationState::eUnconfirmed) {
return false;
}
mTargetConfirmed = aState;
TBS_LOG("%p got confirmed target APZC %p\n", this, mTargetApzc.get());
if (mTargetApzc == aTargetApzc) {
// The confirmed target is the same as the tentative one, so we're done.
@@ -358,28 +379,29 @@ WheelBlockState::SetContentResponse(bool
EndTransaction();
}
return CancelableBlockState::SetContentResponse(aPreventDefault);
}
bool
WheelBlockState::SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput)
+ InputData* aFirstInput,
+ bool aForScrollbarDrag)
{
// The APZC that we find via APZCCallbackHelpers may not be the same APZC
// ESM or OverscrollHandoff would have computed. Make sure we get the right
// one by looking for the first apzc the next pending event can scroll.
RefPtr<AsyncPanZoomController> apzc = aTargetApzc;
if (apzc && aFirstInput) {
apzc = apzc->BuildOverscrollHandoffChain()->FindFirstScrollable(
*aFirstInput, &mAllowedScrollDirections);
}
- InputBlockState::SetConfirmedTargetApzc(apzc, aState, aFirstInput);
+ InputBlockState::SetConfirmedTargetApzc(apzc, aState, aFirstInput, aForScrollbarDrag);
return true;
}
void
WheelBlockState::Update(ScrollWheelInput& aEvent)
{
// We might not be in a transaction if the block never started in a
// transaction - for example, if nothing was scrollable.
@@ -575,32 +597,33 @@ PanGestureBlockState::PanGestureBlockSta
UpdateTargetApzc(apzc);
}
}
}
bool
PanGestureBlockState::SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput)
+ InputData* aFirstInput,
+ bool aForScrollbarDrag)
{
// The APZC that we find via APZCCallbackHelpers may not be the same APZC
// ESM or OverscrollHandoff would have computed. Make sure we get the right
// one by looking for the first apzc the next pending event can scroll.
RefPtr<AsyncPanZoomController> apzc = aTargetApzc;
if (apzc && aFirstInput) {
RefPtr<AsyncPanZoomController> scrollableApzc =
apzc->BuildOverscrollHandoffChain()->FindFirstScrollable(
*aFirstInput, &mAllowedScrollDirections);
if (scrollableApzc) {
apzc = scrollableApzc;
}
}
- InputBlockState::SetConfirmedTargetApzc(apzc, aState, aFirstInput);
+ InputBlockState::SetConfirmedTargetApzc(apzc, aState, aFirstInput, aForScrollbarDrag);
return true;
}
bool
PanGestureBlockState::MustStayActive()
{
return !mInterrupted;
}
--- a/gfx/layers/apz/src/InputBlockState.h
+++ b/gfx/layers/apz/src/InputBlockState.h
@@ -70,17 +70,18 @@ public:
return nullptr;
}
virtual KeyboardBlockState* AsKeyboardBlock() {
return nullptr;
}
virtual bool SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput);
+ InputData* aFirstInput,
+ bool aForScrollbarDrag);
const RefPtr<AsyncPanZoomController>& GetTargetApzc() const;
const RefPtr<const OverscrollHandoffChain>& GetOverscrollHandoffChain() const;
uint64_t GetBlockId() const;
bool IsTargetConfirmed() const;
bool HasReceivedRealConfirmedTarget() const;
virtual bool ShouldDropEvents() const;
@@ -225,17 +226,18 @@ public:
TargetConfirmationFlags aFlags,
const ScrollWheelInput& aEvent);
bool SetContentResponse(bool aPreventDefault) override;
bool MustStayActive() override;
const char* Type() override;
bool SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput) override;
+ InputData* aFirstInput,
+ bool aForScrollbarDrag) override;
WheelBlockState *AsWheelBlock() override {
return this;
}
/**
* Determine whether this wheel block is accepting new events.
*/
@@ -341,17 +343,18 @@ public:
bool SetContentResponse(bool aPreventDefault) override;
bool HasReceivedAllContentNotifications() const override;
bool IsReadyForHandling() const override;
bool MustStayActive() override;
const char* Type() override;
bool SetConfirmedTargetApzc(const RefPtr<AsyncPanZoomController>& aTargetApzc,
TargetConfirmationState aState,
- InputData* aFirstInput) override;
+ InputData* aFirstInput,
+ bool aForScrollbarDrag) override;
PanGestureBlockState *AsPanGestureBlock() override {
return this;
}
/**
* @return Whether or not overscrolling is prevented for this block.
*/
--- a/gfx/layers/apz/src/InputQueue.cpp
+++ b/gfx/layers/apz/src/InputQueue.cpp
@@ -109,17 +109,18 @@ InputQueue::ReceiveTouchInput(const RefP
// If we're already in a fast fling, and a single finger goes down, then
// we want special handling for the touch event, because it shouldn't get
// delivered to content. Note that we don't set this flag when going
// from a fast fling to a pinch state (i.e. second finger goes down while
// the first finger is moving).
block->SetDuringFastFling();
block->SetConfirmedTargetApzc(aTarget,
InputBlockState::TargetConfirmationState::eConfirmed,
- nullptr /* the block was just created so it has no events */);
+ nullptr /* the block was just created so it has no events */,
+ false /* not a scrollbar drag */);
if (gfxPrefs::TouchActionEnabled()) {
block->SetAllowedTouchBehaviors(currentBehaviors);
}
INPQ_LOG("block %p tagged as fast-motion\n", block);
}
CancelAnimationsForNewBlock(block);
@@ -637,17 +638,22 @@ InputQueue::MainThreadTimeout(uint64_t a
CancelableBlockState* block = inputBlock->AsCancelableBlock();
// time out the touch-listener response and also confirm the existing
// target apzc in the case where the main thread doesn't get back to us
// fast enough.
success = block->TimeoutContentResponse();
success |= block->SetConfirmedTargetApzc(
block->GetTargetApzc(),
InputBlockState::TargetConfirmationState::eTimedOut,
- firstInput);
+ firstInput,
+ // This actually could be a scrollbar drag, but we pass
+ // aForScrollbarDrag=false because for scrollbar drags,
+ // SetConfirmedTargetApzc() will also be called by ConfirmDragBlock(),
+ // and we pass aForScrollbarDrag=true there.
+ false);
} else if (inputBlock) {
NS_WARNING("input block is not a cancelable block");
}
if (success) {
ProcessQueue();
}
}
@@ -678,17 +684,22 @@ InputQueue::SetConfirmedTargetApzc(uint6
aInputBlockId, aTargetApzc ? Stringify(aTargetApzc->GetGuid()).c_str() : "");
bool success = false;
InputData* firstInput = nullptr;
InputBlockState* inputBlock = FindBlockForId(aInputBlockId, &firstInput);
if (inputBlock && inputBlock->AsCancelableBlock()) {
CancelableBlockState* block = inputBlock->AsCancelableBlock();
success = block->SetConfirmedTargetApzc(aTargetApzc,
InputBlockState::TargetConfirmationState::eConfirmed,
- firstInput);
+ firstInput,
+ // This actually could be a scrollbar drag, but we pass
+ // aForScrollbarDrag=false because for scrollbar drags,
+ // SetConfirmedTargetApzc() will also be called by ConfirmDragBlock(),
+ // and we pass aForScrollbarDrag=true there.
+ false);
block->RecordContentResponseTime();
} else if (inputBlock) {
NS_WARNING("input block is not a cancelable block");
}
if (success) {
ProcessQueue();
}
}
@@ -704,17 +715,18 @@ InputQueue::ConfirmDragBlock(uint64_t aI
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,
InputBlockState::TargetConfirmationState::eConfirmed,
- firstInput);
+ firstInput,
+ /* aForScrollbarDrag = */ true);
block->RecordContentResponseTime();
}
if (success) {
ProcessQueue();
}
}
void