311295 - select.options[0] doesn't become undefined when last option is removed (was: Litmus script hangs)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Asa Dotzler [:asa]]

starting in the late build from Oct 4 (not present in the morning build Oct 4)
the script that loads when clicking the Firefox smoketest link
http://testrunner.mozilla.org/litmus/run_tests.cgi?product=1&testgroup=1
produces the Unresponsive Script warning.

Comment 1

[Jesse Ruderman]

Attachment: simple testcase

Comment 2

[Asa Dotzler [:asa]]

Possibly fallout from bug 311022 which fits the regression window. Blake, can
you look at this? 

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

This is almost certainly a regression from bug 311090/bug 310351. I'll know more
once I get this in a debugger, but I'll bet the property caching stuff that jst
warned me about when I first showed him that patch is biting us here.

My debug build is in the middle of a full-rebuild, but there may be a one-line
fix to nsGenericArraySH::NewResolve that fixes this (I bet the property we set
needs JSPROP_SHARED).

Comment 4

[Asa Dotzler [:asa]]

Do we think this will be a widespread problem?

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

I was right. This makes my build pass the testcase.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Despite my confusion on IRC, this fix still does fix this bug. The problem is
that bug 296230 hasn't landed on the branch, and is the cause of an outstanding
regression. That bug makes nsArraySH::GetProperty set *vp = JSVAL_VOID, which
fixes this on trunk.

Brendan had some misgivings about this approach on IRC, but I don't see a way
around it... Thoughts welcome.

Comment 7

[Brendan Eich [:brendan]]

(In reply to comment #6)
> Brendan had some misgivings about this approach on IRC, but I don't see a way
> around it... Thoughts welcome.

It's fine to use JSPROP_SHARED so long as the object did not hold the property
value by the slot, and only by that strong ref.  Otherwise, you'll recompute and
recreate the value on every get.  That's not an issue with XPCWrappedNatives,
but it could be with other sorts of objects.

/be

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 198659 [details] [diff] [review]
Fix

This is ok, we think, thanks to XPConnect wrapped native caching.

/be

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Checked in to MOZILLA_1_8_BRANCH, where this is needed. Trunk is OK as is.

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #9)
> Checked in to MOZILLA_1_8_BRANCH, where this is needed. Trunk is OK as is.

I think we want JSPROP_SHARED in both places -- do we have that on the trunk
already?  If we did have it on the trunk, we wouldn't need to set *vp =
JSVAL_VOID in any peterv patch....

/be

Comment 11

[Tracy Walker [:tracy]]

Testcase Pass on:
Windows 2005-10-06-05-mozilla1.8
Linux 2005-10-06-04-mozilla1.8

Comment 12

[Marcia Knous [:marcia]]

Looks good on Mac Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US;
rv:1.8b5) Gecko/20051006 Firefox/1.4.1. Test case passes.