Bug 1468545 - Ensure the drag metrics gets to APZ before the target APZC for inactive scrollframes. r?botond
In the case where an inactive scrollframe's scrollbar gets dragged, the
main thread layerizes the scrollframe and dispatches both a
SetTargetAPZC message and a AsyncDragMetrics message using post-refresh
observers. However, the post-refresh observers are registered such that
the SetTargetAPZC message gets sent first, and APZ will start
processing the drag block immediately upon receipt of that event. This
means that the APZC might not have the correct drag metrics when it
processes those input events. For correct behaviour, we want the
AsyncDragMetrics message to reach APZ first in this scenario, and this
patch accomplishes this by allowing the post-refresh observers to be
registered in the opposite order.
MozReview-Commit-ID: 6LzyYYG1t6F
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1713,36 +1713,43 @@ TabChild::HandleRealMouseButtonEvent(con
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;
+ UniquePtr<DisplayportSetListener> postLayerization;
if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
nsCOMPtr<nsIDocument> document(GetDocument());
- pendingLayerization =
- APZCCallbackHelper::SendSetTargetAPZCNotification(mPuppetWidget, document,
- aEvent, aGuid,
- aInputBlockId);
+ postLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification(
+ mPuppetWidget, document, aEvent, aGuid, aInputBlockId);
}
- InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eIgnore, pendingLayerization);
+ InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eIgnore, postLayerization != nullptr);
WidgetMouseEvent localEvent(aEvent);
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid,
mPuppetWidget->GetDefaultScale());
DispatchWidgetEventViaAPZ(localEvent);
if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
mAPZEventState->ProcessMouseEvent(aEvent, aGuid, aInputBlockId);
}
+
+ // Do this after the DispatchWidgetEventViaAPZ call above, so that if the
+ // mouse event triggered a post-refresh AsyncDragMetrics message to be sent
+ // to APZ (from scrollbar dragging in nsSliderFrame), then that will reach
+ // APZ before the SetTargetAPZC message. This ensures the drag input block
+ // gets the drag metrics before handling the input events.
+ if (postLayerization && postLayerization->Register()) {
+ Unused << postLayerization.release();
+ }
}
mozilla::ipc::IPCResult
TabChild::RecvNormalPriorityRealMouseButtonEvent(
const WidgetMouseEvent& aEvent,
const ScrollableLayerGuid& aGuid,
const uint64_t& aInputBlockId)
{
@@ -1813,18 +1820,22 @@ TabChild::MaybeDispatchCoalescedWheelEve
void
TabChild::DispatchWheelEvent(const WidgetWheelEvent& aEvent,
const ScrollableLayerGuid& aGuid,
const uint64_t& aInputBlockId)
{
WidgetWheelEvent localEvent(aEvent);
if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) {
nsCOMPtr<nsIDocument> document(GetDocument());
- APZCCallbackHelper::SendSetTargetAPZCNotification(
- mPuppetWidget, document, aEvent, aGuid, aInputBlockId);
+ UniquePtr<DisplayportSetListener> postLayerization =
+ APZCCallbackHelper::SendSetTargetAPZCNotification(
+ mPuppetWidget, document, aEvent, aGuid, aInputBlockId);
+ if (postLayerization && postLayerization->Register()) {
+ Unused << postLayerization.release();
+ }
}
localEvent.mWidget = mPuppetWidget;
APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid,
mPuppetWidget->GetDefaultScale());
DispatchWidgetEventViaAPZ(localEvent);
if (localEvent.mCanTriggerSwipe) {
@@ -1889,19 +1900,22 @@ TabChild::RecvRealTouchEvent(const Widge
if (localEvent.mMessage == eTouchStart && AsyncPanZoomEnabled()) {
nsCOMPtr<nsIDocument> document = GetDocument();
if (gfxPrefs::TouchActionEnabled()) {
APZCCallbackHelper::SendSetAllowedTouchBehaviorNotification(
mPuppetWidget, document, localEvent, aInputBlockId,
mSetAllowedTouchBehaviorCallback);
}
- APZCCallbackHelper::SendSetTargetAPZCNotification(mPuppetWidget, document,
- localEvent, aGuid,
- aInputBlockId);
+ UniquePtr<DisplayportSetListener> postLayerization =
+ APZCCallbackHelper::SendSetTargetAPZCNotification(
+ mPuppetWidget, document, localEvent, aGuid, aInputBlockId);
+ if (postLayerization && postLayerization->Register()) {
+ Unused << postLayerization.release();
+ }
}
// Dispatch event to content (potentially a long-running operation)
nsEventStatus status = DispatchWidgetEventViaAPZ(localEvent);
if (!AsyncPanZoomEnabled()) {
// We shouldn't have any e10s platforms that have touch events enabled
// without APZ.
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -717,97 +717,82 @@ SendLayersDependentApzcTargetConfirmatio
LayerTransactionChild* shadow = lm->AsShadowForwarder()->GetShadowManager();
if (!shadow) {
return;
}
shadow->SendSetConfirmedTargetAPZC(aInputBlockId, aTargets);
}
-class DisplayportSetListener : public nsAPostRefreshObserver {
-public:
- DisplayportSetListener(nsIPresShell* aPresShell,
- const uint64_t& aInputBlockId,
- const nsTArray<ScrollableLayerGuid>& aTargets)
- : mPresShell(aPresShell)
- , mInputBlockId(aInputBlockId)
- , mTargets(aTargets)
- {
- }
-
- virtual ~DisplayportSetListener()
- {
- }
-
- void DidRefresh() override {
- if (!mPresShell) {
- MOZ_ASSERT_UNREACHABLE("Post-refresh observer fired again after failed attempt at unregistering it");
- return;
- }
-
- APZCCH_LOG("Got refresh, sending target APZCs for input block %" PRIu64 "\n", mInputBlockId);
- SendLayersDependentApzcTargetConfirmation(mPresShell, mInputBlockId, std::move(mTargets));
-
- 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;
- return;
- }
+DisplayportSetListener::DisplayportSetListener(nsIWidget* aWidget,
+ nsIPresShell* aPresShell,
+ const uint64_t& aInputBlockId,
+ const nsTArray<ScrollableLayerGuid>& aTargets)
+ : mWidget(aWidget)
+ , mPresShell(aPresShell)
+ , mInputBlockId(aInputBlockId)
+ , mTargets(aTargets)
+{
+}
- delete this;
- }
-
-private:
- RefPtr<nsIPresShell> mPresShell;
- uint64_t mInputBlockId;
- nsTArray<ScrollableLayerGuid> mTargets;
-};
-
-// Sends a SetTarget notification for APZC, given one or more previous
-// calls to PrepareForAPZCSetTargetNotification().
-static void
-SendSetTargetAPZCNotificationHelper(nsIWidget* aWidget,
- nsIPresShell* aShell,
- const uint64_t& aInputBlockId,
- const nsTArray<ScrollableLayerGuid>& aTargets,
- bool aWaitForRefresh)
+DisplayportSetListener:: ~DisplayportSetListener()
{
- bool waitForRefresh = aWaitForRefresh;
- if (waitForRefresh) {
- APZCCH_LOG("At least one target got a new displayport, need to wait for refresh\n");
- waitForRefresh = aShell->AddPostRefreshObserver(
- new DisplayportSetListener(aShell, aInputBlockId, std::move(aTargets)));
- }
- 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");
- }
}
bool
+DisplayportSetListener::Register()
+{
+ if (mPresShell->AddPostRefreshObserver(this)) {
+ APZCCH_LOG("Successfully registered post-refresh observer\n");
+ return true;
+ }
+ // In case of failure just send the notification right away
+ APZCCH_LOG("Sending target APZCs for input block %" PRIu64 "\n", mInputBlockId);
+ mWidget->SetConfirmedTargetAPZC(mInputBlockId, mTargets);
+ return false;
+}
+
+void
+DisplayportSetListener::DidRefresh() {
+ if (!mPresShell) {
+ MOZ_ASSERT_UNREACHABLE("Post-refresh observer fired again after failed attempt at unregistering it");
+ return;
+ }
+
+ APZCCH_LOG("Got refresh, sending target APZCs for input block %" PRIu64 "\n", mInputBlockId);
+ SendLayersDependentApzcTargetConfirmation(mPresShell, mInputBlockId, std::move(mTargets));
+
+ 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;
+ return;
+ }
+
+ delete this;
+}
+
+UniquePtr<DisplayportSetListener>
APZCCallbackHelper::SendSetTargetAPZCNotification(nsIWidget* aWidget,
nsIDocument* aDocument,
const WidgetGUIEvent& aEvent,
const ScrollableLayerGuid& aGuid,
uint64_t aInputBlockId)
{
if (!aWidget || !aDocument) {
- return false;
+ return nullptr;
}
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 false;
+ return nullptr;
}
sLastTargetAPZCNotificationInputBlock = aInputBlockId;
if (nsIPresShell* shell = aDocument->GetShell()) {
if (nsIFrame* rootFrame = shell->GetRootFrame()) {
rootFrame = UpdateRootFrameForTouchTargetDocument(rootFrame);
bool waitForRefresh = false;
nsTArray<ScrollableLayerGuid> targets;
@@ -822,28 +807,26 @@ APZCCallbackHelper::SendSetTargetAPZCNot
rootFrame, wheelEvent->mRefPoint, &targets);
} else if (const WidgetMouseEvent* mouseEvent = aEvent.AsMouseEvent()) {
waitForRefresh = PrepareForSetTargetAPZCNotification(aWidget, aGuid,
rootFrame, mouseEvent->mRefPoint, &targets);
}
// TODO: Do other types of events need to be handled?
if (!targets.IsEmpty()) {
- SendSetTargetAPZCNotificationHelper(
- aWidget,
- shell,
- aInputBlockId,
- std::move(targets),
- waitForRefresh);
+ if (waitForRefresh) {
+ APZCCH_LOG("At least one target got a new displayport, need to wait for refresh\n");
+ return MakeUnique<DisplayportSetListener>(aWidget, shell, aInputBlockId, std::move(targets));
+ }
+ APZCCH_LOG("Sending target APZCs for input block %" PRIu64 "\n", aInputBlockId);
+ aWidget->SetConfirmedTargetAPZC(aInputBlockId, targets);
}
-
- return waitForRefresh;
}
}
- return false;
+ return nullptr;
}
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
@@ -6,16 +6,17 @@
#ifndef mozilla_layers_APZCCallbackHelper_h
#define mozilla_layers_APZCCallbackHelper_h
#include "FrameMetrics.h"
#include "InputData.h"
#include "mozilla/EventForwards.h"
#include "mozilla/layers/APZUtils.h"
#include "nsIDOMWindowUtils.h"
+#include "nsRefreshDriver.h"
#include <functional>
class nsIContent;
class nsIDocument;
class nsIPresShell;
class nsIScrollableFrame;
class nsIWidget;
@@ -23,16 +24,35 @@ template<class T> struct already_AddRefe
template<class T> class nsCOMPtr;
namespace mozilla {
namespace layers {
typedef std::function<void(uint64_t, const nsTArray<TouchBehaviorFlags>&)>
SetAllowedTouchBehaviorCallback;
+/* Refer to documentation on SendSetTargetAPZCNotification for this class */
+class DisplayportSetListener : public nsAPostRefreshObserver
+{
+public:
+ DisplayportSetListener(nsIWidget* aWidget,
+ nsIPresShell* aPresShell,
+ const uint64_t& aInputBlockId,
+ const nsTArray<ScrollableLayerGuid>& aTargets);
+ virtual ~DisplayportSetListener();
+ bool Register();
+ void DidRefresh() override;
+
+private:
+ RefPtr<nsIWidget> mWidget;
+ RefPtr<nsIPresShell> mPresShell;
+ uint64_t mInputBlockId;
+ nsTArray<ScrollableLayerGuid> mTargets;
+};
+
/* This class contains some helper methods that facilitate implementing the
GeckoContentController callback interface required by the AsyncPanZoomController.
Since different platforms need to implement this interface in similar-but-
not-quite-the-same ways, this utility class provides some helpful methods
to hold code that can be shared across the different platform implementations.
*/
class APZCCallbackHelper
{
@@ -131,30 +151,34 @@ public:
Modifiers aModifiers,
int32_t aClickCount,
nsIWidget* aWidget);
/* 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.
+ * If any displayports need to be set, this function returns a heap-allocated
+ * object. The caller is responsible for calling Register() on that object,
+ * and release()'ing the UniquePtr if that Register() call returns true.
+ * The object registers itself as a post-refresh observer on the presShell
+ * and ensures that notifications get sent to APZ correctly after the
+ * refresh.
*
- * 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.)
+ * Having the caller manage this object is desirable in case they want to
+ * (a) know about the fact that a displayport needs to be set, and
+ * (b) register a post-refresh observer of their own that will run in
+ * a defined ordering relative to the APZ messages.
*/
- static bool SendSetTargetAPZCNotification(nsIWidget* aWidget,
- nsIDocument* aDocument,
- const WidgetGUIEvent& aEvent,
- const ScrollableLayerGuid& aGuid,
- uint64_t aInputBlockId);
+ static UniquePtr<DisplayportSetListener> 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,
nsIDocument* aDocument,
const WidgetTouchEvent& aEvent,
uint64_t aInputBlockId,
const SetAllowedTouchBehaviorCallback& aCallback);
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -1078,44 +1078,46 @@ nsBaseWidget::ProcessUntransformedAPZEve
// ways that we don't want (i.e. touch points can get stripped out).
nsEventStatus status;
UniquePtr<WidgetEvent> original(aEvent->Duplicate());
DispatchEvent(aEvent, status);
if (mAPZC && !InputAPZContext::WasRoutedToChildProcess() && aInputBlockId) {
// EventStateManager did not route the event into the child process.
// It's safe to communicate to APZ that the event has been processed.
- // TODO: Eventually we'll be able to move the SendSetTargetAPZCNotification
- // call into APZEventState::Process*Event() as well.
+ UniquePtr<DisplayportSetListener> postLayerization;
if (WidgetTouchEvent* touchEvent = aEvent->AsTouchEvent()) {
if (touchEvent->mMessage == eTouchStart) {
if (gfxPrefs::TouchActionEnabled()) {
APZCCallbackHelper::SendSetAllowedTouchBehaviorNotification(this,
GetDocument(), *(original->AsTouchEvent()), aInputBlockId,
mSetAllowedTouchBehaviorCallback);
}
- APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
+ postLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
*(original->AsTouchEvent()), aGuid, aInputBlockId);
}
mAPZEventState->ProcessTouchEvent(*touchEvent, aGuid, aInputBlockId,
aApzResponse, status);
} else if (WidgetWheelEvent* wheelEvent = aEvent->AsWheelEvent()) {
MOZ_ASSERT(wheelEvent->mFlags.mHandledByAPZ);
- APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
+ postLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
*(original->AsWheelEvent()), aGuid, aInputBlockId);
if (wheelEvent->mCanTriggerSwipe) {
ReportSwipeStarted(aInputBlockId, wheelEvent->TriggersSwipe());
}
mAPZEventState->ProcessWheelEvent(*wheelEvent, aGuid, aInputBlockId);
} else if (WidgetMouseEvent* mouseEvent = aEvent->AsMouseEvent()) {
MOZ_ASSERT(mouseEvent->mFlags.mHandledByAPZ);
- APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
+ postLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification(this, GetDocument(),
*(original->AsMouseEvent()), aGuid, aInputBlockId);
mAPZEventState->ProcessMouseEvent(*mouseEvent, aGuid, aInputBlockId);
}
+ if (postLayerization && postLayerization->Register()) {
+ Unused << postLayerization.release();
+ }
}
return status;
}
class DispatchWheelEventOnMainThread : public Runnable
{
public: