Bug 33371 - Infinite recursion in RuntimeObjectImp::getOwnPropertyNames()
Summary: Infinite recursion in RuntimeObjectImp::getOwnPropertyNames()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Kent Hansen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-08 03:02 PST by Kent Hansen
Modified: 2010-01-14 12:13 PST (History)
3 users (show)

See Also:


Attachments
Proposed patch (3.36 KB, patch)
2010-01-08 03:12 PST, Kent Hansen
ggaren: review+
kent.hansen: commit-queue-
Details | Formatted Diff | Diff
Revised patch (3.53 KB, patch)
2010-01-13 02:18 PST, Kent Hansen
no flags Details | Formatted Diff | Diff
Revised patch (remove unused parameter name) (3.53 KB, patch)
2010-01-13 04:38 PST, Kent Hansen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Hansen 2010-01-08 03:02:20 PST
This causes a crash if Object.keys is called on a plugin object, for example.
Comment 1 Kent Hansen 2010-01-08 03:12:13 PST
Created attachment 46126 [details]
Proposed patch

Patch needs to be adapted once the patch for https://bugs.webkit.org/show_bug.cgi?id=32242 is landed.
Comment 2 WebKit Review Bot 2010-01-08 03:15:46 PST
style-queue ran check-webkit-style on attachment 46126 [details] without any errors.
Comment 3 Geoffrey Garen 2010-01-08 10:11:24 PST
Comment on attachment 46126 [details]
Proposed patch

r=me, but this can't land until the other patch does.
Comment 4 Kent Hansen 2010-01-13 02:18:04 PST
Created attachment 46439 [details]
Revised patch

Yay, that other monster patch finally landed. :)
Comment 5 Kent Hansen 2010-01-13 03:30:01 PST
(In reply to comment #4)
> Created an attachment (id=46439) [details]
> Revised patch

I didn't remove the unused mode parameter name. Style bot didn't complain (and check-webkit-style doesn't if I remove the name, either). Is it an unwritten style rule that the name should/should not be present if the argument is unused, or is it a dontcare?
Comment 6 Kent Hansen 2010-01-13 04:38:06 PST
Created attachment 46445 [details]
Revised patch (remove unused parameter name)

Ouch, with the unused parameter name, Safari didn't build for me (warnings treated as errors).
Whereas on the Qt port, unused parameter warnings are just disabled. That's a bit odd.
Comment 7 Darin Adler 2010-01-13 09:26:18 PST
Comment on attachment 46445 [details]
Revised patch (remove unused parameter name)

Since this patch includes a new regression test, it also needs to include the expected results for that test. The patch otherwise looks great!
Comment 8 Kent Hansen 2010-01-14 01:16:02 PST
(In reply to comment #7)
> (From update of attachment 46445 [details])
> Since this patch includes a new regression test, it also needs to include the
> expected results for that test. The patch otherwise looks great!

The test will continue to say SUCCESS if all of it succeeds (including the new part), like before. But it now has a new potential FAILURE output.
So the expected results don't need to be changed, unless you want me to change the test to have separate PASS output for each "part" of the test.
Comment 9 Kent Hansen 2010-01-14 01:48:31 PST
Comment on attachment 46445 [details]
Revised patch (remove unused parameter name)

Confused about Darin's comment (see my response in comment #8), setting review? again in hope of being advised before I do any more changes to the test.
Comment 10 Darin Adler 2010-01-14 11:01:24 PST
(In reply to comment #8)
> The test will continue to say SUCCESS if all of it succeeds (including the new
> part), like before. But it now has a new potential FAILURE output.
> So the expected results don't need to be changed, unless you want me to change
> the test to have separate PASS output for each "part" of the test.

OK. My mistake.
Comment 11 WebKit Commit Bot 2010-01-14 12:13:36 PST
Comment on attachment 46445 [details]
Revised patch (remove unused parameter name)

Clearing flags on attachment: 46445

Committed r53285: <http://trac.webkit.org/changeset/53285>
Comment 12 WebKit Commit Bot 2010-01-14 12:13:42 PST
All reviewed patches have been landed.  Closing bug.