1133760 - Remove unforgeable holders from DOM proxies

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Right now, we have a setup for DOM proxies where unforgeable props are stored on a special "unforgeable holder" object that hangs off the proto.

I'm going to remove this and put the props on the object itself (so on the expando object).  This should be fine, because the only proxy we have with unforgeables is HTMLDocument, and it's not like we have tons of those around.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. The ownPropNames hook can't be called via Xrays, so stop trying to handle that case

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of unforgeable holders; just store unforgeable properties for DOM proxies directly on the expando object

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Diff of HTMLDocumentBinding.cpp

Apart from the removal of sharedRoot, the only changes to generated code are in HTMLDocumentBinding.cpp and ImageDocumentBinding.cpp.  This is a diff of the former; the latter is much the same.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of unforgeable holders; just store unforgeable properties for DOM proxies directly on the expando object

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated HTMLDocument diff

This has those ReleaseWrapper fixes we discussed on IRC.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Get rid of unforgeable holders; just store unforgeable properties for DOM proxies directly on the expando object

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff for part 2 from the previous version

This fixes the problem caught by the second part of the test: named getters can interfere with defineProperty on the proxy, so we really do want to define unforgeables directly on the expando object in the proxy case.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  There is in fact still an issue here, not even counting the bits in bug 928336 that might mean we want to hold on to the holder.

In particular, for the non-proxy case we really do want to define unforgeable stuff before we SetWrapper.  Otherwise the unforgeable bits trigger wrapper preservation...

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, and for proxies, simply creating the expando object triggers wrapper preservation.  And there's no other obvious place to put it where it won't be slow.

Maybe we just need to have different ordering for the SetWrapper() calls in the proxy and non-proxy cases. I think that should be ok.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 updated to the changes discussed in comment 8 and comment 9

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Interdiff for the comment 8/9 changes

Comment 12

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b8c313a2586
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f320d0be370

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1b8c313a2586
https://hg.mozilla.org/mozilla-central/rev/6f320d0be370