Bommels05/Mozilla
Bommels05/Mozilla
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing bug 381285. 323656-5-ref.svg triggers "ASSERTION: can't mark frame dirty during reflow". r+sr=roc@ocallahan.org, a1.9=mtschrep@gmail.com

Browse Source 
git-svn-id: svn://10.0.0.236/trunk@239704 18797224-902f-48f8-a5cc-f745e15eee43
This commit is contained in:
jwatt%jwatt.org 2007-11-20 09:10:19 +00:00
parent cc08e41dea
commit 59572fadea
7 changed files with 220 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
mozilla/layout/reftests/svg/foreignObject-ancestor-style-change-01-ref.svg Executable file
View File

@ -0,0 +1,27 @@
<!--
Any copyright is dedicated to the Public Domain.
http://creativecommons.org/licenses/publicdomain/
-->
<svg xmlns="http://www.w3.org/2000/svg" version="1.1">
<!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=381285 -->
<title>Reference for foreignObject-ancestor-style-change-01.svg</title>
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml"
style="width: 100%; height: 100%; font-size: 16px;">
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
</div>
</foreignObject>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 1.2 KiB

54
mozilla/layout/reftests/svg/foreignObject-ancestor-style-change-01.svg Executable file
View File

@ -0,0 +1,54 @@
<!--
Any copyright is dedicated to the Public Domain.
http://creativecommons.org/licenses/publicdomain/
-->
<svg xmlns="http://www.w3.org/2000/svg" version="1.1"
onload="handle_load();" class="reftest-wait">
<!-- From https://bugzilla.mozilla.org/show_bug.cgi?id=381285 -->
<title>Testcase for style change on foreignObject ancestor</title>
<!--
This testcase checks that foreignObject content is correctly updated when
a style change that requires layout changes occurs on an ancestor.
-->
<script>
function handle_load()
{
setTimeout(change_font_size, 50); // allow some time for layout and rendering
}
function change_font_size()
{
document.getElementById('g').style.fontSize = '16px';
setTimeout(notify_test_finished, 50); // allow some time for layout and rendering
}
function notify_test_finished()
{
document.documentElement.removeAttribute('class');
}
</script>
<g id="g" style="font-size: 26px;">
<foreignObject width="100%" height="100%">
<div xmlns="http://www.w3.org/1999/xhtml" style="width: 100%; height: 100%;">
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
Padding text. Padding text. Padding text. Padding text. Padding text.
</div>
</foreignObject>
</g>
</svg>
Side by Side

After

Width:  |  Height:  |  Size: 1.9 KiB

1
mozilla/layout/reftests/svg/reftest.list
View File

@ -7,6 +7,7 @@ include sizing/reftest.list
fails-if(MOZ_WIDGET_TOOLKIT=="cocoa") == clipPath-basic-01.svg pass.svg # bug 379609
== foreignObject-01.svg pass.svg
== foreignObject-ancestor-style-change-01.svg foreignObject-ancestor-style-change-01-ref.svg
== foreignObject-change-transform-01.svg pass.svg
== foreignObject-display-01.svg pass.svg
== foreignObject-move-repaint-01.svg pass.svg

104
mozilla/layout/svg/base/src/nsSVGForeignObjectFrame.cpp
View File

