1372371 - Fix enumerability handling in the window's resolve hook

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 1364816 that is happening to not bite us because Object.keys doesn't take the fast path of just asking for names on the Window at the moment, which means it doesn't depend on the "enumerable only" boolean passed to the newResolve hook.

We should fix both of these things.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix enumerability handling in the window resolve hook

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8876909 [details] [diff] [review]
Fix enumerability handling in the window resolve hook

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

r+ with test issue addressed

::: dom/base/test/test_window_own_props.html
@@ +21,5 @@
> +
> +  /** Test for Bug 1372371 **/
> +  var list = Object.getOwnPropertyNames(window);
> +  // Pick an interface name we would not have resolved here yet.
> +  isnot(list.indexOf("WebSocket"), 0, "WebSocket should be a property");

Shouldn't this be compared against -1?

Comment 3

[Boris Zbarsky [:bzbarsky]]

> Shouldn't this be compared against -1?

Er, yes it is.  Good catch!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6977697a2dcf
Fix enumerability handling in the window resolve hook.  r=qdot

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6977697a2dcf

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

congratulations, this bug shows an improvement in are we slim yet memory tests:
== Change summary for alert #7298 (as of June 14 2017 23:32 UTC) ==

Improvements:

  3%  JS summary windows10-64-vm opt      186,091,168.93 -> 179,809,159.04
  3%  JS summary linux64 opt              179,213,106.24 -> 173,297,798.10

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=7298

Comment 7

[Boris Zbarsky [:bzbarsky]]

Wow.  We have 6-7 MB of lazy prototype stuff...   There's got to be a better way to do this.  :(