Implement subclassing of DOM objects

RESOLVED FIXED in Firefox 42

Status

()

defect
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
mozilla42
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(3 attachments, 7 obsolete attachments)

No description provided.
Depends on: 1117172
Depends on: 1135962
Depends on: 1135963
Depends on: new.target
Posted patch WIP (obsolete) — Splinter Review
This should all work, I think, except for preserving the wrapper when using a non-default proto.  In particular, I need to sort out the proxy story there, since apparently preserving the wrapper for proxies is ... complicated.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Note to self: Use new XMLSerializer() as a fast constructor to test perf impact of this stuff.
Posted patch WIP2 (obsolete) — Splinter Review
OK, so the preserving works now... .  New problem is that my asserts trigger in a debug buid, because the Promise ctor actually does the wrapping in Promise::Constructor, so the bindings don't get a chance to use the right proto.  Need to think a bit about how to handle that.
Attachment #8600555 - Attachment is obsolete: true
Attachment #8600674 - Attachment is obsolete: true
So I think the main options are to either hack codegen for Promise to pass in the desired proto for now, pending Promise moving into SpiderMonkey, or to change all DOM constructors to pass in the desired proto, in case the callee wraps eagerly.

I'm pretty happy doing the former, honestly.  Promise is a pretty special snowflake.  Peter, do you have a preference?
Flags: needinfo?(peterv)
Attachment #8600675 - Attachment is obsolete: true
Flags: needinfo?(peterv)
Blocks: 1170760
Attachment #8615530 - Flags: review?(peterv)
Attachment #8600750 - Attachment is obsolete: true
Attachment #8615530 - Attachment is obsolete: true
Attachment #8615530 - Flags: review?(peterv)
Attachment #8638582 - Attachment is obsolete: true
Attachment #8638582 - Flags: review?(peterv)
Attachment #8638873 - Flags: review?(peterv)
Attachment #8638873 - Attachment is obsolete: true
Attachment #8638873 - Flags: review?(peterv)
Comment on attachment 8639901 [details] [diff] [review]
And even compiling, with the GetWrapper changes

Review of attachment 8639901 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the comments, they're really helpful.

::: dom/bindings/Codegen.py
@@ +3446,5 @@
> +        if self.descriptor.proxy:
> +            preserveWrapper = dedent(
> +                """
> +                // For DOM proxies, the only reliable way to preserve the wrapper
> +                // is to force creation of the expando object.

:-(
Attachment #8639901 - Flags: review?(peterv) → review+
> :-(

I filed bug 1189822.
https://hg.mozilla.org/mozilla-central/rev/ccf304bee50c
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.