browser_dbg_propertyview-filter-05.js checks for DOM objects are broken

RESOLVED FIXED in Firefox 21

Status

RESOLVED FIXED
6 years ago
2 months ago

People

(Reporter: peterv, Assigned: vporof)

Tracking

unspecified
Firefox 21
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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?
(Reporter)

Updated

6 years ago
Flags: needinfo?(vporof)
(Assignee)

Comment 1

6 years ago
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.
Flags: needinfo?(vporof)
(Assignee)

Comment 2

6 years ago
Created attachment 701417 [details] [diff] [review]
v1
Attachment #701417 - Flags: review?(past)
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)

Comment 4

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/109cd2191d91
Assignee: nobody → vporof
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/109cd2191d91
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21

Updated

2 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.