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

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

Attachment: 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.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: test-assertion-i-eq--1.html

This is a bit tricky. I left in the quitter call and the DDBEGIN/DDEND comments. ymmv.

Comment 2

[Mats Palmgren (inactive)]

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.

Comment 3

[bhavana bajaj [:bajaj]]

: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 ?

Comment 4

[Cameron McCormack (:heycam)]

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.

Comment 5

[Cameron McCormack (:heycam)]

Attachment: test reduced further

Comment 6

[Cameron McCormack (:heycam)]

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.

Comment 7

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 8

[Cameron McCormack (:heycam)]

Attachment: patch v2

With the test as a crashtest.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 823724 [details] [diff] [review]
patch v2

r=dbaron

Comment 10

[bhavana bajaj [:bajaj]]

Given comment #4 not tracking. Feel free to request auorora approval once resolved to get this uplifted if low risk

Comment 11

[Cameron McCormack (:heycam)]

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.

Comment 12

[Cameron McCormack (:heycam)]

Attachment: patch v3

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 824974 [details] [diff] [review]
patch v3

r=dbaron

Comment 15

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/54010dec1aa0

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/54010dec1aa0