311090 - Patch for bug 310351 was mildly bogus

Status: RESOLVED FIXED

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

Description

[Blake Kaplan (:mrbkap) (inactive)]

My method of getting the length was incorrect for _generic_ arrays, where we
can't make any assumptions about our data. Instead, we should go through
javascript (and then back through XPConnect, ick.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: As described

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 198474 [details] [diff] [review]
As described

This is ok, but you might change the JSVAL_IS_INT fretting to cope with
unsigned 32 bit values that do not fit in the nonnegative half of the signed 31
bit domain of int jsvals.

But that's for later, on the trunk.  For this patch, just change the XXX
comment to say how this can really happen (sparse array delegating to the DOM
prototype).

/be

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 198474 [details] [diff] [review]
As described

Prematurely requesting approval, this patch (re-)fixes maps.google.com

Comment 4

[Scott MacGregor]

I e-mailed jst directly this morning asking him to review this ASAP.

Comment 5

[Scott MacGregor]

Comment on attachment 198474 [details] [diff] [review]
As described

I'm going to approve this for the branch. jst, doesn't have internet
connectivity at the moment and we'd rather gets some bits up that we can all
start testing instead of waiting. 

Leaving the review request alone, so we can address any review comments from
jst later.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Checked in on trunk and MOZILLA_1_8_BRANCH.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 198474 [details] [diff] [review]
As described

r=jst

Comment 8

[Peter van der Woude [:Peter6]]

should Bug 311119 be fixed py this patch too ?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Hmm.. This had noticeable (0.5%?) Tp impact on btek.  Anything we can do about that?

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #9)
> Hmm.. This had noticeable (0.5%?) Tp impact on btek.  Anything we can do about
> that?

Sure -- at the cost of a QI test, restore the lost fast path that resulted in
this bug, and do the full get-property on 'length' if the QI to nsIDOMNodeList
fails.

/be

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

bug 311151 filed to track the Tp regression.

Comment 12

[Peter van der Woude [:Peter6]]

Blake, Bug 311204 has the same regressionwindow as all others here.
Could it have been caused by bug 310351 but not fixed by this bug ?