Bug 81893 - Remove custom bindings form ScriptProfileNode.idl of attribute type Array.
Summary: Remove custom bindings form ScriptProfileNode.idl of attribute type Array.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vineet Chaudhary (vineetc)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-22 05:14 PDT by Vineet Chaudhary (vineetc)
Modified: 2012-03-22 07:36 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.86 KB, patch)
2012-03-22 05:23 PDT, Vineet Chaudhary (vineetc)
haraken: review+
haraken: commit-queue-
Details | Formatted Diff | Diff
Updated Patch (3.97 KB, patch)
2012-03-22 06:08 PDT, Vineet Chaudhary (vineetc)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vineet Chaudhary (vineetc) 2012-03-22 05:14:40 PDT
Remove custom bindings for attribute type Array from ScriptProfileNode.idl.
Replace readonly attribute [CustomGetter] Array children; 
To         readonly attribute sequence<ProfileNode> children;
Comment 1 Vineet Chaudhary (vineetc) 2012-03-22 05:22:50 PDT
(In reply to comment #0)
> Remove custom bindings for attribute type Array from ScriptProfileNode.idl.
> Replace readonly attribute [CustomGetter] Array children; 
> To         readonly attribute sequence<ProfileNode> children;

Sorry for typo shouldBe readonly attribute sequence<ScriptProfileNode> children;
Comment 2 Vineet Chaudhary (vineetc) 2012-03-22 05:23:34 PDT
Created attachment 133230 [details]
Patch

Patch for review.
Comment 3 Kentaro Hara 2012-03-22 05:25:21 PDT
Looks OK. Would you change r to r? if you want a review?
Comment 4 Vineet Chaudhary (vineetc) 2012-03-22 05:26:22 PDT
Comment on attachment 133230 [details]
Patch

Thanks for quick review. :)
Comment 5 Kentaro Hara 2012-03-22 05:28:59 PDT
Comment on attachment 133230 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133230&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests.

Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)".

> Source/WebCore/ChangeLog:10
> +        replace [CustomGetter] Array with sequence<ScriptProfileNode>.

Normally, we write such explanations between the "Reviewed by" line and the "Test:" line.
Comment 6 Vineet Chaudhary (vineetc) 2012-03-22 05:34:44 PDT
(In reply to comment #5)
> (From update of attachment 133230 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133230&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        No new tests.
> 
> Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)".

You mean in LayoutTests? Sure I will do that. Also do I need to add this to TestObj.idl too?
Comment 7 Kentaro Hara 2012-03-22 05:38:41 PDT
(In reply to comment #6)
> > Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)".
> 
> You mean in LayoutTests?

Yes. The line will help people understand what your change is going to affect.

> Also do I need to add this to TestObj.idl too?

No:) The test is already added in the previous patch.
Comment 8 Kentaro Hara 2012-03-22 05:41:04 PDT
(In reply to comment #5)
> Would you please write a test that _can_ be affected by this change, if the change were wrong? Something like "Test: xxx.html (No change in the test result)".

Just a clarification: I didn't mean to add a new test. I meant you can add a line that describes an existing test that might be affected by your change.
Comment 9 Vineet Chaudhary (vineetc) 2012-03-22 06:08:34 PDT
Created attachment 133242 [details]
Updated Patch

Sorry for misunderstanding I was not aware of the new style in Changelogs.
Comment 10 Kentaro Hara 2012-03-22 06:26:42 PDT
Comment on attachment 133242 [details]
Updated Patch

OK. Thanks!
Comment 11 WebKit Review Bot 2012-03-22 07:36:08 PDT
Comment on attachment 133242 [details]
Updated Patch

Clearing flags on attachment: 133242

Committed r111692: <http://trac.webkit.org/changeset/111692>
Comment 12 WebKit Review Bot 2012-03-22 07:36:13 PDT
All reviewed patches have been landed.  Closing bug.