Closed Bug 210647 Opened 21 years ago Closed 21 years ago

[FIX] Crash when trying to change charsets to "user defined"

Categories

(Core Graveyard :: GFX: Gtk, defect)

x86
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pratik.solanki, Assigned: MatsPalmgren_bugz)

References

()

Details

(Keywords: crash, platform-parity, regression, Whiteboard: [patch])

Attachments

(4 files, 1 obsolete file)

My steps to reproduce.

1. Create a fresh profile.
2. Add prefs for freetype font support.
3. Start Firebird
4. Change Serif and San-Serif font in Western and User Defined charsets to MS
Verdana in your preferences.
5. Now use View | Character Coding to change back and fro from Wester to User
defined. n the 3rd or 4th try, it crashes.

Talkback ID - TB21348365M

It also happens in Mozilla. I'll attach my copy of prefs.js
I should mention that this started happening with yesterday's (24 June) Firebird
nightly and with my local Mozilla build dated 2003062405
From what I hear, Firebird talkback doesn't seem to have symbols. So here's a
talkback for Mozilla TB 21377778H. Changing the product component appropriately.
Component: General → Layout
Product: Phoenix → Browser
Version: unspecified → Trunk
reassign
Assignee: blaker → other
Severity: major → critical
Keywords: crash, stackwanted
QA Contact: asa → ian
Whiteboard: TB21377778H
->Gfx:Gtk.

Incident ID 21377778
Stack Signature  	_Z19GetNormalLineHeightP14nsIFontMetrics() 81ce01f6
Email Address 	psolanki@netscape.net
Product ID 	MozillaTrunk
Build ID 	2003062605
Trigger Time 	2003-06-26 07:44:49
Platform 	LinuxIntel
Operating System 	Linux 2.4.18-timesys-4.0
Module 	libgklayout.so
URL visited 	
User Comments 	Add this to bug 210647
Trigger Reason 	SIGSEGV: Segmentation Fault: (signal 11)
Source File Name 	
Trigger Line No. 	
Stack Trace 	
_Z19GetNormalLineHeightP14nsIFontMetrics()
_Z17ComputeLineHeightP14nsIPresContextP19nsIRenderingContextP14nsStyleContext()
_ZN17nsHTMLReflowState14CalcLineHeightEP14nsIPresContextP19nsIRenderingContextP8nsIFrame()
_ZN18nsBlockReflowStateC1ERK17nsHTMLReflowStateP14nsIPresContextP12nsBlockFrameRK19nsHTMLReflowMetricsi()
_ZN12nsBlockFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj()
_ZN16nsContainerFrame11ReflowChildEP8nsIFrameP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateiijRj()
_ZN11CanvasFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj()
_ZN19nsBoxToBlockAdaptor6ReflowER16nsBoxLayoutStateP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRjiiiii()
_ZN19nsBoxToBlockAdaptor8DoLayoutER16nsBoxLayoutState()
_ZN5nsBox6LayoutER16nsBoxLayoutState()
_ZN16nsScrollBoxFrame8DoLayoutER16nsBoxLayoutState()
_ZN5nsBox6LayoutER16nsBoxLayoutState()
_ZN14nsContainerBox13LayoutChildAtER16nsBoxLayoutStateP6nsIBoxRK6nsRect()
_ZN21nsGfxScrollFrameInner9LayoutBoxER16nsBoxLayoutStateP6nsIBoxRK6nsRect()
_ZN21nsGfxScrollFrameInner6LayoutER16nsBoxLayoutState()
_ZN16nsGfxScrollFrame8DoLayoutER16nsBoxLayoutState()
_ZN5nsBox6LayoutER16nsBoxLayoutState()
_ZN10nsBoxFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj()
_ZN16nsGfxScrollFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj()
_ZN16nsContainerFrame11ReflowChildEP8nsIFrameP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateiijRj()
_ZN13ViewportFrame6ReflowEP14nsIPresContextR19nsHTMLReflowMetricsRK17nsHTMLReflowStateRj()
_ZN17IncrementalReflow8DispatchEP14nsIPresContextR19nsHTMLReflowMetricsRK6nsSizeR19nsIRenderingContext()
_ZN9PresShell21ProcessReflowCommandsEi()
_ZN11ReflowEvent11HandleEventEv()
_Z13HandlePLEventP11ReflowEvent()
PL_HandleEvent()
PL_ProcessPendingEvents()
_ZN16nsEventQueueImpl20ProcessPendingEventsEv()
_Z24event_processor_callbackPvi17GdkInputCondition()
_Z17our_gdk_io_invokeP11_GIOChannel12GIOConditionPv()
libglib-1.2.so.0 + 0xea7a (0x4026da7a)
libglib-1.2.so.0 + 0x10055 (0x4026f055)
libglib-1.2.so.0 + 0x10659 (0x4026f659)
libglib-1.2.so.0 + 0x107e8 (0x4026f7e8)
libgtk-1.2.so.0 + 0x91203 (0x4018c203)
_ZN10nsAppShell3RunEv()
_ZN17nsAppShellService3RunEv()
_Z5main1iPPcP11nsISupports()
main() 
Assignee: other → blizzard
Component: Layout → GFX: Gtk
Keywords: stackwanted
Whiteboard: TB21377778H
does 
  ...
  Stack Trace 	
  _Z19GetNormalLineHeightP14nsIFontMetrics()
  ...

