Closed
Bug 995163
Opened 11 years ago
Closed 11 years ago
keep the global with the value in Promise in NativePromiseCallback
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: baku, Assigned: baku)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #8405324 -
Flags: feedback?(bzbarsky)
Comment 2•11 years ago
|
||
Comment on attachment 8405324 [details] [diff] [review]
native.patch
We discussed this on IRC, but this code is assuming that AppendNativeHandler is called with JS on the stack, which seems like a pretty bold assumption...
Attachment #8405324 -
Flags: feedback?(bzbarsky) → feedback-
Assignee | ||
Comment 3•11 years ago
|
||
After the discussion by email, is this a reasonable approach to fix this issue?
Attachment #8405324 -
Attachment is obsolete: true
Attachment #8407447 -
Flags: review?(bzbarsky)
Comment 4•11 years ago
|
||
Comment on attachment 8407447 [details] [diff] [review]
wrapper1.patch
>+ NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mWrapper)
You don't need to add that member. Please don't.
In RunTask, you can just use GetOrCreateWrapper.
Also, don't you need to eneter the compartment of the wrapper and wrap the value into it or something? If performace is a concern, you can use MaybeWrapValue().
So what I would propose is that in RunTask you remove the "just for rooting" comment, call GetOrCreateWrapper, enter its compartment (or bail out if null is returned: that means OOM), wrap the value into the context compartment, and then pass cx to the callbacks along with the value.
Attachment #8407447 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8407447 -
Attachment is obsolete: true
Attachment #8414337 -
Flags: review?(bzbarsky)
Comment 6•11 years ago
|
||
Comment on attachment 8414337 [details] [diff] [review]
wrapper1.patch
r=me
Attachment #8414337 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Comment 8•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•