Closed
Bug 861022
Opened 11 years ago
Closed 11 years ago
Root WebIDL prototype and interface object setup
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(2 files)
18.56 KB,
patch
|
peterv
:
review+
terrence
:
review+
|
Details | Diff | Splinter Review |
18.24 KB,
patch
|
peterv
:
review+
terrence
:
review+
|
Details | Diff | Splinter Review |
I think I'll end up doing this in two steps: root the non-global bits, then root the global.
![]() |
Assignee | |
Updated•11 years ago
|
Blocks: ExactRootingBrowser
![]() |
Assignee | |
Comment 1•11 years ago
|
||
Attachment #736639 -
Flags: review?(terrence)
Attachment #736639 -
Flags: review?(peterv)
![]() |
Assignee | |
Updated•11 years ago
|
Assignee: nobody → bzbarsky
![]() |
Assignee | |
Comment 2•11 years ago
|
||
Attachment #736640 -
Flags: review?(terrence)
Attachment #736640 -
Flags: review?(peterv)
![]() |
Assignee | |
Updated•11 years ago
|
Whiteboard: [need review]
Comment 3•11 years ago
|
||
Comment on attachment 736639 [details] [diff] [review] part 1. Root the non-globals in WebIDL prototype and interface object setup. Review of attachment 736639 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bindings/BindingUtils.cpp @@ +554,1 @@ > } This may be venturing into unreasonable paranoia. ::: dom/bindings/DOMJSClass.h @@ +151,5 @@ > eInterfacePrototype > }; > > typedef JSObject* (*ParentGetter)(JSContext* aCx, JSObject* aObj); > +typedef JS::Handle<JSObject*> (*ProtoGetter)(JSContext* aCx, JSObject* aGlobal); It's not immediately obvious (or really apparent at all), why it is that the referenced proto will always outlive the returned handle. It would be good to have a comment somewhere explaining how this works and stating the invariants required for it to continue to working in the future.
Attachment #736639 -
Flags: review?(terrence) → review+
Updated•11 years ago
|
Attachment #736640 -
Flags: review?(terrence) → review+
Comment 4•11 years ago
|
||
(In reply to Terrence Cole [:terrence] from comment #3) > Comment on attachment 736639 [details] [diff] [review] > part 1. Root the non-globals in WebIDL prototype and interface object setup. > > Review of attachment 736639 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/bindings/BindingUtils.cpp > @@ +554,1 @@ > > } > > This may be venturing into unreasonable paranoia. Heh, I was expecting splinter to include 5 lines of context, as it normally does. To be clear, this was supposed to be: > else { > MOZ_ASSERT(!proto); > }
![]() |
Assignee | |
Comment 5•11 years ago
|
||
> It would be good to have a comment somewhere explaining how this works
I'll add the following comment above the typedef:
/**
* Returns a handle to the relevent WebIDL prototype object for the given global
* (which may be a handle to null on out of memory). Once allocated, the
* prototype object is guaranteed to exist as long as the global does, since the
* global traces its array of WebIDL prototypes and constructors..
*/
Is that clearer?
Flags: needinfo?(terrence)
Comment 6•11 years ago
|
||
Double-period at the end, otherwise it's perfect, thanks!
Flags: needinfo?(terrence)
Comment 7•11 years ago
|
||
Comment on attachment 736639 [details] [diff] [review] part 1. Root the non-globals in WebIDL prototype and interface object setup. Review of attachment 736639 [details] [diff] [review]: ----------------------------------------------------------------- Ditto on a comment explaining the deal with ProtoGetter returning a Handle.
Attachment #736639 -
Flags: review?(peterv) → review+
Updated•11 years ago
|
Attachment #736640 -
Flags: review?(peterv) → review+
![]() |
Assignee | |
Comment 8•11 years ago
|
||
OK, I'm reversing the order of this and bug 860841, because I need to get this landed....
![]() |
Assignee | |
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/60e671f364b9 https://hg.mozilla.org/integration/mozilla-inbound/rev/a606d921d45a
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla23
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/60e671f364b9 https://hg.mozilla.org/mozilla-central/rev/a606d921d45a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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
•