Closed Bug 1135961 Opened 6 years ago Closed 5 years ago

Implement subclassing of DOM objects

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(3 files, 7 obsolete files)

No description provided.
Depends on: 1117172
Depends on: 1135962
Depends on: 1135963
Depends on: new.target
Attached 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.
Attached 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
Attached patch With a fix for an Xray SNAFU (obsolete) — Splinter Review
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
Attached file Small manual testcase
Attachment #8615530 - Flags: review?(peterv)
Attachment #8600750 - Attachment is obsolete: true
Attachment #8638582 - Flags: review?(peterv)
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: 5 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.