From a850d5d255d8a82cdcfac10129e01509a6f7db7e Mon Sep 17 00:00:00 2001 From: "kin%netscape.com" Date: Mon, 19 Aug 2002 04:41:57 +0000 Subject: [PATCH] Fix for bug #96736: [FLOATER]The "Top 1000 Reviewer" is overwritten by text mozilla/layout/base/src/nsSpaceManager.cpp mozilla/layout/base/src/nsSpaceManager.h - Implemented nsSpaceManager methods PushState() and PopState(). mozilla/layout/html/base/src/nsBlockReflowContext.cpp - Modified nsBlockReflowContext::DoReflowBlock() to call PushState() and PopState() to remove any floaters that may have been added to the SpaceManager during the intermediate "unconstrained" Reflow() call, used to calculate the max block width. This allows the Reflow() call that immediately follows it to properly place the floaters. r=waterson@netscape.com sr=dbaron@fas.harvard.edu git-svn-id: svn://10.0.0.236/trunk@127595 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/layout/base/src/nsSpaceManager.cpp | 95 +++++++++++++++++++ mozilla/layout/base/src/nsSpaceManager.h | 24 +++++ .../layout/generic/nsBlockReflowContext.cpp | 4 + mozilla/layout/generic/nsSpaceManager.cpp | 95 +++++++++++++++++++ mozilla/layout/generic/nsSpaceManager.h | 24 +++++ .../html/base/src/nsBlockReflowContext.cpp | 4 + 6 files changed, 246 insertions(+) diff --git a/mozilla/layout/base/src/nsSpaceManager.cpp b/mozilla/layout/base/src/nsSpaceManager.cpp index 061ef410767..26b2eb4e5fe 100644 --- a/mozilla/layout/base/src/nsSpaceManager.cpp +++ b/mozilla/layout/base/src/nsSpaceManager.cpp @@ -111,6 +111,7 @@ nsSpaceManager::nsSpaceManager(nsIPresShell* aPresShell, nsIFrame* aFrame) MOZ_COUNT_CTOR(nsSpaceManager); mX = mY = 0; mFrameInfoMap = nsnull; + mSavedStates = nsnull; } void @@ -128,6 +129,14 @@ nsSpaceManager::~nsSpaceManager() MOZ_COUNT_DTOR(nsSpaceManager); mBandList.Clear(); ClearFrameInfo(); + + NS_ASSERTION(!mSavedStates, "states remaining on state stack"); + + while (mSavedStates){ + SpaceManagerState *state = mSavedStates; + mSavedStates = state->mNext; + delete state; + } } // static @@ -983,6 +992,92 @@ nsSpaceManager::ClearRegions() mBandList.Clear(); } +void +nsSpaceManager::PushState() +{ + // This is a quick and dirty push implementation, which + // only saves the (x,y) and last frame in the mFrameInfoMap + // which is enough info to get us back to where we should be + // when pop is called. + // + // The alternative would be to make full copies of the contents + // of mBandList and mFrameInfoMap and restore them when pop is + // called, but I'm not sure it's worth the effort/bloat at this + // point, since this push/pop mechanism is only used to undo any + // floaters that were added during the unconstrained reflow + // in nsBlockReflowContext::DoReflowBlock(). (See bug 96736) + // + // It should also be noted that the state for mFloatDamage is + // intentionally not saved or restored in PushState() and PopState(), + // since that could lead to bugs where damage is missed/dropped when + // we move from position A to B (during the intermediate incremental + // reflow mentioned above) and then from B to C during the subsequent + // reflow. In the typical case A and C will be the same, but not always. + // Allowing mFloatDamage to accumulate the damage incurred during both + // reflows ensures that nothing gets missed. + + SpaceManagerState *state = new SpaceManagerState; + + NS_ASSERTION(state, "PushState() failed!"); + + if (!state) { + return; + } + + state->mX = mX; + state->mY = mY; + + if (mFrameInfoMap) { + state->mLastFrame = mFrameInfoMap->mFrame; + } + + // Now that we've saved our state, add it to mSavedStates. + + state->mNext = mSavedStates; + mSavedStates = state; +} + +void +nsSpaceManager::PopState() +{ + // This is a quick and dirty pop implementation, to + // match the current implementation of PushState(). The + // idea here is to remove any frames that have been added + // to the mFrameInfoMap since the last call to PushState(). + + NS_ASSERTION(mSavedStates, "Invalid call to PopState()!"); + + if (!mSavedStates) { + return; + } + + // mFrameInfoMap is LIFO so keep removing what it points + // to until we hit mLastFrame. + + while (mFrameInfoMap && mFrameInfoMap->mFrame != mSavedStates->mLastFrame) { + RemoveRegion(mFrameInfoMap->mFrame); + } + + // If we trip this assertion it means that someone added + // PushState()/PopState() calls around code that actually + // removed mLastFrame from mFrameInfoMap, which means our + // state is now out of sync with what we thought it should be. + + NS_ASSERTION(((mSavedStates->mLastFrame && mFrameInfoMap) || + (!mSavedStates->mLastFrame && !mFrameInfoMap)), + "Unexpected outcome!"); + + mX = mSavedStates->mX; + mY = mSavedStates->mY; + + // Now that we've restored our state, pop the topmost + // state and delete it. + + SpaceManagerState *state = mSavedStates; + mSavedStates = mSavedStates->mNext; + delete state; +} + #ifdef DEBUG void DebugListSpaceManager(nsSpaceManager *aSpaceManager) diff --git a/mozilla/layout/base/src/nsSpaceManager.h b/mozilla/layout/base/src/nsSpaceManager.h index 2441fabbe3c..6c2640b8d49 100644 --- a/mozilla/layout/base/src/nsSpaceManager.h +++ b/mozilla/layout/base/src/nsSpaceManager.h @@ -293,6 +293,17 @@ public: return mFloatDamage.Intersects(aIntervalBegin + mY, aIntervalEnd + mY); } + /** + * Pushes the current state of the space manager onto a state stack. + */ + void PushState(); + + /** + * Restores the space manager to the state at the top of the state stack, + * then pops this state off the stack. + */ + void PopState(); + #ifdef DEBUG /** * Dump the state of the spacemanager out to a file @@ -316,6 +327,17 @@ protected: #endif }; + // Structure that stores the current state of a frame manager for + // Save/Restore purposes. + struct SpaceManagerState { + nscoord mX, mY; + nsIFrame *mLastFrame; + SpaceManagerState *mNext; + + SpaceManagerState() : mX(0), mY(0), mLastFrame(nsnull), mNext(nsnull) {} + ~SpaceManagerState() {} + }; + public: // Doubly linked list of band rects struct BandRect : PRCListStr { @@ -387,6 +409,8 @@ protected: FrameInfo* mFrameInfoMap; nsIntervalSet mFloatDamage; + SpaceManagerState *mSavedStates; + protected: FrameInfo* GetFrameInfoFor(nsIFrame* aFrame); FrameInfo* CreateFrameInfo(nsIFrame* aFrame, const nsRect& aRect); diff --git a/mozilla/layout/generic/nsBlockReflowContext.cpp b/mozilla/layout/generic/nsBlockReflowContext.cpp index 8ec85c0e17a..398f0b5d663 100644 --- a/mozilla/layout/generic/nsBlockReflowContext.cpp +++ b/mozilla/layout/generic/nsBlockReflowContext.cpp @@ -544,6 +544,8 @@ nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState &aReflowState, // See if this is the child's initial reflow and we are supposed to // compute our maximum width if (mComputeMaximumWidth && (eReflowReason_Initial == aReason)) { + mOuterReflowState.mSpaceManager->PushState(); + nscoord oldAvailableWidth = aReflowState.availableWidth; nscoord oldComputedWidth = aReflowState.mComputedWidth; @@ -563,6 +565,8 @@ nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState &aReflowState, aReflowState.availableWidth = oldAvailableWidth; aReflowState.mComputedWidth = oldComputedWidth; aReason = eReflowReason_Resize; + + mOuterReflowState.mSpaceManager->PopState(); } rv = aFrame->Reflow(mPresContext, mMetrics, aReflowState, diff --git a/mozilla/layout/generic/nsSpaceManager.cpp b/mozilla/layout/generic/nsSpaceManager.cpp index 061ef410767..26b2eb4e5fe 100644 --- a/mozilla/layout/generic/nsSpaceManager.cpp +++ b/mozilla/layout/generic/nsSpaceManager.cpp @@ -111,6 +111,7 @@ nsSpaceManager::nsSpaceManager(nsIPresShell* aPresShell, nsIFrame* aFrame) MOZ_COUNT_CTOR(nsSpaceManager); mX = mY = 0; mFrameInfoMap = nsnull; + mSavedStates = nsnull; } void @@ -128,6 +129,14 @@ nsSpaceManager::~nsSpaceManager() MOZ_COUNT_DTOR(nsSpaceManager); mBandList.Clear(); ClearFrameInfo(); + + NS_ASSERTION(!mSavedStates, "states remaining on state stack"); + + while (mSavedStates){ + SpaceManagerState *state = mSavedStates; + mSavedStates = state->mNext; + delete state; + } } // static @@ -983,6 +992,92 @@ nsSpaceManager::ClearRegions() mBandList.Clear(); } +void +nsSpaceManager::PushState() +{ + // This is a quick and dirty push implementation, which + // only saves the (x,y) and last frame in the mFrameInfoMap + // which is enough info to get us back to where we should be + // when pop is called. + // + // The alternative would be to make full copies of the contents + // of mBandList and mFrameInfoMap and restore them when pop is + // called, but I'm not sure it's worth the effort/bloat at this + // point, since this push/pop mechanism is only used to undo any + // floaters that were added during the unconstrained reflow + // in nsBlockReflowContext::DoReflowBlock(). (See bug 96736) + // + // It should also be noted that the state for mFloatDamage is + // intentionally not saved or restored in PushState() and PopState(), + // since that could lead to bugs where damage is missed/dropped when + // we move from position A to B (during the intermediate incremental + // reflow mentioned above) and then from B to C during the subsequent + // reflow. In the typical case A and C will be the same, but not always. + // Allowing mFloatDamage to accumulate the damage incurred during both + // reflows ensures that nothing gets missed. + + SpaceManagerState *state = new SpaceManagerState; + + NS_ASSERTION(state, "PushState() failed!"); + + if (!state) { + return; + } + + state->mX = mX; + state->mY = mY; + + if (mFrameInfoMap) { + state->mLastFrame = mFrameInfoMap->mFrame; + } + + // Now that we've saved our state, add it to mSavedStates. + + state->mNext = mSavedStates; + mSavedStates = state; +} + +void +nsSpaceManager::PopState() +{ + // This is a quick and dirty pop implementation, to + // match the current implementation of PushState(). The + // idea here is to remove any frames that have been added + // to the mFrameInfoMap since the last call to PushState(). + + NS_ASSERTION(mSavedStates, "Invalid call to PopState()!"); + + if (!mSavedStates) { + return; + } + + // mFrameInfoMap is LIFO so keep removing what it points + // to until we hit mLastFrame. + + while (mFrameInfoMap && mFrameInfoMap->mFrame != mSavedStates->mLastFrame) { + RemoveRegion(mFrameInfoMap->mFrame); + } + + // If we trip this assertion it means that someone added + // PushState()/PopState() calls around code that actually + // removed mLastFrame from mFrameInfoMap, which means our + // state is now out of sync with what we thought it should be. + + NS_ASSERTION(((mSavedStates->mLastFrame && mFrameInfoMap) || + (!mSavedStates->mLastFrame && !mFrameInfoMap)), + "Unexpected outcome!"); + + mX = mSavedStates->mX; + mY = mSavedStates->mY; + + // Now that we've restored our state, pop the topmost + // state and delete it. + + SpaceManagerState *state = mSavedStates; + mSavedStates = mSavedStates->mNext; + delete state; +} + #ifdef DEBUG void DebugListSpaceManager(nsSpaceManager *aSpaceManager) diff --git a/mozilla/layout/generic/nsSpaceManager.h b/mozilla/layout/generic/nsSpaceManager.h index 2441fabbe3c..6c2640b8d49 100644 --- a/mozilla/layout/generic/nsSpaceManager.h +++ b/mozilla/layout/generic/nsSpaceManager.h @@ -293,6 +293,17 @@ public: return mFloatDamage.Intersects(aIntervalBegin + mY, aIntervalEnd + mY); } + /** + * Pushes the current state of the space manager onto a state stack. + */ + void PushState(); + + /** + * Restores the space manager to the state at the top of the state stack, + * then pops this state off the stack. + */ + void PopState(); + #ifdef DEBUG /** * Dump the state of the spacemanager out to a file @@ -316,6 +327,17 @@ protected: #endif }; + // Structure that stores the current state of a frame manager for + // Save/Restore purposes. + struct SpaceManagerState { + nscoord mX, mY; + nsIFrame *mLastFrame; + SpaceManagerState *mNext; + + SpaceManagerState() : mX(0), mY(0), mLastFrame(nsnull), mNext(nsnull) {} + ~SpaceManagerState() {} + }; + public: // Doubly linked list of band rects struct BandRect : PRCListStr { @@ -387,6 +409,8 @@ protected: FrameInfo* mFrameInfoMap; nsIntervalSet mFloatDamage; + SpaceManagerState *mSavedStates; + protected: FrameInfo* GetFrameInfoFor(nsIFrame* aFrame); FrameInfo* CreateFrameInfo(nsIFrame* aFrame, const nsRect& aRect); diff --git a/mozilla/layout/html/base/src/nsBlockReflowContext.cpp b/mozilla/layout/html/base/src/nsBlockReflowContext.cpp index 8ec85c0e17a..398f0b5d663 100644 --- a/mozilla/layout/html/base/src/nsBlockReflowContext.cpp +++ b/mozilla/layout/html/base/src/nsBlockReflowContext.cpp @@ -544,6 +544,8 @@ nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState &aReflowState, // See if this is the child's initial reflow and we are supposed to // compute our maximum width if (mComputeMaximumWidth && (eReflowReason_Initial == aReason)) { + mOuterReflowState.mSpaceManager->PushState(); + nscoord oldAvailableWidth = aReflowState.availableWidth; nscoord oldComputedWidth = aReflowState.mComputedWidth; @@ -563,6 +565,8 @@ nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState &aReflowState, aReflowState.availableWidth = oldAvailableWidth; aReflowState.mComputedWidth = oldComputedWidth; aReason = eReflowReason_Resize; + + mOuterReflowState.mSpaceManager->PopState(); } rv = aFrame->Reflow(mPresContext, mMetrics, aReflowState,