Closed Bug 458453 Opened 11 years ago Closed 11 years ago

"ASSERTION: Cached frame is incorrect!" with filter in non-SVG content


(Core :: Layout, defect)

Not set





(Reporter: jruderman, Assigned: longsonr)


(Blocks 1 open bug)


(Keywords: assertion, testcase)


(2 files, 1 obsolete file)

Attached file testcase
###!!! ASSERTION: Cached frame is incorrect!: 'mElement.get() && static_cast<nsGenericElement*>(mElement.get())->GetPrimaryFrame() == mReferencedFrame', file layout/svg/base/src/nsSVGEffects.cpp, line 77
I think this is a false alarm.  The frame constructor DoDeletingFrameSubtree()
has notified the frame manager with RemoveAsPrimaryFrame() for the subtree
we're removing so that's why GetPrimaryFrame() can't find a frame.
Attached patch Like so? (obsolete) — Splinter Review
This is good enough?  Make nsFrameManager::GetShallowPrimaryFrameFor()
that does a lookup in its table and if it's not there just returns null,
and then tweak the assertion to accept that.

Now that I think about it, using the regular GetPrimaryFrame() is kind
of bad (only affects DEBUG builds though) since it could potentially
re-enter a dying frame into the frame manager tables again.

This patch contains some stuff from bug 458493 at the end, please
disregard that.
Attachment #341847 - Flags: superreview?(roc)
Attachment #341847 - Flags: review?(roc)
Attachment #341847 - Flags: superreview?(roc)
Attachment #341847 - Flags: superreview+
Attachment #341847 - Flags: review?(roc)
Attachment #341847 - Flags: review+
Comment on attachment 341847 [details] [diff] [review]
Like so?

Maybe call it LookupPrimaryFrameFor?
Attached patch Good to goSplinter Review
LookupPrimaryFrameFor it is.  Also included a crashtest.
Assignee: nobody → mats.palmgren
Attachment #341847 - Attachment is obsolete: true
Waiting for bug 455314 to see if this NS_ASSERTION remains after that bug.
Depends on: 455314
OS: Mac OS X → All
Hardware: PC → All
Blocks: 459666
Is this an alternative to bug 455314 comment 9?
I think it was fixed by bug 459883.  Applying that patch to an older build
that still asserts makes it go away.

Assignee: mats.palmgren → longsonr
Depends on: 459883
Closed: 11 years ago
Resolution: --- → FIXED
Pushed the crashtest:
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.