952338 - crash in nsIDocument::InnerWindowID()

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[alex_mayorga]

This bug was filed from the Socorro interface and is 
report bp-c0ba85b6-856e-4c89-a1b4-a95062131220.
=============================================================

Instant crash loading http://www.aubert.com/AUBERT-VELIZY.html as reported on http://forums.mozillazine.org/viewtopic.php?p=13261889#p13261889

Comment 1

[Gary [:streetwolf52]]

Crashes on Windows 8.1 x64.

Mozilla/5.0 (Windows NT 6.3; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

Comment 2

[mikedl]

Crash, as well, on Win7 x64.

Comment 3

[Alice0775 White]

STR
1. Open http://www.aubert.com/AUBERT-VELIZY.html
2. Click Reload button and repeat Step2 if necessary

bp-96a699d6-f0bb-4ead-aee2-29ae42131220
bp-e731d750-52e4-4758-8b97-627a32131220
bp-f6b1bee9-9c9a-4677-b334-3c41c2131220
bp-11488959-fd41-4809-a438-662e52131220
bp-db04da20-b7bb-4aaf-aad3-06b7a2131220
bp-db04da20-b7bb-4aaf-aad3-06b7a2131220
bp-3950b506-c955-447f-84ca-f3bfd2131220


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/1ad9af3a2ab8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131211190246
Bad:
http://hg.mozilla.org/mozilla-central/rev/d15ed5648a5f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131212035347
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1ad9af3a2ab8&tochange=d15ed5648a5f


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/8344fa6ecea6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131211180953
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/1390f263b8ce
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20131211181145
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8344fa6ecea6&tochange=1390f263b8ce

Regressed by: Bug 773296


FYI,
layout.css.variables.enabled = does not help...

Comment 4

[Cameron McCormack (:heycam)]

I'm surprised turning off the pref doesn't work, since we're crashing while resolving a property with a variable reference.

The mSheet that the nsCSSValueTokenStream has a raw pointer to has bad data in it.  I wonder if we should be keeping a strong reference to the sheet in the nsCSSValueTokenStream, in case it goes away.  (Although I'm not really sure when the sheet would go away while we still have the nsCSSValueTokenStream around to use.)

Comment 5

[Cameron McCormack (:heycam)]

Just making nsCSSValueTokenStream::mSheet an nsRefPtr avoids the crash but causes a leak.  The sheet references rules, and style rules have a Declaration, and the Declaration is where the nsCSSValue that has an nsCSSValueTokenStream in it is stored.  I guess the straightforward thing to do would be, inside nsCSSStyleSheet::UnlinkInner, to go through all the Declarations and clear out any references to the sheet that are still in the nsCSSValueTokenStreams.  But this seems to be a lot of work.  Do you have any better suggestions David?

Comment 6

[Benjamin Kerensa [:bkerensa]]

How frequently would our users run into this? Just trying to gauge how big of an impact this will have on our users.

Comment 7

[B.J. Herbison]

The error occurred for me (twice) when I maximized the window on the site.

bp-97f306bd-126a-4eee-be28-e9a392131220	12/20/2013	04:29 AM
bp-dc282313-240b-42d5-bb62-59c5a2131220	12/20/2013	04:24 AM

Comment 8

[Cameron McCormack (:heycam)]

Attachment: don't pass in sheet

Comment 9

[Cameron McCormack (:heycam)]

Until I get some time to look into this properly, let's not pass the sheet in to the CSS parser when we resolve a property's variable-containing value.  That will result in any error messages not going to the Web Console, but will avoid the crash.

Comment 10

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d2f3f483867

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d2f3f483867

Comment 12

[Cameron McCormack (:heycam)]

With the crash fix landed, I think we don't need to track for Firefox 29.  (The absence of a full solution just means that CSS parsing errors when resolving variables might not be shown in the error console.)

Comment 13

[Benjamin Kerensa [:bkerensa]]

Based on Comment 12 were not going to track this at this time.

Thanks,

Comment 14

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Can it be closed now?

Comment 15

[Cameron McCormack (:heycam)]

OK, let's close this as the crash is resolved, and work on the solution to restoring mSheet in a followup, bug 1170078.