Closed Bug 1012798 Opened 5 years ago Closed 5 years ago

Remove the expando hackery we have for window.window

Categories

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

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(4 files, 2 obsolete files)

Right now this is explicitly defined as an expando on Window.  We should probably move to using a StoreInSlot property.
Attached patch sketchSplinter Review
We really want to use StoreInSlot here, but need to make that work with our situation in this case, where outerizing while creating the window object will crash.  Basically, need to special-case this getter somehow.
Note to self: talked to Peter and there are two options:

1)  Just special-case the codegen to not spit out the "get in the wrapping func" bit for window.window.

2)  Change window.window to have a self-hosted getter of some sort.  We'd need to figure out what sort, and how to have it do (or in Ion not do as needed!) this typechecks.
BTW, I'd be ok with doing 1 for now if 2 is going to take too long.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
This makes things pretty close to as fast as they used to be.  The one issue left is a pointless guard on the window's shape; I filed bug 1100757 on that.
Attachment #8524322 - Flags: review?(efaustbmo)
Comment on attachment 8524322 [details] [diff] [review]
part 3.  When doing a DOM slot get in Ion, check whether we're doing a get of a constant value on a singleton object (e.g. .window) and if so just use the constant value directly

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

Yeah, this is cute. r=me
Attachment #8524322 - Flags: review?(efaustbmo) → review+
Comment on attachment 8524319 [details] [diff] [review]
part 2.  Stop defining a value property named "window" on the global

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

::: dom/base/nsGlobalWindow.cpp
@@ +2597,5 @@
>  
>      if (!aState) {
> +      // Get the "window" property once so it will be cached on our inner.  We
> +      // have to do this here, not in binding code, because this has to happen
> +      // after we've created the GetWrapperPreserveColor() for our outer.

Hmm, that last sentence doesn't make sense, did you mean "after we've created our new outer window proxy"?
Attachment #8524319 - Flags: review?(peterv) → review+
I meant created it and set it on the outer nsGlobalWindow.  I'll adjust the comment to make that clearer.
Attachment #8524319 - Attachment is obsolete: true
Attachment #8526022 - Attachment is obsolete: true
Comment on attachment 8524318 [details] [diff] [review]
part 1.  Add one more available reserved slot on globals, because we need it for Window

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

All yours.
Attachment #8524318 - Flags: review?(jorendorff) → review+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.