browser_dbg_propertyview-filter-05.js has a function called ignoreExtraMatchedProperties that seems to want to filter out DOM prototypes. It does it by matching the string value against "DOM" or "XPC" and "__proto__", but that doesn't work for the new DOM bindings (whose stringification follows the WebIDL spec). We've already had to change this test multiple times when using new DOM bindings for more classes, so it would be good to fix this test to not depend on the old incorrect XPConnect stringification. I have no idea why we have to filter these properties out, Victor?
This test (and a similar one, browser_dbg_propertyview-filter-01.js) filters all the variables and properties available (expanded) at some point in the debugger's Variables View pane, to make sure searching works properly. It does so for all the different scopes available (including the global, which in this case is the window), when the debugger is paused on a breakpoint, in the top-level frame. The origin of the ignoreExtraMatchedProperties function was in browser_dbg_propertyview-filter-01.js, where a variable available on the global scope, "tracemallocdumpallocations" was present *only* in debug builds, causing the test to fail. That function simply worked around this inconvenience by filtering out that variable. It also assumed that the global object is a moving target, hence more variables may need to be filtered out in the future. Since then, ignoreExtraMatchedProperties was repurposed to filter out more stuff, both in *filter-01.js, and it was also added in *filter-05.js for similar test failures. It almost sounds to me like some parts of the tests themselves may be considered flawed, since relying on a certain structure (available variables) of a global is inherently wrong. However, it worked pretty well until now and performing tests on this scope adds good and necessary coverage (both for code and performance). Since *filter-01.js didn't pose any problems so far, and only *filter-05.js is rather aggressive when calling ignoreExtraMatchedProperties, it's sufficient to only remove these checks for this test and keep the coverage in *filter-01.js.
Comment on attachment 701417 [details] [diff] [review] v1 Review of attachment 701417 [details] [diff] [review]: ----------------------------------------------------------------- Fine by me.
Attachment #701417 - Flags: review?(past) → review+
Assignee: nobody → vporof
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
You need to log in before you can comment on or make changes to this bug.