@ -91,8 +91,21 @@ NS_INTERFACE_MAP_END_INHERITING(nsSVGForeignObjectFrameBase)
//----------------------------------------------------------------------
// nsIFrame methods
NS_IMETHODIMP
nsSVGForeignObjectFrame::Init(nsIContent* aContent,
nsIFrame* aParent,
nsIFrame* aPrevInFlow)
{
nsresult rv = nsSVGForeignObjectFrameBase::Init(aContent, aParent, aPrevInFlow);
if (NS_SUCCEEDED(rv)) {
nsSVGUtils::GetOuterSVGFrame(this)->RegisterForeignObject(this);
}
return rv;
}
void nsSVGForeignObjectFrame::Destroy()
{
nsSVGUtils::GetOuterSVGFrame(this)->UnregisterForeignObject(this);
// Delete any clipPath/filter/mask properties _before_ we die. The properties
// and property hash table have weak pointers to us that are dereferenced
// when the properties are destroyed.
@ -115,7 +128,7 @@ nsSVGForeignObjectFrame::AttributeChanged(PRInt32 aNameSpaceID,
if (aAttribute == nsGkAtoms::width ||
aAttribute == nsGkAtoms::height) {
UpdateGraphic(); // update mRect before requesting reflow
// XXXjwatt: why are we calling MarkIntrinsicWidthsDirty on our ancestors???
// XXXjwatt: why mark intrinsic widths dirty? can't we just use eResize?
RequestReflow(nsIPresShell::eStyleChange);
} else if (aAttribute == nsGkAtoms::x ||
aAttribute == nsGkAtoms::y) {
@ -137,23 +150,6 @@ nsSVGForeignObjectFrame::DidSetStyleContext()
return NS_OK;
}
/* virtual */ void
nsSVGForeignObjectFrame::MarkIntrinsicWidthsDirty()
{
// Since we don't know whether this call is because of a style change on an
// ancestor or a descendant, mark the kid dirty. If it's a descendant,
// all we need is the NS_FRAME_HAS_DIRTY_CHILDREN that our caller is
// going to set, though. (If we could differentiate between a style change on
// an ancestor or descendant, we'd need to add a parameter to RequestReflow
// to pass either NS_FRAME_IS_DIRTY or NS_FRAME_HAS_DIRTY_CHILDREN.)
//
// This is really a style change, except we're already being called
// from MarkIntrinsicWidthsDirty, so say it's a resize to avoid doing
// the same work over again.
RequestReflow(nsIPresShell::eResize);
}
NS_IMETHODIMP
nsSVGForeignObjectFrame::Reflow(nsPresContext* aPresContext,
nsHTMLReflowMetrics& aDesiredSize,
@ -386,13 +382,35 @@ NS_IMETHODIMP
nsSVGForeignObjectFrame::NotifyCanvasTMChanged(PRBool suppressInvalidation)
{
mCanvasTM = nsnull;
// If our width/height has a percentage value then we need to reflow if the
// width/height of our parent coordinate context changes. Actually we also
// need to reflow if our scale changes. Glyph metrics do not necessarily
// scale uniformly with the change in scale, so a change in scale can
// (perhaps unexpectedly) cause text to break at different points.
RequestReflow(nsIPresShell::eResize);
UpdateGraphic();
// XXX we should really have a separate notification for viewport changes and
// not overload NotifyCanvasTMChanged, e.g. we wouldn't need to check
// IsReflowLocked below. Note both notifications would be required for
// viewport changes when there's a viewBox though!
//
// If our width/height have a percentage value then we need to reflow if the
// width/height of our parent coordinate context changes. XXX Perhaps
// unexpectedly we also reflow if our CTM changes. This is because glyph
// metrics do not necessarily scale uniformly with change in scale and, as a
// result, CTM changes may require text to break at different points. roc
// says we shouldn't do this. See bug 381285 comment 20.
UpdateGraphic(); // update mRect before requesting reflow
// If we're called while the PresShell is handling reflow events then we
// must have been called as a result of the NotifyViewportChange() call in
// our nsSVGOuterSVGFrame's Reflow() method. We must not call RequestReflow
// at this point (i.e. during reflow) because it could confuse the PresShell
// and prevent it from reflowing us properly in future. Besides that,
// nsSVGOuterSVGFrame::DidReflow will take care of reflowing us
// synchronously, so there's no need.
PRBool reflowing;
PresContext()->PresShell()->IsReflowLocked(&reflowing);
if (!reflowing) {
RequestReflow(nsIPresShell::eResize); // XXX use mState & NS_FRAME_IN_REFLOW?
}
return NS_OK;
}
@ -524,23 +542,6 @@ void nsSVGForeignObjectFrame::RequestReflow(nsIPresShell::IntrinsicDirty aType)
if (!kid)
return;
/* commenting out to fix reftest failure - see bug 381285
// If we're called while the PresShell is handling reflow events we must do
// the reflow synchronously here and now. Calling FrameNeedsReflow could
// confuse the PresShell and prevent us from being reflowed correctly in
// future.
PRBool reflowing;
PresContext()->PresShell()->IsReflowLocked(&reflowing);
if (reflowing) {
NS_ASSERTION(aType == nsIPresShell::eResize, "Failed to invalidate stored intrinsic widths!");
// only refow here and now if we the PresShell isn't already planning to
if (!(kid->GetStateBits() & NS_FRAME_IS_DIRTY)) {
DoReflow();
}
return;
}
*/
PresContext()->PresShell()->FrameNeedsReflow(kid, aType, NS_FRAME_IS_DIRTY);
}
@ -582,6 +583,25 @@ void nsSVGForeignObjectFrame::UpdateGraphic()
mDirtyRegion.SetEmpty();
}
void
nsSVGForeignObjectFrame::MaybeReflowFromOuterSVGFrame()
{
// If we're already scheduled to reflow (i.e. our kid is dirty) we don't
// want to reflow now or else our presShell will do extra work trying to
// reflow us a second time. (It will also complain if it finds that a reflow
// root scheduled for reflow isn't dirty).
nsIFrame* kid = GetFirstChild(nsnull);
if (kid->GetStateBits() & NS_FRAME_IS_DIRTY) {
return;
}
kid->AddStateBits(NS_FRAME_IS_DIRTY); // we must be fully marked dirty
if (kid->GetStateBits() & NS_FRAME_HAS_DIRTY_CHILDREN) {
return;
}
DoReflow();
}
void
nsSVGForeignObjectFrame::DoReflow()
{

15
mozilla/layout/svg/base/src/nsSVGForeignObjectFrame.h
View File

@ -63,6 +63,9 @@ private:
public:
// nsIFrame:
NS_IMETHOD Init(nsIContent* aContent,
nsIFrame* aParent,
nsIFrame* aPrevInFlow);
virtual void Destroy();
NS_IMETHOD AttributeChanged(PRInt32 aNameSpaceID,
nsIAtom* aAttribute,
@ -74,13 +77,6 @@ public:
NS_IMETHOD DidSetStyleContext();
/**
* We need to reflow our decendants whenever style changes requiring reflow
* occur on an ancestor. Most SVG doesn't participate in reflow, but we can
* use MarkIntrinsicWidthsDirty to detect when this happens.
*/
virtual void MarkIntrinsicWidthsDirty();
NS_IMETHOD Reflow(nsPresContext* aPresContext,
nsHTMLReflowMetrics& aDesiredSize,
const nsHTMLReflowState& aReflowState,
@ -134,7 +130,10 @@ public:
nsPoint TransformPointFromOuter(nsPoint aPt);
already_AddRefed<nsIDOMSVGMatrix> GetCanvasTM();
// This method allows our nsSVGOuterSVGFrame to reflow us as necessary.
void MaybeReflowFromOuterSVGFrame();
protected:
// implementation helpers:
void DoReflow();

46
mozilla/layout/svg/base/src/nsSVGOuterSVGFrame.cpp
View File

@ -40,6 +40,7 @@
#include "nsIDOMSVGSVGElement.h"
#include "nsSVGSVGElement.h"
#include "nsSVGTextFrame.h"
#include "nsSVGForeignObjectFrame.h"
#include "nsSVGRect.h"
#include "nsDisplayList.h"
#include "nsStubMutationObserver.h"
@ -362,6 +363,14 @@ nsSVGOuterSVGFrame::Reflow(nsPresContext* aPresContext,
return NS_OK;
}
PR_STATIC_CALLBACK(PLDHashOperator)
ReflowForeignObject(nsVoidPtrHashKey *aEntry, void* aUserArg)
{
static_cast<nsSVGForeignObjectFrame*>
(const_cast<void*>(aEntry->GetKey()))->MaybeReflowFromOuterSVGFrame();
return PL_DHASH_NEXT;
}
NS_IMETHODIMP
nsSVGOuterSVGFrame::DidReflow(nsPresContext* aPresContext,
const nsHTMLReflowState* aReflowState,
@ -385,6 +394,14 @@ nsSVGOuterSVGFrame::DidReflow(nsPresContext* aPresContext,
}
UnsuspendRedraw(); // For the SuspendRedraw in InitSVG
} else {
// Now that all viewport establishing descendants have their correct size,
// tell our foreignObject descendants to reflow their children.
if (mForeignObjectHash.IsInitialized()) {
PRUint32 count = mForeignObjectHash.EnumerateEntries(ReflowForeignObject, nsnull);
NS_ASSERTION(count == mForeignObjectHash.Count(),
"We didn't reflow all our nsSVGForeignObjectFrames!");
}
}
return rv;
@ -723,6 +740,35 @@ nsSVGOuterSVGFrame::GetCanvasTM()
//----------------------------------------------------------------------
// Implementation helpers
void
nsSVGOuterSVGFrame::RegisterForeignObject(nsSVGForeignObjectFrame* aFrame)
{
NS_ASSERTION(aFrame, "Who on earth is calling us?!");
if (!mForeignObjectHash.IsInitialized()) {
if (!mForeignObjectHash.Init()) {
NS_ERROR("Failed to initialize foreignObject hash.");
return;
}
}
NS_ASSERTION(!mForeignObjectHash.GetEntry(aFrame),
"nsSVGForeignObjectFrame already registered!");
mForeignObjectHash.PutEntry(aFrame);
NS_ASSERTION(mForeignObjectHash.GetEntry(aFrame),
"Failed to register nsSVGForeignObjectFrame!");
}
void
nsSVGOuterSVGFrame::UnregisterForeignObject(nsSVGForeignObjectFrame* aFrame) {
NS_ASSERTION(aFrame, "Who on earth is calling us?!");
NS_ASSERTION(mForeignObjectHash.GetEntry(aFrame),
"nsSVGForeignObjectFrame not in registry!");
return mForeignObjectHash.RemoveEntry(aFrame);
}
PRBool
nsSVGOuterSVGFrame::EmbeddedByReference(nsIFrame **aEmbeddingFrame)
{

23
mozilla/layout/svg/base/src/nsSVGOuterSVGFrame.h
View File

@ -44,6 +44,8 @@
#include "nsIDOMSVGPoint.h"
#include "nsIDOMSVGNumber.h"
class nsSVGForeignObjectFrame;
////////////////////////////////////////////////////////////////////////
// nsSVGOuterSVGFrame class
@ -64,6 +66,14 @@ private:
NS_IMETHOD_(nsrefcnt) Release() { return 1; }
public:
#ifdef DEBUG
~nsSVGOuterSVGFrame() {
NS_ASSERTION(mForeignObjectHash.Count() == 0,
"foreignObject(s) still registered!");
}
#endif
// nsIFrame:
virtual nscoord GetMinWidth(nsIRenderingContext *aRenderingContext);
virtual nscoord GetPrefWidth(nsIRenderingContext *aRenderingContext);
@ -130,6 +140,14 @@ public:
// nsSVGContainerFrame methods:
virtual already_AddRefed<nsIDOMSVGMatrix> GetCanvasTM();
/* Methods to allow descendant nsSVGForeignObjectFrame frames to register and
* unregister themselves with their nearest nsSVGOuterSVGFrame ancestor so
* they can be reflowed. The methods return PR_TRUE on success or PR_FALSE on
* failure.
*/
void RegisterForeignObject(nsSVGForeignObjectFrame* aFrame);
void UnregisterForeignObject(nsSVGForeignObjectFrame* aFrame);
protected:
/* Returns true if our content is the document element and our document is
@ -138,6 +156,11 @@ protected:
*/
PRBool EmbeddedByReference(nsIFrame **aEmbeddingFrame = nsnull);
// A hash-set containing our nsSVGForeignObjectFrame descendants. Note we use
// a hash-set to avoid the O(N^2) behavior we'd get tearing down an SVG frame
// subtree if we were to use a list (see bug 381285 comment 20).
nsTHashtable<nsVoidPtrHashKey> mForeignObjectHash;
PRUint32 mRedrawSuspendCount;
nsCOMPtr<nsIDOMSVGMatrix> mCanvasTM;