Closed Bug 927734 Opened 7 years ago Closed 7 years ago

Assertion failure: i == -1 @ TreeMatchContext::AssertHasAllStyleScopes

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox27 - affected

People

(Reporter: bc, Assigned: heycam)

References

()

Details

(Keywords: assertion, regression, testcase, Whiteboard: [qa-])

Attachments

(4 files, 2 obsolete files)

Attached file stack
1. http://boards.4chan.org/v/
2. Assertion failure: i == -1, at /work/mozilla/builds/nightly/mozilla/layout/style/nsRuleProcessorData.h:192

Nightly Linux, OSX, Windows

Found regression between 20131010190115-20131011185030
Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=672cd63528d3&tochange=73f37c7a3860
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/10/2013-10-11-mozilla-central-debug/firefox-27.0a1.en-US.debug-linux-i686.tar.bz2
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013/10/2013-10-12-mozilla-central-debug/firefox-27.0a1.en-US.debug-linux-i686.tar.bz2

I see this also in the plugin-container process though it doesn't take the browser down. The assertion in the main process will take the full browser down.
This is a bit tricky. I left in the quitter call and the DDBEGIN/DDEND comments. ymmv.
The tinderbox builds from
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-linux64-debug/
shows the regression occurred between 1381440224 and 1381476834:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=93e7fdddd724&tochange=266532efd5cb
In that range I suspect bug 899808 given that the testcase has a scoped style element.
Blocks: 899808
Priority: -- → P2
:heycam given this is a fallout from 899808, can you help understand how this failure may impact an end-user to help with justification if this is a serious issue to track ?
Flags: needinfo?(cam)
I think the styles are still being applied correctly here, so to the end user there shouldn't be any impact.  I'll investigate this and bug 930270 today.
Flags: needinfo?(cam)
Attached file test reduced further
Assignee: nobody → cam
Status: NEW → ASSIGNED
The issue is that we call PushScope on the TreeMatchContext with the same element twice, and the recently added AssertHasAllStyleScopes doesn't expect this.  It looks like this is just how the ElementRestyler works in cases where we have multiple frames that have the same mContent, like with this test case.  The frame tree looks like this:

  nsTableCellFrame          -- content = the <td>
    anonymous nsBlockFrame  -- content = the same <td>
      nsInlineframe         -- content = the <span>

We push the <td> twice, which should be harmless.  I'll adjust AssertHasAllStyleScopes.
Attached patch patch (obsolete) — Splinter Review
Attachment #823719 - Flags: review?(dbaron)
Attached patch patch v2 (obsolete) — Splinter Review
With the test as a crashtest.
Attachment #823719 - Attachment is obsolete: true
Attachment #823719 - Flags: review?(dbaron)
Attachment #823724 - Flags: review?(dbaron)
Comment on attachment 823724 [details] [diff] [review]
patch v2

r=dbaron
Attachment #823724 - Flags: review?(dbaron) → review+
Given comment #4 not tracking. Feel free to request auorora approval once resolved to get this uplifted if low risk
I'm changing my mind about what AssertHasAllStyleScopes should be checking.  I had thought that I could make it check mStyleScopes more strictly than AssertHasAllAncestors does the ancestor filter, but since we can have repeated elements like in this bug, and in the presence of XBL, other elements that are not actually in the path from the element to the root:

  https://hg.mozilla.org/mozilla-central/annotate/tip/layout/base/nsCSSFrameConstructor.cpp#l3548

it's probably best just to make it assert that all of the direct ancestors that are style scope roots are in mStyleScopes, and leave it at that.
Attached patch patch v3Splinter Review
Attachment #823724 - Attachment is obsolete: true
Attachment #824974 - Flags: review?(dbaron)
Duplicate of this bug: 932585
Comment on attachment 824974 [details] [diff] [review]
patch v3

r=dbaron
Attachment #824974 - Flags: review?(dbaron) → review+
https://hg.mozilla.org/mozilla-central/rev/54010dec1aa0
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Flags: in-testsuite+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.