mean this is the code where the crash happens:
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsHTMLReflowState.cpp#2276
2276 GetNormalLineHeight(nsIFontMetrics* aFontMetrics)
2277 {
2278   NS_PRECONDITION(nsnull != aFontMetrics, "no font metrics");
2279 
2280   nscoord normalLineHeight;
2281 
2282 #ifdef FONT_LEADING_APIS_V2
2283   nscoord externalLeading, internalLeading, emHeight;
2284   aFontMetrics->GetExternalLeading(externalLeading);
2285   aFontMetrics->GetInternalLeading(internalLeading);
2286   aFontMetrics->GetEmHeight(emHeight);
2287   switch (GetNormalLineHeightCalcControl()) {
2288   case eIncludeExternalLeading:
2289     normalLineHeight = emHeight+ internalLeading + externalLeading;
2290     break;
2291   case eCompensateLeading:
2292     if (!internalLeading && !externalLeading)
2293       normalLineHeight = NSToCoordRound(emHeight * NORMAL_LINE_HEIGHT_FACTOR);
2294     else
2295       normalLineHeight = emHeight+ internalLeading + externalLeading;
2296     break;
2297   default:
2298     //case eNoExternalLeading:
2299     normalLineHeight = emHeight + internalLeading;
2300   }
2301 #else
2302   aFontMetrics->GetNormalLineHeight(normalLineHeight);
2303 #endif // FONT_LEADING_APIS_V2
2304   return normalLineHeight;
2305 }

If so, then does this imply that aFontMetrics is null?
Attached file Stacktrace from gdb
Attached is the stacktrace I see under GDB. The problem I see is that
aRenderingContext.GetFontMetrics(mNormalFont) returned with mNormalFont as
NULL; thus crashing on the next line.
Looking up in my terminal I see the following

intl.charset.detector = off
intl.charset.detector = off
Charset Overlay menu item pressed: x-user-defined
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
/psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
/psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
/psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659
WARNING: empty damage rect: update caller to avoid fcn call overhead, file
/psolanki/mozilla/layout/html/base/src/nsFrame.cpp, line 2659
###!!! ASSERTION: font metrics should not be null - bug 136248:
'NS_SUCCEEDED(rv)', file /psolanki/mozilla/gfx/src/nsDeviceContext.cpp, line 706
Break: at file /psolanki/mozilla/gfx/src/nsDeviceContext.cpp, line 706

CCing rbs@maths.uq.edu.au who had fixed bug 136248.
> 2. Add prefs for freetype font support.
[...]
> Charset Overlay menu item pressed: x-user-defined

Troubles arise if the user-defined charset/font is not handled well, see e.g.,
bug 147222. Te root cause here seems precisely that the user-defined
charset/font is not handled well by the AA freetype support. My guess is that
fm->Init() is failing in line 668 and line 687 in the code, just before the
assertion above. 

Here is a handy way to try to see what is happening with your debugger. Add
the following assertion on line 667:
  aMetrics = nsnull;
  nsresult rv = CreateFontMetricsInstance(&fm);
  if (NS_FAILED(rv)) return rv;
  rv = fm->Init(aFont, aLangGroup, mContext);
+ NS_ASSERTION(NS_SUCCEEDED(rv), "init failed");

and then break in the debugger from that point, and step inside the _next_
Init() [where the fontmetrics is given a second chance to redeem itself]
  rv = fm->Init(aFont, aLangGroup, mContext);
