121055 - Trunk M099 Crash [@ nsRuleNode::WalkRuleTree][@ nsIBox::AddCSSPrefSize][@ nsFont::Equals][@ nsAString::do_AssignFromReadable]

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Blake Ross]

I managed to crash here in a 1/17 nightly, but unfortunately can't remember 
what I was doing. I think I may have been switching skins, but I'm not sure.

nsRuleNode::WalkRuleTree 
[d:\builds\seamonkey\mozilla\content\base\src\nsRuleNode.cpp, line 1499] 
nsRuleNode::GetPositionData 
[d:\builds\seamonkey\mozilla\content\base\src\nsRuleNode.cpp, line 1366] 
nsRuleNode::GetStyleData 
[d:\builds\seamonkey\mozilla\content\base\src\nsRuleNode.cpp, line 4728] 
nsStyleContext::GetStyleData 
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleContext.cpp, line 366] 
nsIBox::AddCSSPrefSize 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 1226] 
nsContainerBox::GetPrefSize 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 500] 
nsBoxFrame::GetPrefSize 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1026] 
nsMenuFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuFrame.cpp, line 965] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsSprocketLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp, line 
529] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsStackLayout::Layout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsStackLayout.cpp, line 331] 
nsContainerBox::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 612] 
nsBoxFrame::DoLayout 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1151] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line 
1052] 
nsBoxFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 953] 
nsRootBoxFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsRootBoxFrame.cpp, line 243] 
nsContainerFrame::ReflowChild 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 
771] 
ViewportFrame::Reflow 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp, line 
574] 
nsHTMLReflowCommand::Dispatch 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLReflowCommand.cpp, line 
217] 
PresShell::ProcessReflowCommand 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6188] 
PresShell::ProcessReflowCommands 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6243] 
ReflowEvent::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6099] 
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591] 
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, 
line 524] 
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, 
line 1072] 
USER32.dll + 0x3c076 (0x77d7c076) 
USER32.dll + 0x3c076 (0x77d7c076) 
netscp6.exe + 0x65a0 (0x004065a0) 
kernel32.dll + 0x3bb86 (0x77e9bb86)

Comment 1

[greer]

I crashed the M099 release twice this morning at this signature by switching 
back to Classic from the LCARStrek theme. I suppose this crash will be dealt 
with by the removal of dynamic switching.

Comment 2

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

Any ideas which windows you had open when switching themes?  Were you switching
via View | Apply Theme or through Edit | Preferences?

Comment 3

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

And were you also using Windows XP, or another platform where Classic has
native-theme capability?

Comment 4

[greer]

I was changing themes via edit | prefs on an NT4 box.

Comment 5

[greer]

  David, I can still crash the browser switching to LCARStrek and back to 
Classic, but the crash migrated to the nsFont::Equals signature (bug 121638) as 
long as I only switched between LCARStrek and Classic. 
  Just now I tried a crash switching first to LopburiFlat and back to Classic, 
then switching to LCARStrek and back to Classic which crashed at 
nsRuleNode::WalkRuleTree.

Comment 6

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

I just noticed that both this bug and bug 121638 are related to menu frames.  I
suspect they're the same bug, and I have a good guess at the fix (moving stuff
in FlushMiscWidgetInfo into DidSetStyleContext, as for bug 116038).

Comment 7

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

...except that approach doesn't work for menus, since it messes with their
interactivity.  So maybe I should try and figure out why FlushMiscWidgetInfo
isn't doing its job...

Comment 8

[greer]

Attachment: Stack and comments from crashes at nsIBox::AddCSSPrefSize

Some M099 and Trunk users are crashing this one earlier in the stack at
nsIBox::AddCSSPrefSize. Both signatures are current topcrashers.

Comment 9

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

I think bug 121638 and bug 130696 may be duplicates of this bug.

Comment 10

[Jay Patel [:jay]]

*** Bug 121963 has been marked as a duplicate of this bug. ***

Comment 11

[Jay Patel [:jay]]

Moving over keywords from duped 121963...I hope that's ok.

Comment 12

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

*** Bug 121638 has been marked as a duplicate of this bug. ***

Comment 13

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

Attachment: the reason FlushMiscWidgetInfo didn't seem to be working

This patch should, at least in theory, fix both bug 116038 (although the fix to
that bug probably fixed some dynamic style-reresolution issues with outliner
too) and this bug.  I'm seeing a crash with it that I don't understand, but the
problem could be that my tree is in too weird a state right now.  I'm going to
try it on a cleaner tree in a bit.

Comment 14

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

Attachment: possible patch?

This fixes the problem I was seeing, but along with that fix no longer fixes
bug 116038 without my patch there (see the "NOTE" in the patch for why). 
However, it works fine against an existing tree.

I need to try to do two things:
 * Figure out if it's theoretically possible for something that's crashing now
due to this problem not to be covered by the "NOTE" that describes what could
still crash.
 * If it is possible, figure out if the existing frames where we flush style
contexts (menu frames) actually behave in the way so that this will fix the
problems (i.e., they lazily create their extra style contexts and don't create
any extra style contexts before the ReResolve).

Comment 15

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

I think the answer to my first question above is that it's not possible, so I
think the best approach is to go to a two-pass FlushMiscWidgetInfo.

Comment 16

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

Another possibility is doing FlushMiscWidgetInfo after the ReResolve.  To know
what's right I really need to know why we're doing the various things that
FlushMiscWidgetInfo does.  I'm sure hyatt could tell me that...

Comment 17

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

*** Bug 130696 has been marked as a duplicate of this bug. ***

Comment 18

[David Hyatt]

The basic problem with menus is that they do not have style resolved on 
them during ReResolveStyleContext.  The popup frames are deliberately 
kept hidden from ReResolve.  I did this a long time ago because if they 
weren't kept hidden, new window time took a hit.

This all predated improvements to the style system, so it might not be so 
terrible to include menu popups in ReResolveStyleContext at this point.

FlushMiscWidgetInfo was a hacky workaround to ensure that the popup 
frames get properly re-resolved without using ReResolveStyleContext to 
do it.

As far as timing goes, the ideal solution would have menu popups being 
re-resolved at the same time as other frames (as you drill down through 
the style tree).

Comment 19

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

So the menu stuff would be better if done after the rest of the ReResolve.  What
about the tree and the XBL binding stuff?

Comment 20

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

Attachment: possible patch, assuming the tree/XBL framechange stuff is OK as-is

Comment 21

[greer]

Moving nsFont::Equals signature forward from 121638 which dbaron duped to this 
bug.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 74883 [details] [diff] [review]
possible patch, assuming the tree/XBL framechange stuff is OK as-is

looks reasonable.  r=bzbarsky, bug get hyatt to sr, please.

Comment 23

[David Hyatt]

sr=hyatt

Comment 24

[Scott Collins]

Comment on attachment 74883 [details] [diff] [review]
possible patch, assuming the tree/XBL framechange stuff is OK as-is

a=scc, though annoyed that hyatt didn't actually check the 'has-super-review'
box.  Doing that for him on the basis of comment #23

Comment 25

[Jay Patel [:jay]]

Adding [@ nsAString::do_AssignFromReadable] to summary from duped bug 130696.  

There was a new bug logged today with the same stack signature and stack (bug
132647).  I wonder if that's a similar problem or even a dup.  dbaron, you might
want to take a quick look.

Comment 26

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

Fix checked in 2002-03-21 15:44 PST.

Comment 27

[Jay Patel [:jay]]

*** Bug 133421 has been marked as a duplicate of this bug. ***

Comment 28

[Jay Patel [:jay]]

I haven't seen any of these crashes in Talkback data after the fix was checked
in.  Now that dynamic theme switching is disabled...hopefully we won't ever see
these crashes again.  Marking verified.

Comment 29

[Jay Patel [:jay]]

*** Bug 125518 has been marked as a duplicate of this bug. ***

Comment 30

[Jay Patel [:jay]]

Adding [@ nsRuleNode::ComputePositionData] from duped bug 125518 for future
reference.

Comment 31

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

I un-duped bug 125518 since it's not related.