nsDOMScriptObjectFactory::GetScriptRuntimeByID should fill in the array for fast lookup

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Alfred Peng, Assigned: Alfred Peng)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
The language service should be filled into the array mLanguageArray for future fast lookup by ID.
(Assignee)

Comment 1

12 years ago
Created attachment 228772 [details] [diff] [review]
Patch v1
Attachment #228772 - Flags: review?(mhammond)

Comment 2

12 years ago
Comment on attachment 228772 [details] [diff] [review]
Patch v1

The 'if' block in the patch can go - we have already checked the array elt is NULL.  So that block can be reduced to simply:

mLanguageArray[NS_STID_INDEX(aLanguageID)] = lang;
(Assignee)

Comment 3

12 years ago
Created attachment 228914 [details] [diff] [review]
Patch v2

No ASSERTION needed here?
Assignee: general → alfred.peng
Attachment #228772 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #228914 - Flags: review?(mhammond)
Attachment #228772 - Flags: review?(mhammond)

Comment 4

12 years ago
Comment on attachment 228914 [details] [diff] [review]
Patch v2

That looks good to me.  No assertion is necessary, as the array element must be NULL.  Asking bz for sr, as I know he looked in this code at least once :)  Apologies in advance if that isn't appropriate.
Attachment #228914 - Flags: superreview?(bzbarsky)
Attachment #228914 - Flags: review?(mhammond)
Attachment #228914 - Flags: review+
Comment on attachment 228914 [details] [diff] [review]
Patch v2

Looks good!  sr=bzbarsky
Attachment #228914 - Flags: superreview?(bzbarsky) → superreview+

Comment 6

12 years ago
Checking in dom/src/base/nsDOMScriptObjectFactory.cpp;
/cvsroot/mozilla/dom/src/base/nsDOMScriptObjectFactory.cpp,v  <--  nsDOMScriptObjectFactory.cpp
new revision: 1.15; previous revision: 1.14
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.