You're right. It does assert. In fact both init()s are failing. I can't seem to
get Mozilla to run under debugger (although I can attach to it once it crashes)
so I can't step through it like you suggested. I have the same font for Western
and User Defined so I can't understand why it would crash. Does anyone else
besides me see the crash? Is this reproducible or does it have something to do
with my machine/font configuration?
*** Bug 213176 has been marked as a duplicate of this bug. ***
URL from dupe'd bug. Also here is a link to the testcase attached to that bug
which causes Mozilla to crash.

http://bugzilla.mozilla.org/attachment.cgi?id=128077&action=view

P.S. Bugzilla should have a way of having multiple bugs point to the same
attachment. Sort of like "Add existing attachment [id] to this bug"
*** Bug 216246 has been marked as a duplicate of this bug. ***
*** Bug 215478 has been marked as a duplicate of this bug. ***
this happens with and without freetype fonts.  it's the user defined charset
that causes the crash (see dupes, or just try changing to user defined without
freetype fonts)

this regressed between linux trunk 2003061105 and 2003061205, apparently bug
207438.  I'll try backing it out.
Keywords: regression
Summary: Crash when trying to change charsets (with freetype fonts) → Crash when trying to change charsets to "user defined"
backing out bug 207438.  the other checkin during that window to touch relevant
files was bug 206379.  In fact, that checkin modified the UserDefined section of
nsFontMetricsGTK.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/gfx/src/gtk&command=DIFF&root=/cvsroot&file=nsFontMetricsGTK.cpp&rev1=1.252&rev2=1.253#4

could that change be responsible for this behavior?
> backing out bug 207438.  the other checkin...

ugh.  "backing out bug 207438 didn't help."
*** Bug 216444 has been marked as a duplicate of this bug. ***
Attached patch Patch (obsolete) — Splinter Review
The error was introduced between -r1.252 and -r1.253 of nsFontMetricsGTK.cpp
as Andrew Schultz points out (with the bonsai link) in comment 16.

We should not return prematurely if |gUserDefinedConverter| has already been
initialized.  This bit of code that the patch removes was previously the "else"

part of the |GetCharsetAtom2| result test.
Keywords: pp
Summary: Crash when trying to change charsets to "user defined" → [FIX] Crash when trying to change charsets to "user defined"
Whiteboard: [patch]
Attachment #129935 - Flags: review?(alecf)
Since this is a crasher that was caused by a "typo", it should be risc-free
to include for 1.5b
Flags: blocking1.5b?
Flags: blocking1.5b? → blocking1.5b+
Comment on attachment 129935 [details] [diff] [review]
Patch

sr=dbaron if you convert the existing code to

if (NS_FAILED(res))
  return res;

rather than having the returns in the |else|, so that this doesn't happen
again.
Attached patch Patch rev. 2Splinter Review
Attachment #129935 - Attachment is obsolete: true
Note that we have always ignored the return value from
|SetOutputErrorBehavior|.
Maybe we should comment that this is intentional?  and even remove the
assignment to |res|?
Attachment #129935 - Flags: review?(alecf)
Attachment #129940 - Flags: review?(alecf)
Comment on attachment 129940 [details] [diff] [review]
Patch rev. 2

r=alecf
Generally this should go to i18n, though this particular fix is somewhat
obvious...
Attachment #129940 - Flags: review?(alecf) → review+
Comment on attachment 129941 [details] [diff] [review]
Patch rev. 2 (diff -uw for review only)

The patch misses the gfx/src/xlib part... ;-/
Attachment #129941 - Flags: review-
Reassign to smontagu per IRC discussion, he said he takes care about the Xlib
version of the patch...
Assignee: blizzard → smontagu
Back to Mats. I don't see that Xlib even *has* the bug :-)
Assignee: smontagu → mats.palmgren
So whats stopping this bug from being checked in? The patch has an r+ and an sr+
(Roland's r- doesn't seem to apply). 
Xlib and Windows does not have the bug.
Mac does not have corresponding code AFAICT.
Comment on attachment 129941 [details] [diff] [review]
Patch rev. 2 (diff -uw for review only)

approved for 1.5b.

/be
Attachment #129941 - Flags: approval1.5b? → approval1.5b+
need to get this checked in today if its going to make 1.5b...  trying to make
builds thursday or friday morning.  we could pick this up for final if it
doesn't make the beta train.
looks like this would be a good one to get.  showing as #5 talkback crash list
for linux on the 1.4 release, but seems to only affect a small number of people...


Blocks: 216980
Checked patch in (since Mats informed me that he doesn't have CVS access)
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 216980 has been marked as a duplicate of this bug. ***
Thanks Mats. I can confirm that the bug is fixed in the latest build - 2003082422.
*** Bug 226898 has been marked as a duplicate of this bug. ***
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: