Fix enumerability handling in the window's resolve hook

RESOLVED FIXED in Firefox 56

Status

()

defect
RESOLVED FIXED
2 years ago
2 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

53 Branch
mozilla56
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(1 attachment)

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.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
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?
Attachment #8876909 - Flags: review?(kyle) → review+
> Shouldn't this be compared against -1?

Er, yes it is.  Good catch!

Comment 4

2 years ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6977697a2dcf
Fix enumerability handling in the window resolve hook.  r=qdot

Comment 5

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6977697a2dcf
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
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
Wow.  We have 6-7 MB of lazy prototype stuff...   There's got to be a better way to do this.  :(
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.