474537 - Understand the speculative parsing crash

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect, P1)

Description

[Blake Kaplan (:mrbkap) (inactive)]

For bug 458440, I checked in a workaround to a crash that we were only seeing on tinderbox. We should really understand what's going on there before we release, though.

I'm filing this so it doesn't fall off the radar entirely (which I almost let happen).

Comment 1

[Johnny Stenback (:jst)]

If we were shipping tomorrow and this was the only bug left, I don't think I would hold the release only to figure this out, given how long we've had speculative parsing in the tree. Obviously we need to figure this out, and the sooner the better, but I won't hold the release for this.

Comment 2

[Boris Zbarsky [:bzbarsky]]

For what it's worth, the security implications here scare me...  We have all sorts of places that assume that a given namespace + tag really does mean that certain interfaces are implemented.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, I tried landing http://hg.mozilla.org/mozilla-central/rev/7c4cf0adb51f to see what sort of information I could get out of this stuff.  Unfortunately, the TinderboxPrint there doesn't get seen as such by tinderbox, so you have to download build logs to see what's going on.  I did find one such log, and here's the output:

NOISE: Tinderbox Print: FOUND BOGUS INPUT
NOISE: Passed in content pointer is: 05666400
NOISE: Passed in content has namespace: 0
NOISE: Passed in content has localName: input
NOISE: Passed in content QI to nsIDOMHTMLInputElement is: 00000000
NOISE: Passed in content QI to nsIClassInfo is: 073CBDF4
NOISE: Passed in classinfo description is: HTMLUnknownElement
NOISE: Classinfo knows about interface: {a6cf9085-15b3-11d2-932e-00805f8add32}
NOISE: Classinfo knows about interface: {7f142f9a-fba7-4949-93d6-cf08a974ac51}
NOISE: Classinfo knows about interface: {99715845-95fc-4a56-aa53-214b65c26e22}
NOISE: Classinfo knows about interface: {1c773b30-d1cf-11d2-bd95-00805f8ae3f4}
NOISE: Classinfo knows about interface: {29fb2a18-1dd2-11b2-8dd9-a6fd5d5ad12f}
NOISE: Classinfo knows about interface: {f0aef489-18c5-4de6-99d5-58b3758b098c}
NOISE: Classinfo knows about interface: {7cebc153-168a-416c-ba5a-56a8c2ddb2ec}

Those classinfo interfaces are nsIDOMHTMLElement, nsIDOMNSHTMLElement, nsIDOMElementCSSInlineStyle, nsIDOMEventTarget, nsIDOM3Node, nsIDOMNSElement, nsIDOMNodeSelector.

So that really is an nsHTMLUnknownElement, looks like.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Shades of bug 264956!

Comment 5

[Boris Zbarsky [:bzbarsky]]

I would guess that the only way to end up in this situation is for HTMLContentSink::CreateContentObject to get an aNodeType that doesn't match the text in the aNode or if the atomization of the node text screws up somehow, right?

I backed out the above logging patch and will write one for CreateContentObject.

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, I landed http://hg.mozilla.org/mozilla-central/rev/ebb854a35643 to do some content sink logging.  Let's see what happens.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Well, that logging code got hit.  Here's the skinny:

NOISE: BOGUS INPUT IN CreateContentObject
NOISE: aNodeType is userdefined: 1
NOISE: aNode.GetText().Length(): 5
NOISE: aNode.GetText: 'input'
NOISE: PRUnichars: 0x105 0x110 0x112 0x117 0x116 
NOISE: GOT INTO CreateHTMLElement WITH BOGUS INPUT
NOISE: BOGUS INPUT DETECTED IN FRAME CONSTRUCTOR
Failed tp:

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

I am so blind.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

I don't know how I missed this before -- I thought I read this entire file from start to finish. The location of the fix was obvious once I knew for sure that the token's internal tag was wrong.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 360210 [details] [diff] [review]
Fix

Nice catch!

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 360210 [details] [diff] [review]
Fix

Approved for 1.9.1, no doubt we want this there.

Comment 12

[Boris Zbarsky [:bzbarsky]]

r=me too, for what it's worth.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Diff for the 1.9.1 branch

The nsCSSFrameConstructor.cpp changes needed merging.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/d96c4efe014f and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/733307028fef