WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
42258
Improve coverage of fast/dom/Window/window-properties test
https://bugs.webkit.org/show_bug.cgi?id=42258
Summary
Improve coverage of fast/dom/Window/window-properties test
Kent Hansen
Reported
2010-07-14 07:10:14 PDT
The fast/dom/Window/window-properties test uses a for..in statement to enumerate the properties of an object. An issue with that approach is that non-enumerable properties (such as the built-in ECMA properties) are not enumerated, and hence not tested, even though they are (quote test description) "reachable from the window object". Since we now have the ES5 function Object.getOwnPropertyNames implemented, it could be used in place of for..in; Object.getOwnPropertyNames returns both enumerable and non-enumerable properties. This will increase the test coverage, and as a bonus the test won't be sensitive to changes in the enumerable flag of existing properties (see
https://bugs.webkit.org/show_bug.cgi?id=28771
). It should be OK to change the test like this because the test is made for checking reachability, not enumerability (i.e. the more, the merrier).
Attachments
Patch
(45.10 KB, patch)
2010-07-14 07:27 PDT
,
Kent Hansen
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Kent Hansen
Comment 1
2010-07-14 07:27:09 PDT
Created
attachment 61516
[details]
Patch Introduces a helper function, propertiesOnObject(), to compute an object's properties. Properties in the prototype chain are added as well, to correspond with for..in behavior. However, it stops at the built-in prototypes to avoid spewing out a bunch of "redundant" properties (e.g. including properties from Object.prototype for every visited object blows up the expected results immensely). I've updated the gtk results even though this test is currently skipped on that platform (
https://bugs.webkit.org/show_bug.cgi?id=30594
).
Sam Weinig
Comment 2
2010-07-15 22:19:36 PDT
Comment on
attachment 61516
[details]
Patch You may have other platforms that need to have there results updated. We should really try and figure out what the differences are and and move those properties to their own tests.
Kent Hansen
Comment 3
2010-07-16 05:39:10 PDT
(In reply to
comment #2
)
> (From update of
attachment 61516
[details]
) > You may have other platforms that need to have there results updated. We should really try and figure out what the differences are and and move those properties to their own tests.
There are no existing *-expected.txt that I haven't updated, at least. The only port I haven't tested is Windows. The test is skipped on Chromium already (from test_expectations.txt: "This fails because we're missing various useless apple-specific properties on the window object"). The added properties due to the patch can be divided into four groups: - ECMA DontEnum properties. These should always be there, regardless of features enabled. - Properties of existing prototype objects (e.g. window.DOMException.prototype.toString). These are marked with attribute DontEnum in the IDL files, they're not apple-specific. - property "length" on some constructors. This is due to somewhat of a quirk in the JS code generator. A length property is only created if the constructor expects > 0 arguments, so only constructors with e.g. ConstructorParameters=1 in the IDL, or some manually implemented classes (e.g. JSAudioConstructor), will have a length property. This is "apple" (JSC) specific AFAICT. - styleMedia.constructor property. This is inherited from the StyleMedia prototype object, should always be there. Regarding differences between ports (relative to Safari): - Qt is missing some of the SVG constructors, adds TouchEvent constructor and webkitNotifications - Gtk is missing BeforeLoadEvent, DomStringMap, Element.prototype.webkitMatchesSelector, Event.prototype.stopImmediatePropagation; accounts for a typo ("EventSourceContructor" :D ); a bunch of SVG stuff, ... It would indeed be nice to split this and other "world tests" into smaller chunks that can be maintained and updated more easily; e.g. a separate test for SVG classes. This would mean using a static (generated?) list of classes rather than relying on property enumeration. There is some relevant discussion in
https://bugs.webkit.org/show_bug.cgi?id=32596
WebKit Commit Bot
Comment 4
2010-07-19 01:23:51 PDT
Comment on
attachment 61516
[details]
Patch Clearing flags on attachment: 61516 Committed
r63645
: <
http://trac.webkit.org/changeset/63645
>
WebKit Commit Bot
Comment 5
2010-07-19 01:23:55 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug