378982 - Parser doesn't handle Terminate() called multiple times well

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See discussion in bug 375051.  In particular, we end up calling DidBuildModel on the sink multiple times, which I think breaks the contract...

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Can't really determine if this is blocking or not based on current info. Blake, bz, please renominate and explain why if you think this should be a blocker.

Comment 2

[Boris Zbarsky [:bzbarsky]]

As long as all sink impls (including 3rd party ones) that we expect to be used with 1.9 deal, I guess it's not a blocker...

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Potential fix

I haven't even compiled this, but it seems like this might fix this bug. I'm not sure if the "warning" is correct, but it seems bogus to ask the parser to terminate two times.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 265331 [details] [diff] [review]
Potential fix

Please leave the warning only if there's a way to ask the parser whether it's already been terminated.  Otherwise callers have no way to avoid calling it twice.

If it _is_ possible to tell, we should make nsDocument do it (followup bug).

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: What I'm about to check in

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

I don't think it's worth adding a method to see if Terminate has been called. I do wonder if it's worth making IsComplete serve that purpose, though.