1012798 - Remove the expando hackery we have for window.window

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Right now this is explicitly defined as an expando on Window.  We should probably move to using a StoreInSlot property.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: sketch

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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.

Comment 3

[Peter Van der Beken [:peterv]]

BTW, I'd be ok with doing 1 for now if 2 is going to take too long.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Add one more available reserved slot on globals, because we need it for Window

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. Stop defining a value property named "window" on the global

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: 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

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.

Comment 7

[Eric Faust [:efaust]]

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

Comment 8

[Peter Van der Beken [:peterv]]

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"?

Comment 9

[Boris Zbarsky [:bzbarsky]]

I meant created it and set it on the outer nsGlobalWindow.  I'll adjust the comment to make that clearer.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 updated to review comments and with wpt annotations fixed

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 with yet another todo removed; should pass tests now

Comment 12

[Jason Orendorff [:jorendorff]]

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.

Comment 13

[Boris Zbarsky [:bzbarsky]]

  https://hg.mozilla.org/integration/mozilla-inbound/rev/cfeb16066597
  https://hg.mozilla.org/integration/mozilla-inbound/rev/013ca85eaa42
  https://hg.mozilla.org/integration/mozilla-inbound/rev/b58035bdc84f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cfeb16066597
https://hg.mozilla.org/mozilla-central/rev/013ca85eaa42
https://hg.mozilla.org/mozilla-central/rev/b58035bdc84f