Last Comment Bug 536692 - "ASSERTION: Placeholder relationship should have been torn down already"
: "ASSERTION: Placeholder relationship should have been torn down already"
Status: RESOLVED FIXED
: assertion, regression, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla10
Assigned To: :Ehsan Akhgari
:
Mentors:
Depends on: 10209
Blocks: stirtable 508473 ZDI-CAN-852
  Show dependency treegraph
 
Reported: 2009-12-24 10:26 PST by Jesse Ruderman
Modified: 2011-09-29 16:06 PDT (History)
7 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (202 bytes, application/xhtml+xml)
2009-12-24 10:26 PST, Jesse Ruderman
no flags Details

Description Jesse Ruderman 2009-12-24 10:26:55 PST
Created attachment 419132 [details]
testcase

###!!! ASSERTION: Placeholder relationship should have been torn down already; this might mean we have a stray placeholder in the tree.: '!placeholder || nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, placeholder)', file /Users/jruderman/central/layout/generic/nsFrame.cpp, line 443

This assertion was added in rev 1e37be566afa:
user:        fantasai
date:        Thu Dec 24 00:21:15 2009 -0500
summary:     Bug 508473 part III: Pass destruction root to frame destruction methods r=bz sr=roc
Comment 1 Jesse Ruderman 2009-12-24 10:27:33 PST
layout/base/crashtests/243519-1.html hits this assertion too.
Comment 2 Robert O'Callahan (:roc) (email my personal email if necessary) 2010-07-13 13:19:15 PDT
Need this fixed so we can backport frame destruction cleanup to branch.
Comment 3 fantasai 2010-08-11 23:08:39 PDT
So the problem here is that when we have a fixed-positioned table, it's not capturing any of its abspos out-of-flows. It keeps the placeholder, but the abspos jumps out to the CanvasFrame, which is not an ancestor of the fixedpos element. Thus frame destruction gets confused.

Here's the frame tree (columns and scrollbars cut out for simplicity):

    Viewport(-1)@0xb4b31628  pst=:-moz-viewport<
      HTMLScroll(html)(-1)@0xb4b24388 pst=:-moz-viewport-scroll<
        Canvas(html)(-1)@0xb4b24270 pst=:-moz-scrolled-canvas<
          Block(html)(-1)@0xb4b363c0<
            line 0xb4b366d8:<
              Block(body)(1)@0xb4b36678<
                line 0xb4b388c8:<
                  Placeholder(table)(1)@0xb4b36af8
                    outOfFlowFrame=TableOuter(table)(1)@0xb4b36920
                >
              >
            >
          >
        >
        Absolute-list<
          Block(tr)(0)@0xb4b383a8<
          >
        >
      >
    >
    Fixed-list<
      TableOuter(table)(1)@0xb4b36920 pst=:-moz-table-outer<
        Table(table)(1)@0xb4b36a48<
          TableRowGroup(table)(1)@0xb4b36e50 pst=:-moz-table-row-group<
            TableRow(table)(1)@0xb4b38010 pst=:-moz-table-row<
              TableCell(table)(1)@0xb4b381b0 pst=:-moz-table-cell<
                Block(table)(1)@0xb4b38348 pst=:-moz-cell-content<
                  line 0xb4b38540:<
                    Placeholder(tr)(0)@0xb4b38500
                      outOfFlowFrame=Block(tr)(0)@0xb4b383a8
                  >
                >
              >
            >
          >
        >
      >
    >

Everything *is* eventually getting destroyed correctly, because we eventually get to the placeholder for the table frame, and that gets us to the fixedpos table and the abspos frame's placeholder. We're not hitting it in the right order because the fixed list is outside the parent of the CanvasFrame, which is destroying its abspos list before its contents.

One correct fix for this bug is to make one of the table frames capture OOFs. In other words, fix bug 63895. Probably the easiest would be to have the anonymous block inside a table cell pretend it has position: relative whenever its closest element ancestor has position != static.

Another fix for this bug would be to destroy the main child lists before the abspos/fixed lists. But this is difficult to do because the child lists tend to get destroyed by a call to the superclass destructor, which eventually hits nsFrame::Destroy, which has code that must be executed last in the Destroy() order of operations. (In other words, we can't move the the regular child list destruction calls before the abspos/fixed list destruction calls because the child list destruction calls are bundled in the superclass destructor--which must be called last.) To flex the order we'd have to unbundle "destroy all your children" from "destroy yourself". (Both responsibilities are combined under the current Destroy() design.)
Comment 4 :Ehsan Akhgari 2011-05-02 12:16:05 PDT
My patches in bug 10209 are going to fix this bug, I believe.
Comment 5 :Ehsan Akhgari 2011-09-29 16:06:16 PDT
I pushed a crashtest for this in http://hg.mozilla.org/mozilla-central/rev/34f184d2a6f8.

Note You need to log in before you can comment on or make changes to this bug.