110174 - Crash when try to visit a unicode test page

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Yuying Long]

Build: 11-14 win32 trunk on Win98-EN.

When I tried to verify bug 97343, I got crashed when I tried to load the test
page that Shanjian attached there:
http://bugzilla.mozilla.org/attachment.cgi?id=54921&action=view

Steps to reproduce:
1. Launch browser.
2. Edit | Preferences | Fonts.
3. For the fonts for western, set Serif to MS Serif and Sans-Serif to MS Sans
Serif, click on OK.
4. Load the test page:
   http://bugzilla.mozilla.org/attachment.cgi?id=54921&action=view

Result:
Crash

Note: No crash on WinME-Ja though.

Comment 1

[Roy Yokoyama]

Can you try with 11-13 build? 

Comment 2

[Yuying Long]

Crashed on 11-13 also.

Since Shanjian pointed out the fix for bug 97343 was checked in on 10-26, so I
installed 10-25-09, 10-26, 10-27 trunk builds, they are all crashed.  So it
might not cause by that checked-in if Shanjian was checked in on 10-26.

Talk back ID: 38025373, 38024649
-------------------------------------------------
Incident ID 38025373 
Stack Signature nsRenderingContextWin::GetTextDimensions 44731c27  
Bug ID  
Trigger Time  2001-11-14 17:18:15  
Email Address  [e-mail]  
URL visited   
User Comments  crash on another win98 machine  
Build ID 2001111410  
Product ID MozillaTrunk  
Platform ID Win32  
Trigger Reason Access violation  
Stack Trace  
nsRenderingContextWin::GetTextDimensions
[d:\builds\seamonkey\mozilla\gfx\src\windows\nsRenderingContextWin.cpp, line 2065] 
nsTextFrame::MeasureText
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsTextFrame.cpp, line 4699] 
nsTextFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsTextFrame.cpp, line 5155] 
nsLineLayout::ReflowFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsLineLayout.cpp, line 1038] 
nsInlineFrame::ReflowInlineFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsInlineFrame.cpp, line 713] 
nsInlineFrame::ReflowFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsInlineFrame.cpp, line 522] 
nsInlineFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsInlineFrame.cpp, line 438] 
nsLineLayout::ReflowFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsLineLayout.cpp, line 1038] 
nsBlockFrame::ReflowInlineFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3675] 
nsBlockFrame::DoReflowInlineFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3556] 
nsBlockFrame::DoReflowInlineFramesAuto
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3481] 
nsBlockFrame::ReflowInlineFrames
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3426] 
nsBlockFrame::ReflowLine
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2493] 
nsBlockFrame::ReflowDirtyLines
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2150] 
nsBlockFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 826] 
nsBlockReflowContext::DoReflowBlock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockReflowContext.cpp, line
581] 
nsBlockReflowContext::ReflowBlock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockReflowContext.cpp, line
359] 
nsBlockFrame::ReflowBlockFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3169] 
nsBlockFrame::ReflowLine
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2372] 
nsBlockFrame::ReflowDirtyLines
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2150] 
nsBlockFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 826] 
nsBlockReflowContext::DoReflowBlock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockReflowContext.cpp, line
581] 
nsBlockReflowContext::ReflowBlock
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockReflowContext.cpp, line
359] 
nsBlockFrame::ReflowBlockFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 3169] 
nsBlockFrame::ReflowLine
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2372] 
nsBlockFrame::ReflowDirtyLines
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 2150] 
nsBlockFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsBlockFrame.cpp, line 826] 
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 737] 
CanvasFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLFrame.cpp, line 570] 
nsBoxToBlockAdaptor::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxToBlockAdaptor.cpp, line 891] 
nsBoxToBlockAdaptor::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxToBlockAdaptor.cpp, line 540] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line
1002] 
nsScrollBoxFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsScrollBoxFrame.cpp, line 392] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line
1002] 
nsContainerBox::LayoutChildAt
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp, line 653] 
nsGfxScrollFrameInner::LayoutBox
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 1029] 
nsGfxScrollFrameInner::Layout
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 1140] 
nsGfxScrollFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 1037] 
nsBox::Layout [d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp, line
1002] 
nsBoxFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 928] 
nsGfxScrollFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp, line 755] 
nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 737] 
ViewportFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp, line 576] 
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 6017] 
PresShell::ProcessReflowCommands
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 6072] 
ReflowEvent::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5928] 
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] 
KERNEL32.DLL + 0x24407 (0xbff94407) 
0x00688bfe 

Comment 3

[Roy Yokoyama]

ylong: I hate to ask this; but can you identify which build started to crash?

Comment 4

[Yuying Long]

It crashes on 10-15-06 trunk build - this is the earliest trunk build in
sweetlou, unfortunatily, I don't have win98, I have to use win98 in lab, but
there is no older build there.

I'll try to find another win98 see if I'm lucky enough to find an old build
which doesn't crash. 

Comment 5

[rbs]

Are you working on this, Roy? Feel free to re-assign to me. I know where the 
problem comes from (a test needs to be relaxed to include bitmap fonts in the 
handling of the dreaded substitute fonts).

Comment 6

[Roy Yokoyama]

-> [e-mail] 
Thanks

Comment 7

[rbs]

Attachment: patch to fix the problem and save font resources

The problem here is that FindSubstitute() only consider TrueType fonts. With
the change of the prefs, the fonts involved became bitmap fonts (<font face="MS
Sans Serif"> for the testcase, and the generic fonts from the prefs are all
bitmap fonts), so FindSubstitute() returned nsnull (and so, it broke the
implicit assumption that the substitute font is always there to fallback to).

To minimize font resources, I have also changed LoadGenericFont() so as to only
load generic fonts that help to represent a char. Now, only the local fonts in
the font-family list are loaded without conditions. Any other font is kept
around only if it is indeed useful in representing a char.

 nsFontWin*
 nsFontMetricsWin::LoadGenericFont(HDC aDC, PRUnichar aChar, nsString* aName)
 {
  [...]
   nsFontWin* font = LoadFont(aDC, aName);
-  if (font && font->HasGlyph(aChar)) {
-    return font;
+  if (font) {
+    if (font->HasGlyph(aChar))
+      return font;
+    mLoadedFonts.RemoveElement(font);
+    delete font;
   }
   return nsnull;
 }

Comment 8

[rbs]

Summary of the changes:
- enable bitmap fonts as substitute fonts as well;
- don't keep generic fonts in mLoadedFonts[] if they don't represent any char;
- since mLoadedFonts[] doesn't include generic fonts anymore, make sure to
try the global list of fonts as well when hunting for a substitute font that has
the question mark. (A code like that used to be there before, but was mostly a 
no-op because generic fonts were in mLoadedFonts[] so that the first loop always 
suceeded. It was removed in:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&fi
le=nsFontMetricsWin.cpp&root=/cvsroot&subdir=mozilla/gfx/src/windows&command=DIF
F_FRAMESET&rev1=3.94&rev2=3.95)

Comment 9

[rbs]

Attachment: same patch with an extension for the 'A' case too

Comment 10

[rbs]

Attachment: final patch - ready for r/sr

It is the same patch but I have removed the bits that I added to unload
unecessary generic fonts. I have filed the separate bug 110361 for that.

Comment 11

[rbs]

Seeking r=shanjian, sr=attinasi for:
http://bugzilla.mozilla.org/attachment.cgi?id=58017&action=view

Comment 12

[Shanjian Li]

Comment on attachment 58017 [details] [diff] [review]
final patch - ready for r/sr

r=shanjian

Comment 13

[Marc Attinasi]

Comment on attachment 58017 [details] [diff] [review]
final patch - ready for r/sr

sr=attinasi

Comment 14

[rbs]

Fixed.

Comment 15

[Yuying Long]

Fixed verified on 11-19-10 trunk build on win98.