Bug 1286179 - For APZ repaint requests that are triggered by main-thread updates, don't attempt to re-scroll the main thread. r?tnikkel
Avoiding these re-scrolls prevents APZ repaint requests from clobbering the
main-thread scroll position (which may have changed in the meantime) in some
cases. See https://bugzilla.mozilla.org/show_bug.cgi?id=1286179#c8 for an example
of a scenario where this re-scroll is problematic.
MozReview-Commit-ID: 7he2A2sygji
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -51,16 +51,20 @@ public:
// will begin at.
enum ScrollOffsetUpdateType : uint8_t {
eNone, // The default; the scroll offset was not updated
eMainThread, // The scroll offset was updated by the main thread.
ePending, // The scroll offset was updated on the main thread, but not
// painted, so the layer texture data is still at the old
// offset.
+ eUserAction, // In an APZ repaint request, this means the APZ generated
+ // the scroll position based on user action (the alternative
+ // is eNone which means it's just request a repaint because
+ // it got a scroll update from the main thread).
eSentinel // For IPC use only
};
FrameMetrics()
: mScrollId(NULL_SCROLL_ID)
, mPresShellResolution(1)
, mCompositionBounds(0, 0, 0, 0)
@@ -241,16 +245,21 @@ public:
void UpdatePendingScrollInfo(const ScrollUpdateInfo& aInfo)
{
mScrollOffset = aInfo.mScrollOffset;
mScrollGeneration = aInfo.mScrollGeneration;
mScrollUpdateType = ePending;
}
+ void SetRepaintDrivenByUserAction(bool aUserAction)
+ {
+ mScrollUpdateType = aUserAction ? eUserAction : eNone;
+ }
+
public:
void SetPresShellResolution(float aPresShellResolution)
{
mPresShellResolution = aPresShellResolution;
}
float GetPresShellResolution() const
{
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2815,36 +2815,37 @@ bool AsyncPanZoomController::IsPannable(
return mX.CanScroll() || mY.CanScroll();
}
int32_t AsyncPanZoomController::GetLastTouchIdentifier() const {
RefPtr<GestureEventListener> listener = GetGestureEventListener();
return listener ? listener->GetLastTouchIdentifier() : -1;
}
-void AsyncPanZoomController::RequestContentRepaint() {
+void AsyncPanZoomController::RequestContentRepaint(bool aUserAction) {
// Reinvoke this method on the main thread if it's not there already. It's
// important to do this before the call to CalculatePendingDisplayPort, so
// that CalculatePendingDisplayPort uses the most recent available version of
// mFrameMetrics, just before the paint request is dispatched to content.
if (!NS_IsMainThread()) {
// use the local variable to resolve the function overload.
- auto func = static_cast<void (AsyncPanZoomController::*)()>
+ auto func = static_cast<void (AsyncPanZoomController::*)(bool)>
(&AsyncPanZoomController::RequestContentRepaint);
- NS_DispatchToMainThread(NewRunnableMethod(this, func));
+ NS_DispatchToMainThread(NewRunnableMethod<bool>(this, func, aUserAction));
return;
}
MOZ_ASSERT(NS_IsMainThread());
ReentrantMonitorAutoEnter lock(mMonitor);
ParentLayerPoint velocity = GetVelocityVector();
mFrameMetrics.SetDisplayPortMargins(CalculatePendingDisplayPort(mFrameMetrics, velocity));
mFrameMetrics.SetUseDisplayPortMargins(true);
mFrameMetrics.SetPaintRequestTime(TimeStamp::Now());
+ mFrameMetrics.SetRepaintDrivenByUserAction(aUserAction);
RequestContentRepaint(mFrameMetrics, velocity);
}
/*static*/ CSSRect
GetDisplayPortRect(const FrameMetrics& aFrameMetrics)
{
// This computation is based on what happens in CalculatePendingDisplayPort. If that
// changes then this might need to change too
@@ -2895,16 +2896,18 @@ AsyncPanZoomController::RequestContentRe
info << " velocity " << aVelocity;
std::string str = info.str();
mCheckerboardEvent->UpdateRendertraceProperty(
CheckerboardEvent::RequestedDisplayPort, GetDisplayPortRect(aFrameMetrics),
str);
}
}
+ MOZ_ASSERT(aFrameMetrics.GetScrollUpdateType() == FrameMetrics::eNone ||
+ aFrameMetrics.GetScrollUpdateType() == FrameMetrics::eUserAction);
controller->RequestContentRepaint(aFrameMetrics);
mExpectedGeckoMetrics = aFrameMetrics;
mLastPaintRequestMetrics = aFrameMetrics;
}
bool AsyncPanZoomController::UpdateAnimation(const TimeStamp& aSampleTime,
nsTArray<RefPtr<Runnable>>* aOutDeferredTasks)
{
@@ -3410,17 +3413,18 @@ void AsyncPanZoomController::NotifyLayer
mFrameMetrics.CopySmoothScrollInfoFrom(aLayerMetrics);
needContentRepaint = true;
mExpectedGeckoMetrics = aLayerMetrics;
SmoothScrollTo(mFrameMetrics.GetSmoothScrollOffset());
}
if (needContentRepaint) {
- RequestContentRepaint();
+ // This repaint request is not driven by a user action on the APZ side
+ RequestContentRepaint(false);
}
UpdateSharedCompositorFrameMetrics();
}
const FrameMetrics& AsyncPanZoomController::GetFrameMetrics() const {
mMonitor.AssertCurrentThreadIn();
return mFrameMetrics;
}
@@ -3558,16 +3562,17 @@ void AsyncPanZoomController::ZoomToRect(
// Schedule a repaint now, so the new displayport will be painted before the
// animation finishes.
ParentLayerPoint velocity(0, 0);
endZoomToMetrics.SetDisplayPortMargins(
CalculatePendingDisplayPort(endZoomToMetrics, velocity));
endZoomToMetrics.SetUseDisplayPortMargins(true);
endZoomToMetrics.SetPaintRequestTime(TimeStamp::Now());
+ endZoomToMetrics.SetRepaintDrivenByUserAction(true);
if (NS_IsMainThread()) {
RequestContentRepaint(endZoomToMetrics, velocity);
} else {
// use a local var to resolve the function overload
auto func = static_cast<void (AsyncPanZoomController::*)(const FrameMetrics&, const ParentLayerPoint&)>
(&AsyncPanZoomController::RequestContentRepaint);
NS_DispatchToMainThread(
NewRunnableMethod<FrameMetrics, ParentLayerPoint>(
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -579,17 +579,17 @@ protected:
/**
* Utility function to send updated FrameMetrics to Gecko so that it can paint
* the displayport area. Calls into GeckoContentController to do the actual
* work. This call will use the current metrics. If this function is called
* from a non-main thread, it will redispatch itself to the main thread, and
* use the latest metrics during the redispatch.
*/
- void RequestContentRepaint();
+ void RequestContentRepaint(bool aUserAction = true);
/**
* Send the provided metrics to Gecko to trigger a repaint. This function
* may filter duplicate calls with the same metrics. This function must be
* called on the main thread.
*/
void RequestContentRepaint(const FrameMetrics& aFrameMetrics,
const ParentLayerPoint& aVelocity);
--- a/gfx/layers/apz/util/APZCCallbackHelper.cpp
+++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ -61,34 +61,41 @@ RecenterDisplayPort(mozilla::layers::Fra
{
ScreenMargin margins = aFrameMetrics.GetDisplayPortMargins();
margins.right = margins.left = margins.LeftRight() / 2;
margins.top = margins.bottom = margins.TopBottom() / 2;
aFrameMetrics.SetDisplayPortMargins(margins);
}
static CSSPoint
-ScrollFrameTo(nsIScrollableFrame* aFrame, const CSSPoint& aPoint, bool& aSuccessOut)
+ScrollFrameTo(nsIScrollableFrame* aFrame, const FrameMetrics& aMetrics, bool& aSuccessOut)
{
aSuccessOut = false;
+ CSSPoint targetScrollPosition = aMetrics.GetScrollOffset();
if (!aFrame) {
- return aPoint;
+ return targetScrollPosition;
}
- CSSPoint targetScrollPosition = aPoint;
+ CSSPoint geckoScrollPosition = CSSPoint::FromAppUnits(aFrame->GetScrollPosition());
+
+ // If the repaint request was triggered due to a previous main-thread scroll
+ // offset update sent to the APZ, then we don't need to do another scroll here
+ // and we can just return.
+ if (!aMetrics.GetScrollOffsetUpdated()) {
+ return geckoScrollPosition;
+ }
// If the frame is overflow:hidden on a particular axis, we don't want to allow
// user-driven scroll on that axis. Simply set the scroll position on that axis
// to whatever it already is. Note that this will leave the APZ's async scroll
// position out of sync with the gecko scroll position, but APZ can deal with that
// (by design). Note also that when we run into this case, even if both axes
// have overflow:hidden, we want to set aSuccessOut to true, so that the displayport
// follows the async scroll position rather than the gecko scroll position.
- CSSPoint geckoScrollPosition = CSSPoint::FromAppUnits(aFrame->GetScrollPosition());
if (aFrame->GetScrollbarStyles().mVertical == NS_STYLE_OVERFLOW_HIDDEN) {
targetScrollPosition.y = geckoScrollPosition.y;
}
if (aFrame->GetScrollbarStyles().mHorizontal == NS_STYLE_OVERFLOW_HIDDEN) {
targetScrollPosition.x = geckoScrollPosition.x;
}
// If the scrollable frame is currently in the middle of an async or smooth
@@ -124,17 +131,17 @@ ScrollFrame(nsIContent* aContent,
// Scroll the window to the desired spot
nsIScrollableFrame* sf = nsLayoutUtils::FindScrollableFrameFor(aMetrics.GetScrollId());
if (sf) {
sf->ResetScrollInfoIfGeneration(aMetrics.GetScrollGeneration());
sf->SetScrollableByAPZ(!aMetrics.IsScrollInfoLayer());
}
bool scrollUpdated = false;
CSSPoint apzScrollOffset = aMetrics.GetScrollOffset();
- CSSPoint actualScrollOffset = ScrollFrameTo(sf, apzScrollOffset, scrollUpdated);
+ CSSPoint actualScrollOffset = ScrollFrameTo(sf, aMetrics, scrollUpdated);
if (scrollUpdated) {
if (aMetrics.IsScrollInfoLayer()) {
// In cases where the APZ scroll offset is different from the content scroll
// offset, we want to interpret the margins as relative to the APZ scroll
// offset except when the frame is not scrollable by APZ. Therefore, if the
// layer is a scroll info layer, we leave the margins as-is and they will
// be interpreted as relative to the content scroll offset.