Closed
Bug 1034191
Opened 10 years ago
Closed 10 years ago
Make nsDependentJSString users work with Latin1 strings and nursery-allocated strings
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla33
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(1 file)
57.09 KB,
patch
|
bzbarsky
:
review+
bholley
:
feedback+
|
Details | Diff | Splinter Review |
This patch renames nsDependentJSString to nsAutoJSString, and makes it inherit from nsAutoString instead of nsDependentString. I also moved the optimized AssignJSString function from BindingUtils.h to nsJSUtils.h so that we can use it. Almost all places where we use nsDependentJSString seem to be slow paths. Sorry for the pretty big patch, but hopefully this is the last string-related one. After this there are a number of places where we use the JS string API functions directly, but hopefully some of them can also use nsAutoJSString.
Attachment #8450394 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 1•10 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #0) > Sorry for the pretty big patch, but hopefully this is the last > string-related one. Er, the last *big* string-related one.
Comment 2•10 years ago
|
||
Comment on attachment 8450394 [details] [diff] [review] Patch r=me on most of this. The only question I have is about infallibleInit going away in the jsid case. Can we no longer depend on stable jschar* for atoms? Bobby, does anything really depend on infallibleInit existing?
Attachment #8450394 -
Flags: review?(bzbarsky)
Attachment #8450394 -
Flags: review+
Attachment #8450394 -
Flags: feedback?(bobbyholley)
Updated•10 years ago
|
Flags: needinfo?(bobbyholley)
Comment 6•10 years ago
|
||
Comment on attachment 8450394 [details] [diff] [review] Patch Review of attachment 8450394 [details] [diff] [review]: ----------------------------------------------------------------- As this patch demonstrates, I don't think there's any place where we actually depend on infallibleInit - it's always just a convenience thing. The one place I know of where we access string characters without a JSContext is in IsPermitted (AccessCheck.cpp), when we invoke JS_GetInternedStringCharsAndLength. That could probable be rejiggered pretty easily though.
Attachment #8450394 -
Flags: feedback?(bobbyholley) → feedback+
Updated•10 years ago
|
Flags: needinfo?(bobbyholley)
Assignee | ||
Comment 7•10 years ago
|
||
While converting the rest of the browser, I found one or two places where I'd like to use nsAutoJSString with a jsid and where we don't have a JSContext readily available. For instance, _utf8fromidentifier in dom/plugins/base/nsNPAPIPlugin.cpp. Similar functions in that file use AutoSafeJSContext, so that will probably work, but I may add infallibleInit(jsid) back just to be safe.
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/102fae9cacc1
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/102fae9cacc1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•