263285 - crash changing minimum font size [@ js_GetGCThingFlags]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Andrew Schultz]

If I change the minimum font size in the preferences menu, Mozilla (usually)
crashes when I hit OK.  I'll attach a stack, which has js_GetGCThingFlags at the
top, similar to bug 237081 comment 3.

This (and the crash at people.aol.com) regressed between linux trunk builds
2004100405 and 2004100506.  The only thing I see in that window that could cause
this looks like bug 246441.

Comment 1

[Andrew Schultz]

Attachment: stacktrace

Comment 2

[Brendan Eich [:brendan]]

Stephen, did you see the people.aol.com crash regress in the same window, or was
it crashing (intermittently) before the E4X patch landed?

/be

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Brendan: layout of http://people.aol.com becomes horribly broken beginning with
the 10-01 builds, so it's hard to say.  However, I traced back to 9-24, and none
of those crashed, yet they laid out the page perfectly, so I'd assume this is a
very recent regression.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Okay.  Further testing shows that it doesn't crash with the 4th build, but does
with the 5th, however the 4th, for some reason, has a ton of missing content
upon layout (which makes me uncertain whether it would crash were it to display
the rest of the page properly).

Comment 5

[Andrew Schultz]

I backed out mozilla/js to before the bug 246441 checkins.  The crash on loading
http://people.aol.com/ was gone but the page layout was hosed.

Also the big chunk of missing content will show up in older builds if you induce
a reflow (change the minimum font size :) for example).  The layout isn't 100%
fixed (the search dialog is too low), but nothing really bad.

Comment 6

[Brendan Eich [:brendan]]

Taking, I fix, I fix!

/be

Comment 7

[Brendan Eich [:brendan]]

Attachment: short-term fix

I'm checking this in now and leaving this bug open for the non-#if-0-hacked
long-term fix, and in case there are still problems.  With this patch, I can
load http://people.aol.com/ using a trunk Firefox I just built.  Without this
patch, my build crashed straight away.

E4X imposes a big change on ECMAScript implementations: property ids, formerly
spec'd as strings and optimizable into ints, now may be objects, but when used
to name properties in XML objects.  The E4X patch let object ids flow through
to non-XML objects, which pretty quickly choked on such oddly-tagged
identifiers.  This patch #if 0's out the macro that gate-keeps object property
ids, #else'ing back the pre-E4X version of that macro.

/be

Comment 8

[Brendan Eich [:brendan]]

> E4X imposes a big change on ECMAScript implementations: property ids, formerly
> spec'd as strings and optimizable into ints, now may be objects....

The ECMA-357 spec doesn't actually say this, and if you use a string form of an
XML name such as *, a::b, @a, or @n::* to access a property, you'll get the same
result as you would had you used an object of the right type (QName, attribute
name, anyname) that converted to such a string.

For SpiderMonkey's E4X, my goal is to avoid converting from object to string and
back all the time, so I'm trying to create XML names at compile time.  This does
indeed require JSID_OBJECT, but strictly speaking, E4X does not require object
or other than string identifiers.

/be

Comment 9

[timeless]

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

Comment 10

[Brendan Eich [:brendan]]

Attachment: long-term fix, plus other e4x fixes

E4X is still in the newborn, lots of glop to clean off its pink 'lil body
phase. ;-)  This patch only *looks* big.  Detailed comments:

- Fix XML scanning to conform to http://www.w3.org/TR/xml-names11, so that
  XML tag and attribute names are QNames, PI targets never contain :, etc.
- Ripped out odious JSOP_CALLMETHOD and other over-literal transcription of
  ECMA_357 stuff to-do with method-calling.  It was too much code, it broke
  error messages (requiring yet more code, not written yet, in the value
  generator decompiler), and it created redundant stack frames.
- Renamed JSCLASS_W3C_XML_INFO_ITEM JSCLASS_DOCUMENT_OBSERVER -- long live
  simpler names, and look for Gecko DOM hook-up soon.
- Hid the JSID macros in jsprvtd.h, they never should have gone in jsapi.h,
  although JS_DEFAULT_XML_NAMESPACE_ID is still there, and even more magic.
  Added INT_JSVAL_TO_JSID and similar jsval<=>jsid helpers.
- Fixed bug 263285 properly by making all object-id code paths compile only
  #if JS_HAS_XML_SUPPORT.  This requires non-empty CHECK_ELEMENT_ID(obj, id)
  macro expansions when compiling E4X support in, and at runtime, extra id
  tag bit tests for all element accesses (property lookups by computed name).
  This change eliminated JSOP_BINDXMLPROP and js_BindXMLProperty.
- Added missing SAVE_SP calls before OBJ_LOOKUP_PROPERTY and JS_Enumerate.
  These may fix talkback mystery-crash bugs.  I'll propose them for aviary
  and 1.7 branches if appropriate.
- Reordered some statements in the interpreter to use more constants directly,
  and expanded the guts of ELEMENT_OP to include a CHECK_ELEMENT_ID call.
- Regularized and clarified similar logic in FoldXMLConstants.
- Fold XMLNames at compile time, avoiding object=>string=>object runtime.
- Fix GC safety bugs underneath the js_ParseNodeToXML function, which is now
  js_ParseNodeToXMLObject.
- A bunch of fixes to implement namespaces for XML.
- Fixed GetNamespace to construct the new namespace object.
- Optimized ToXMLName a bit.
- Implemented the missing index-name property-get case for XMLList.
- Axed XML_COPY_ON_WRITE and xml->flags -- unnecessary given xml->object.
- Added missing OOM failure checks for non-zero-lenght XMLARRAY_INIT calls.
- (js_)ParseNodeToXML moved up in jsxml.c.

/be

Comment 11

[Brendan Eich [:brendan]]

Comment on attachment 161923 [details] [diff] [review]
long-term fix, plus other e4x fixes

Worksforme with JS_HAS_XML_SUPPORT off (by default, IOW) on Firefox trunk. 
Checking the JS test suite, with E4X on and off.  Better start reading! ;-)

/be

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 161923 [details] [diff] [review]
long-term fix, plus other e4x fixes

Updated patch coming.

/be

Comment 13

[Brendan Eich [:brendan]]

I'll do the long-term fix over in bug 246441.

/be

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on http://people.aol.com.

While I can't verify everything (anything?) from comment 10, this crash is now
gone on Window XP, build 2004-11-12-04 using Seamonkey.