Bug 963278 - Better fix to deal with concurrent scroll updates from APZ and other places. r=tn,botond,jimm

This commit is contained in:
Kartikaya Gupta 2014-02-05 17:43:20 -05:00
parent cdac679592
commit 5fde241c65
22 changed files with 193 additions and 32 deletions

View File

@ -344,6 +344,13 @@ child:
UpdateFrame(FrameMetrics frame) compress;
/**
* Acknowledge the receipt of a scroll offset update from the content
* process. This is used to manage concurrent scroll updates from many
* sources.
*/
AcknowledgeScrollUpdate(ViewID aScrollId, uint32_t aScrollGeneration);
/**
* Requests handling of a double tap. |point| is in CSS pixels, relative to
* the scroll offset. This message is expected to round-trip back to

View File

@ -1517,6 +1517,14 @@ TabChild::RecvUpdateFrame(const FrameMetrics& aFrameMetrics)
return ProcessUpdateFrame(mLastRootMetrics);
}
bool
TabChild::RecvAcknowledgeScrollUpdate(const ViewID& aScrollId,
const uint32_t& aScrollGeneration)
{
APZCCallbackHelper::AcknowledgeScrollUpdate(aScrollId, aScrollGeneration);
return true;
}
bool
TabChild::ProcessUpdateFrame(const FrameMetrics& aFrameMetrics)
{

View File

@ -220,6 +220,8 @@ public:
const nsIntSize& size,
const ScreenOrientation& orientation) MOZ_OVERRIDE;
virtual bool RecvUpdateFrame(const mozilla::layers::FrameMetrics& aFrameMetrics) MOZ_OVERRIDE;
virtual bool RecvAcknowledgeScrollUpdate(const ViewID& aScrollId,
const uint32_t& aScrollGeneration) MOZ_OVERRIDE;
virtual bool RecvHandleDoubleTap(const CSSIntPoint& aPoint) MOZ_OVERRIDE;
virtual bool RecvHandleSingleTap(const CSSIntPoint& aPoint) MOZ_OVERRIDE;
virtual bool RecvHandleLongTap(const CSSIntPoint& aPoint) MOZ_OVERRIDE;

View File

@ -506,6 +506,14 @@ TabParent::UpdateFrame(const FrameMetrics& aFrameMetrics)
}
}
void
TabParent::AcknowledgeScrollUpdate(const ViewID& aScrollId, const uint32_t& aScrollGeneration)
{
if (!mIsDestroyed) {
unused << SendAcknowledgeScrollUpdate(aScrollId, aScrollGeneration);
}
}
void TabParent::HandleDoubleTap(const CSSIntPoint& aPoint, int32_t aModifiers)
{
if (!mIsDestroyed) {

View File

@ -206,6 +206,7 @@ public:
void Show(const nsIntSize& size);
void UpdateDimensions(const nsRect& rect, const nsIntSize& size);
void UpdateFrame(const layers::FrameMetrics& aFrameMetrics);
void AcknowledgeScrollUpdate(const ViewID& aScrollId, const uint32_t& aScrollGeneration);
void HandleDoubleTap(const CSSIntPoint& aPoint, int32_t aModifiers);
void HandleSingleTap(const CSSIntPoint& aPoint, int32_t aModifiers);
void HandleLongTap(const CSSIntPoint& aPoint, int32_t aModifiers);

View File

@ -60,9 +60,10 @@ public:
, mMayHaveTouchListeners(false)
, mIsRoot(false)
, mHasScrollgrab(false)
, mUpdateScrollOffset(false)
, mDisableScrollingX(false)
, mDisableScrollingY(false)
, mUpdateScrollOffset(false)
, mScrollGeneration(0)
{}
// Default copy ctor and operator= are fine
@ -298,10 +299,6 @@ public:
// Whether or not this frame is for an element marked 'scrollgrab'.
bool mHasScrollgrab;
// Whether mScrollOffset was updated by something other than the APZ code, and
// if the APZC receiving this metrics should update its local copy.
bool mUpdateScrollOffset;
public:
bool GetDisableScrollingX() const
{
@ -323,6 +320,22 @@ public:
mDisableScrollingY = aDisableScrollingY;
}
void SetScrollOffsetUpdated(uint32_t aScrollGeneration)
{
mUpdateScrollOffset = true;
mScrollGeneration = aScrollGeneration;
}
bool GetScrollOffsetUpdated() const
{
return mUpdateScrollOffset;
}
uint32_t GetScrollGeneration() const
{
return mScrollGeneration;
}
private:
// New fields from now on should be made private and old fields should
// be refactored to be private.
@ -331,6 +344,12 @@ private:
// |overflow: hidden|.
bool mDisableScrollingX;
bool mDisableScrollingY;
// Whether mScrollOffset was updated by something other than the APZ code, and
// if the APZC receiving this metrics should update its local copy.
bool mUpdateScrollOffset;
// The scroll generation counter used to acknowledge the scroll offset update.
uint32_t mScrollGeneration;
};
/**

View File

@ -75,7 +75,7 @@
fm.mScrollOffset.x, fm.mScrollOffset.y, \
fm.mScrollableRect.x, fm.mScrollableRect.y, fm.mScrollableRect.width, fm.mScrollableRect.height, \
fm.mDevPixelsPerCSSPixel.scale, fm.mResolution.scale, fm.mCumulativeResolution.scale, fm.mZoom.scale, \
fm.mUpdateScrollOffset); \
fm.GetScrollOffsetUpdated()); \
// Static helper functions
namespace {
@ -1637,23 +1637,22 @@ void AsyncPanZoomController::NotifyLayersUpdated(const FrameMetrics& aLayerMetri
// If the layers update was not triggered by our own repaint request, then
// we want to take the new scroll offset.
if (aLayerMetrics.mUpdateScrollOffset) {
if (aLayerMetrics.GetScrollOffsetUpdated()) {
APZC_LOG("%p updating scroll offset from (%f, %f) to (%f, %f)\n", this,
mFrameMetrics.mScrollOffset.x, mFrameMetrics.mScrollOffset.y,
aLayerMetrics.mScrollOffset.x, aLayerMetrics.mScrollOffset.y);
mFrameMetrics.mScrollOffset = aLayerMetrics.mScrollOffset;
// It is possible that when we receive this mUpdateScrollOffset flag, we have
// just sent a content repaint request, and it is pending inflight. That repaint
// request would have our old scroll offset, and will get processed on the content
// thread as we're processing this mUpdateScrollOffset flag. This would leave
// things in a state where content has the old APZC scroll offset and the APZC
// has the new content-specified scroll offset. In such a case we want to trigger
// another repaint request to bring things back in sync. In most cases this repaint
// request will be a no-op and get filtered out in RequestContentRepaint, so it
// shouldn't have bad performance implications.
needContentRepaint = true;
// Once layout issues a scroll offset update, it becomes impervious to
// scroll offset updates from APZ until we acknowledge the update it sent.
// This prevents APZ updates from clobbering scroll updates from other
// more "legitimate" sources like content scripts.
nsRefPtr<GeckoContentController> controller = GetGeckoContentController();
if (controller) {
controller->AcknowledgeScrollUpdate(aLayerMetrics.mScrollId,
aLayerMetrics.GetScrollGeneration());
}
}
}

View File

@ -28,6 +28,14 @@ public:
*/
virtual void RequestContentRepaint(const FrameMetrics& aFrameMetrics) = 0;
/**
* Acknowledges the recipt of a scroll offset update for the scrollable
* frame with the given scroll id. This is used to maintain consistency
* between APZ and other sources of scroll changes.
*/
virtual void AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration) = 0;
/**
* Requests handling of a double tap. |aPoint| is in CSS pixels, relative to
* the current scroll offset. This should eventually round-trip back to

View File

@ -30,6 +30,7 @@ class Task;
class MockContentController : public GeckoContentController {
public:
MOCK_METHOD1(RequestContentRepaint, void(const FrameMetrics&));
MOCK_METHOD2(AcknowledgeScrollUpdate, void(const FrameMetrics::ViewID&, const uint32_t& aScrollGeneration));
MOCK_METHOD2(HandleDoubleTap, void(const CSSIntPoint&, int32_t));
MOCK_METHOD2(HandleSingleTap, void(const CSSIntPoint&, int32_t));
MOCK_METHOD2(HandleLongTap, void(const CSSIntPoint&, int32_t));

View File

@ -642,8 +642,9 @@ static void RecordFrameMetrics(nsIFrame* aForFrame,
// something other than the APZ code, we want to tell the APZ to update
// its scroll offset.
nsIAtom* originOfLastScroll = scrollableFrame->OriginOfLastScroll();
metrics.mUpdateScrollOffset = (originOfLastScroll && originOfLastScroll != nsGkAtoms::apz);
scrollableFrame->ResetOriginOfLastScroll();
if (originOfLastScroll && originOfLastScroll != nsGkAtoms::apz) {
metrics.SetScrollOffsetUpdated(scrollableFrame->CurrentScrollGeneration());
}
}
else {
nsRect contentBounds = aForFrame->GetRect();

View File

@ -1538,6 +1538,7 @@ ScrollFrameHelper::ScrollFrameHelper(nsContainerFrame* aOuter,
, mOuter(aOuter)
, mAsyncScroll(nullptr)
, mOriginOfLastScroll(nullptr)
, mScrollGeneration(0)
, mDestination(0, 0)
, mScrollPosAtLastPaint(0, 0)
, mRestorePos(-1, -1)
@ -2053,6 +2054,7 @@ ScrollFrameHelper::ScrollToImpl(nsPoint aPt, const nsRect& aRange, nsIAtom* aOri
// Update frame position for scrolling
mScrolledFrame->SetPosition(mScrollPort.TopLeft() - pt);
mOriginOfLastScroll = aOrigin;
mScrollGeneration++;
// We pass in the amount to move visually
ScrollVisual(oldScrollFramePos);

View File

@ -302,7 +302,12 @@ public:
void HandleScrollbarStyleSwitching();
nsIAtom* OriginOfLastScroll() const { return mOriginOfLastScroll; }
void ResetOriginOfLastScroll() { mOriginOfLastScroll = nullptr; }
uint32_t CurrentScrollGeneration() const { return mScrollGeneration; }
void ResetOriginIfScrollAtGeneration(uint32_t aGeneration) {
if (aGeneration == mScrollGeneration) {
mOriginOfLastScroll = nullptr;
}
}
// owning references to the nsIAnonymousContentCreator-built content
nsCOMPtr<nsIContent> mHScrollbarContent;
@ -323,6 +328,7 @@ public:
nsRefPtr<ScrollbarActivity> mScrollbarActivity;
nsTArray<nsIScrollPositionListener*> mListeners;
nsIAtom* mOriginOfLastScroll;
uint32_t mScrollGeneration;
nsRect mScrollPort;
// Where we're currently scrolling to, if we're scrolling asynchronously.
// If we're not in the middle of an asynchronous scroll then this is
@ -649,8 +655,11 @@ public:
virtual nsIAtom* OriginOfLastScroll() MOZ_OVERRIDE {
return mHelper.OriginOfLastScroll();
}
virtual void ResetOriginOfLastScroll() MOZ_OVERRIDE {
mHelper.ResetOriginOfLastScroll();
virtual uint32_t CurrentScrollGeneration() MOZ_OVERRIDE {
return mHelper.CurrentScrollGeneration();
}
virtual void ResetOriginIfScrollAtGeneration(uint32_t aGeneration) MOZ_OVERRIDE {
mHelper.ResetOriginIfScrollAtGeneration(aGeneration);
}
// nsIStatefulFrame
@ -950,8 +959,11 @@ public:
virtual nsIAtom* OriginOfLastScroll() MOZ_OVERRIDE {
return mHelper.OriginOfLastScroll();
}
virtual void ResetOriginOfLastScroll() MOZ_OVERRIDE {
mHelper.ResetOriginOfLastScroll();
virtual uint32_t CurrentScrollGeneration() MOZ_OVERRIDE {
return mHelper.CurrentScrollGeneration();
}
virtual void ResetOriginIfScrollAtGeneration(uint32_t aGeneration) MOZ_OVERRIDE {
mHelper.ResetOriginIfScrollAtGeneration(aGeneration);
}
// nsIStatefulFrame

View File

@ -281,9 +281,16 @@ public:
*/
virtual nsIAtom* OriginOfLastScroll() = 0;
/**
* Clears the "origin of last scroll" property stored in this frame.
* Returns the current generation counter for the scroll. This counter
* increments every time the scroll position is set.
*/
virtual void ResetOriginOfLastScroll() = 0;
virtual uint32_t CurrentScrollGeneration() = 0;
/**
* Clears the "origin of last scroll" property stored in this frame, if
* the generation counter passed in matches the current scroll generation
* counter.
*/
virtual void ResetOriginIfScrollAtGeneration(uint32_t aGeneration) = 0;
};
#endif

View File

@ -521,6 +521,24 @@ public:
aFrameMetrics));
}
virtual void AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration) MOZ_OVERRIDE
{
if (MessageLoop::current() != mUILoop) {
// We have to send this message from the "UI thread" (main
// thread).
mUILoop->PostTask(
FROM_HERE,
NewRunnableMethod(this, &RemoteContentController::AcknowledgeScrollUpdate,
aScrollId, aScrollGeneration));
return;
}
if (mRenderFrame) {
TabParent* browser = static_cast<TabParent*>(mRenderFrame->Manager());
browser->AcknowledgeScrollUpdate(aScrollId, aScrollGeneration);
}
}
virtual void HandleDoubleTap(const CSSIntPoint& aPoint,
int32_t aModifiers) MOZ_OVERRIDE
{

View File

@ -1930,6 +1930,13 @@ AndroidBridge::RequestContentRepaint(const mozilla::layers::FrameMetrics& aFrame
mNativePanZoomController->RequestContentRepaintWrapper(dp.x, dp.y, dp.width, dp.height, resolution.scale);
}
void
AndroidBridge::AcknowledgeScrollUpdate(const mozilla::layers::FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration)
{
// FIXME implement this
}
void
AndroidBridge::HandleDoubleTap(const CSSIntPoint& aPoint, int32_t aModifiers)
{

View File

@ -431,6 +431,8 @@ public:
NativePanZoomController* SetNativePanZoomController(jobject obj);
// GeckoContentController methods
void RequestContentRepaint(const mozilla::layers::FrameMetrics& aFrameMetrics) MOZ_OVERRIDE;
void AcknowledgeScrollUpdate(const mozilla::layers::FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration) MOZ_OVERRIDE;
void HandleDoubleTap(const CSSIntPoint& aPoint, int32_t aModifiers) MOZ_OVERRIDE;
void HandleSingleTap(const CSSIntPoint& aPoint, int32_t aModifiers) MOZ_OVERRIDE;
void HandleLongTap(const CSSIntPoint& aPoint, int32_t aModifiers) MOZ_OVERRIDE;

View File

@ -50,6 +50,13 @@ ParentProcessController::RequestContentRepaint(const FrameMetrics& aFrameMetrics
}
}
void
ParentProcessController::AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration)
{
APZCCallbackHelper::AcknowledgeScrollUpdate(aScrollId, aScrollGeneration);
}
void
ParentProcessController::PostDelayedTask(Task* aTask, int aDelayMs)
{

View File

@ -17,6 +17,8 @@ class ParentProcessController : public mozilla::layers::GeckoContentController
public:
virtual void RequestContentRepaint(const FrameMetrics& aFrameMetrics) MOZ_OVERRIDE;
virtual void AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aViewId,
const uint32_t& aScrollGeneration) MOZ_OVERRIDE;
virtual void PostDelayedTask(Task* aTask, int aDelayMs) MOZ_OVERRIDE;
// No-ops

View File

@ -232,6 +232,17 @@ APZController::RequestContentRepaint(const FrameMetrics& aFrameMetrics)
}
}
void
APZController::AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration)
{
#ifdef DEBUG_CONTROLLER
WinUtils::Log("APZController::AcknowledgeScrollUpdate scrollid=%I64d gen=%lu",
aScrollId, aScrollGeneration);
#endif
APZCCallbackHelper::AcknowledgeScrollUpdate(aScrollId, aScrollGeneration);
}
void
APZController::HandleDoubleTap(const CSSIntPoint& aPoint, int32_t aModifiers)
{

View File

@ -33,6 +33,7 @@ public:
// GeckoContentController interface
virtual void RequestContentRepaint(const FrameMetrics& aFrameMetrics);
virtual void AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId, const uint32_t& aScrollGeneration);
virtual void HandleDoubleTap(const mozilla::CSSIntPoint& aPoint, int32_t aModifiers);
virtual void HandleSingleTap(const mozilla::CSSIntPoint& aPoint, int32_t aModifiers);
virtual void HandleLongTap(const mozilla::CSSIntPoint& aPoint, int32_t aModifiers);

View File

@ -97,12 +97,6 @@ ScrollFrameTo(nsIScrollableFrame* aFrame, const CSSPoint& aPoint)
// Also if the scrollable frame got a scroll request from something other than us
// since the last layers update, then we don't want to push our scroll request
// because we'll clobber that one, which is bad.
// Note that content may have just finished sending a layers update with a scroll
// offset update to the APZ, in which case the origin will be reset to null and we
// might actually be clobbering the content-side scroll offset with a stale APZ
// scroll offset. This is unavoidable because of the async communication between
// APZ and content; however the code in NotifyLayersUpdated should reissue a new
// repaint request to bring everything back into sync.
if (!aFrame->IsProcessingAsyncScroll() &&
(!aFrame->OriginOfLastScroll() || aFrame->OriginOfLastScroll() == nsGkAtoms::apz)) {
aFrame->ScrollToCSSPixelsApproximate(aPoint, nsGkAtoms::apz);
@ -242,5 +236,44 @@ APZCCallbackHelper::GetScrollIdentifiers(const nsIContent* aContent,
return utils && (utils->GetPresShellId(aPresShellIdOut) == NS_OK);
}
class AcknowledgeScrollUpdateEvent : public nsRunnable
{
typedef mozilla::layers::FrameMetrics::ViewID ViewID;
public:
AcknowledgeScrollUpdateEvent(const ViewID& aScrollId, const uint32_t& aScrollGeneration)
: mScrollId(aScrollId)
, mScrollGeneration(aScrollGeneration)
{
}
NS_IMETHOD Run() {
MOZ_ASSERT(NS_IsMainThread());
nsIScrollableFrame* sf = nsLayoutUtils::FindScrollableFrameFor(mScrollId);
if (sf) {
sf->ResetOriginIfScrollAtGeneration(mScrollGeneration);
}
return NS_OK;
}
protected:
ViewID mScrollId;
uint32_t mScrollGeneration;
};
void
APZCCallbackHelper::AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration)
{
nsCOMPtr<nsIRunnable> r1 = new AcknowledgeScrollUpdateEvent(aScrollId, aScrollGeneration);
if (!NS_IsMainThread()) {
NS_DispatchToMainThread(r1);
} else {
r1->Run();
}
}
}
}

View File

@ -62,6 +62,11 @@ public:
static bool GetScrollIdentifiers(const nsIContent* aContent,
uint32_t* aPresShellIdOut,
FrameMetrics::ViewID* aViewIdOut);
/* Tell layout that we received the scroll offset update for the given view ID, so
that it accepts future scroll offset updates from APZ. */
static void AcknowledgeScrollUpdate(const FrameMetrics::ViewID& aScrollId,
const uint32_t& aScrollGeneration);
};
}