Closed
Bug 752446
Opened 13 years ago
Closed 6 years ago
Need a story for wrapper reparenting for slim wrappers and new DOM objects
Categories
(Core :: XPConnect, defect)
Core
XPConnect
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: bholley, Unassigned)
Details
Peterv and I were talking about bug 751995 and we realized that this is an issue.
Currently, when we do document.open, we call ReparentContentWrappersInScope to move all the wrappers to the new inner window. But this only iterates over XPCWNs. If we find a slim wrapper on the parent chain we morph it. But for leaf slim wrappers and new binding objects, we don't reparent. And we don't have a way to, modulo iterating over every JS object in the compartment (which might be more feasibly post-CPG).
This leads me to wonder though: why do we need to move these wrappers to the new scope/compartment at all? I'm assuming there's a reason we do it for XPCWNs, but given that we've been skipping it for slim wrappers and XHR for this long, I'm wondering what the point is.
If we could kill this code, just use cross-compartment wrappers, and transplant when we CloneAndAdopt (as we do now), I would be very happy. ;-)
Reporter | ||
Comment 1•13 years ago
|
||
To clarify, I'm only talking about the whole-scope reparenting that goes on for document.write. We obviously still need to reparent/transplant objects for the per-wrapper CloneAndAdopt case.
Comment 2•13 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #0)
> This leads me to wonder though: why do we need to move these wrappers to the
> new scope/compartment at all? I'm assuming there's a reason we do it for
> XPCWNs, but given that we've been skipping it for slim wrappers and XHR for
> this long, I'm wondering what the point is.
When someone does document.open (directly or indirectly) the desired behavior is that we keep the same document object but clear all global variables/changes from the previous document and restart with a blank slate. In practice for us, this means that we create a new inner window and move the document from the old window to the new one.
The problem with that is that in XPConnect, each inner window has its own XPCWrappedNativeScope. When we change the document's global object to the new inner window, that means that its scope changes. For DOM XPCWrappedNatives, we have an invariant that there is only ever one JS object per C++ object (implying only one XPCWrappedNative as well). Because almost all DOM objects tell XPConnect to use their document as the scope for the new JS object to create (in PreCreate) when the document's scope changes, that means that the next time we try to return one of the DOM objects that was created *before* the call to document.open, we'll look in the *new* scope for the XPCWrappedNative, not find it, and create a second one. This leads to things like crashes and is generally not a good thing.
So, in order to prevent this, when we do a document.open, we update all of the XPCWrappedNatives to be in the new scope so that we can continue to find them. That, incidentally, is why we can skip reparenting slim wrappers, since we can always find them easily (and they're not parented directly to their global, but rather to the document, we don't have to touch them at all).
Reporter | ||
Comment 3•13 years ago
|
||
Blake and I just talked about this on IRC. The main constraint here seems to be the preservation of identity on the document. So we're wondering about just creating a new document and doing a brain transplant, which would let us leave the entire old document in the old compartment/scope.
Reporter | ||
Comment 4•13 years ago
|
||
Alternatively, we could just reuse the recursive code we use for CloneAndAdopt. This would let us kill the nastier parts of MoveWrappers without having to mess with a brain transplant.
Conceptually though, I like the idea of leaving the document on the old scope better.
Comment 5•6 years ago
|
||
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INACTIVE
Comment 6•6 years ago
|
||
Slimwrappers are no more. For webidl objects, wrappercaching means most of the issues comment 2 describes are not a problem. So I think we should wontfix this.
Resolution: INACTIVE → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•