From f9e919a8f4b038893e58a25e282762bddc423d16 Mon Sep 17 00:00:00 2001 From: Botond Ballo Date: Tue, 28 Apr 2020 01:34:22 +0000 Subject: [PATCH] Bug 1556556 - Remove applications of the visual-to-layout transform at the process boundary (and equivalent places for non-e10s). r=kats Note that the propagation of the target guid to places where the transform will be applied is best-effort at the moment. In particular, the InputAPZContext will result in the correct guid being available in places that are called synchronously from the Recv*() functions, but not places called asynhcronously (e.g. via DelayedFireSingleTapEvent). To mitigate this, places where the transform is applied fall back on the RCD-RSF if a guid is not available via InputAPZContext (added in a subsequent patch). The cases that this gets wrong are fairly edge casey (it requires (a) an asynchronous codepath, (b) an event targeting a subframe, and (c) that subframe having a "could not accept the APZ scroll position" transform), so we just punt on them for now. If it turns out to be important to handle, then options for doing so include (1) propagating the guid through each of the affected asynchronous codepaths, or (2) attaching the guid to the event itself. Differential Revision: https://phabricator.services.mozilla.com/D68723 --- dom/ipc/BrowserChild.cpp | 51 +++++++++++++------ .../apz/util/ChromeProcessController.cpp | 20 ++++---- widget/PuppetWidget.cpp | 9 ---- widget/nsBaseWidget.cpp | 11 ---- 4 files changed, 45 insertions(+), 46 deletions(-) diff --git a/dom/ipc/BrowserChild.cpp b/dom/ipc/BrowserChild.cpp index 4f76721a101d..55926968e39e 100644 --- a/dom/ipc/BrowserChild.cpp +++ b/dom/ipc/BrowserChild.cpp @@ -1339,8 +1339,13 @@ mozilla::ipc::IPCResult BrowserChild::RecvHandleTap( } CSSToLayoutDeviceScale scale( presShell->GetPresContext()->CSSToDevPixelScale()); - CSSPoint point = - APZCCallbackHelper::ApplyCallbackTransform(aPoint / scale, aGuid); + CSSPoint point = aPoint / scale; + + // Stash the guid in InputAPZContext so that when the visual-to-layout + // transform is applied to the event's coordinates, we use the right transform + // based on the scroll frame being targeted. + // The other values don't really matter. + InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eSentinel); switch (aType) { case GeckoContentController::TapType::eSingleTap: @@ -1635,6 +1640,16 @@ mozilla::ipc::IPCResult BrowserChild::RecvRealMouseButtonEvent( void BrowserChild::HandleRealMouseButtonEvent(const WidgetMouseEvent& aEvent, const ScrollableLayerGuid& aGuid, const uint64_t& aInputBlockId) { + WidgetMouseEvent localEvent(aEvent); + localEvent.mWidget = mPuppetWidget; + + // We need one InputAPZContext here to propagate |aGuid| to places in + // SendSetTargetAPZCNotification() which apply the visual-to-layout transform, + // and another below to propagate the |postLayerization| flag (whose value + // we don't know until SendSetTargetAPZCNotification() returns) into + // the event dispatch code. + InputAPZContext context1(aGuid, aInputBlockId, nsEventStatus_eSentinel); + // 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 @@ -1642,23 +1657,19 @@ void BrowserChild::HandleRealMouseButtonEvent(const WidgetMouseEvent& aEvent, // Since thos events didn't go through APZ, we don't need to send // notifications for them. UniquePtr postLayerization; - if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) { + if (aInputBlockId && localEvent.mFlags.mHandledByAPZ) { nsCOMPtr document(GetTopLevelDocument()); postLayerization = APZCCallbackHelper::SendSetTargetAPZCNotification( - mPuppetWidget, document, aEvent, aGuid.mLayersId, aInputBlockId); + mPuppetWidget, document, localEvent, aGuid.mLayersId, aInputBlockId); } - InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eIgnore, - postLayerization != nullptr); + InputAPZContext context2(aGuid, aInputBlockId, nsEventStatus_eSentinel, + postLayerization != nullptr); - WidgetMouseEvent localEvent(aEvent); - localEvent.mWidget = mPuppetWidget; - APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid, - mPuppetWidget->GetDefaultScale()); DispatchWidgetEventViaAPZ(localEvent); - if (aInputBlockId && aEvent.mFlags.mHandledByAPZ) { - mAPZEventState->ProcessMouseEvent(aEvent, aInputBlockId); + if (aInputBlockId && localEvent.mFlags.mHandledByAPZ) { + mAPZEventState->ProcessMouseEvent(localEvent, aInputBlockId); } // Do this after the DispatchWidgetEventViaAPZ call above, so that if the @@ -1745,8 +1756,13 @@ void BrowserChild::DispatchWheelEvent(const WidgetWheelEvent& aEvent, } localEvent.mWidget = mPuppetWidget; - APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid, - mPuppetWidget->GetDefaultScale()); + + // Stash the guid in InputAPZContext so that when the visual-to-layout + // transform is applied to the event's coordinates, we use the right transform + // based on the scroll frame being targeted. + // The other values don't really matter. + InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eSentinel); + DispatchWidgetEventViaAPZ(localEvent); if (localEvent.mCanTriggerSwipe) { @@ -1800,8 +1816,11 @@ mozilla::ipc::IPCResult BrowserChild::RecvRealTouchEvent( WidgetTouchEvent localEvent(aEvent); localEvent.mWidget = mPuppetWidget; - APZCCallbackHelper::ApplyCallbackTransform(localEvent, aGuid, - mPuppetWidget->GetDefaultScale()); + // Stash the guid in InputAPZContext so that when the visual-to-layout + // transform is applied to the event's coordinates, we use the right transform + // based on the scroll frame being targeted. + // The other values don't really matter. + InputAPZContext context(aGuid, aInputBlockId, aApzResponse); if (localEvent.mMessage == eTouchStart && AsyncPanZoomEnabled()) { nsCOMPtr document = GetTopLevelDocument(); diff --git a/gfx/layers/apz/util/ChromeProcessController.cpp b/gfx/layers/apz/util/ChromeProcessController.cpp index b68f83993306..65dbcc0a5f7a 100644 --- a/gfx/layers/apz/util/ChromeProcessController.cpp +++ b/gfx/layers/apz/util/ChromeProcessController.cpp @@ -15,6 +15,7 @@ #include "mozilla/layers/APZEventState.h" #include "mozilla/layers/APZThreadUtils.h" #include "mozilla/layers/IAPZCTreeManager.h" +#include "mozilla/layers/InputAPZContext.h" #include "mozilla/layers/DoubleTapToZoom.h" #include "mozilla/dom/Document.h" #include "nsIInterfaceRequestorUtils.h" @@ -139,14 +140,7 @@ void ChromeProcessController::HandleDoubleTap( return; } - // CalculateRectToZoomTo performs a hit test on the frame associated with the - // Root Content Document. Unfortunately that frame does not know about the - // resolution of the document and so we must remove it before calculating - // the zoomToRect. - PresShell* presShell = document->GetPresShell(); - const float resolution = presShell->GetResolution(); - CSSPoint point(aPoint.x / resolution, aPoint.y / resolution); - CSSRect zoomToRect = CalculateRectToZoomTo(document, point); + CSSRect zoomToRect = CalculateRectToZoomTo(document, aPoint); uint32_t presShellId; ScrollableLayerGuid::ViewID viewId; @@ -191,8 +185,14 @@ void ChromeProcessController::HandleTap( } CSSToLayoutDeviceScale scale( presShell->GetPresContext()->CSSToDevPixelScale()); - CSSPoint point = - APZCCallbackHelper::ApplyCallbackTransform(aPoint / scale, aGuid); + + CSSPoint point = aPoint / scale; + + // Stash the guid in InputAPZContext so that when the visual-to-layout + // transform is applied to the event's coordinates, we use the right transform + // based on the scroll frame being targeted. + // The other values don't really matter. + InputAPZContext context(aGuid, aInputBlockId, nsEventStatus_eSentinel); switch (aType) { case TapType::eSingleTap: diff --git a/widget/PuppetWidget.cpp b/widget/PuppetWidget.cpp index 1457a3d6a1f6..748a26436a39 100644 --- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -395,15 +395,6 @@ nsEventStatus PuppetWidget::DispatchInputEvent(WidgetInputEvent* aEvent) { return nsEventStatus_eIgnore; } - if (PresShell* presShell = mBrowserChild->GetTopLevelPresShell()) { - // Because the root resolution is conceptually at the parent/child process - // boundary, we need to apply that resolution here because we're sending - // the event from the child to the parent process. - LayoutDevicePoint pt(aEvent->mRefPoint); - pt = pt * presShell->GetResolution(); - aEvent->mRefPoint = LayoutDeviceIntPoint::Round(pt); - } - switch (aEvent->mClass) { case eWheelEventClass: Unused << mBrowserChild->SendDispatchWheelEvent(*aEvent->AsWheelEvent()); diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index e50332056d6e..f6feac7fbfd9 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -968,17 +968,6 @@ nsEventStatus nsBaseWidget::ProcessUntransformedAPZEvent( InputAPZContext context(aApzResult.mTargetGuid, inputBlockId, aApzResult.mStatus); - // If this is an event that the APZ has targeted to an APZC in the root - // process, apply that APZC's callback-transform before dispatching the - // event. If the event is instead targeted to an APZC in the child process, - // the transform will be applied in the child process before dispatching - // the event there (see e.g. BrowserChild::RecvRealTouchEvent()). - if (aApzResult.mTargetGuid.mLayersId == - mCompositorSession->RootLayerTreeId()) { - APZCCallbackHelper::ApplyCallbackTransform(*aEvent, targetGuid, - GetDefaultScale()); - } - // Make a copy of the original event for the APZCCallbackHelper helpers that // we call later, because the event passed to DispatchEvent can get mutated in // ways that we don't want (i.e. touch points can get stripped out).