Closed Bug 4433 Opened 26 years ago Closed 25 years ago

[win32opt] parser calls extra CloseContainer() to XUL content sink

Categories

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

x86
Windows NT
defect

Tracking

()

VERIFIED WORKSFORME

People

(Reporter: waterson, Assigned: nisheeth_mozilla)

Details

(Keywords: verifyme)

apprunner is crashing on startup (with the default navigator.xul) because the on-load handler appears to be running before the window.frames slot if valid.
here is specifically what is happening. 1. the XUL stream finishes loading. 2. the nsWellFormedDTD sends the XUL content sink an one too many close tags. 3. the XUL content sink is surprised and returns NS_ERROR_UNEXPECTED. 4. the error result inhibits the nsWellFormedDTD from calling DidBuildModel. 5. because DidBuildModel is never called, layout is never performed, so no frames are constructed. 6. the on-load handler runs, passing a null pointer into the back-end, which crashes. The flakiest part of this whole chain is (2): it seems to be intermittent. I suspect that there may be either use of uninitialized memory, a bug with recycling tokens, or who knows what. I that we unblock the world by fixing the XUL content sink to return NS_OK instead of NS_ERROR_FAILURE. Then Nisheeth or Rick can tinker with the well-formed DTD and parser in the opt build at their leisure.
Assignee: sar → nisheeth
Summary: [win32opt] apprunner crash on startup → [win32opt] parser calls extra CloseContainer() to XUL content sink
I checked in a "fix" so that at least we wouldn't crash anymore. There are three things we need to fix (IMO): 1. We need to fix the bug in the parser that's sending extra close-tags to the content sink in the optimized build. That's just wrong. Nisheeth/Rick: you should be able to debug this by doing an opt/symbols build (MOZ_DEBUG=<null>, MOZ_PROFILE=1), and setting a breakpoint in XULContentSinkImpl::CloseContainer() (see mozilla/rdf/datasources/xul/nsXULContentSink.cpp). You'll see a comment that announces the bug. I've re-named the bug and reassigned it to nisheeth to track it. 2. We should fix XUL so that the "on load" handler _doesn't run_ if the document hasn't loaded. I'll assign a new bug against danm on this. 3. We should be more rigorous in _all_ code that accepts values across interfaces. Use NS_PRECONDITION to check for null pointers, esp. when being called from JS!!! There is a slew of code for gramps and the appcore guys to clean up. I'll assign a new bug against gramps on this.
Target Milestone: M4
Component: Apprunner → Parser
QA Contact: 3853 → 3847
Assignee: nisheeth → rickg
Assigning this to Rick Gessner for further analysis...
Target Milestone: M4 → M6
Assignee: rickg → nisheeth
Nisheeth: this bug has sat in my list so long it's likely to be stale. In particular, item 1 really isn't a parser issue per se, it's a XUL DTD issue. Please take a look into the status of this, and get me involved again if there's something fundamental going on.
Status: NEW → ASSIGNED
Accepting bug and setting milestone to M12...
Target Milestone: M10 → M12
Move non-PDT+ bugs to M13...
Moving non block/inline bugs to M14...
I did an opt build with debug symbols on and ran mozilla. The extra CloseContainer() call to the XUL content sink does not happen anymore. Marking this bug worksforme.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → WORKSFORME
Keywords: verifyme
Verified per Nisheeth's comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.