Bug 81893 – Remove custom bindings form ScriptProfileNode.idl of attribute type Array.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Vineet Chaudhary (vineetc)]

Remove custom bindings for attribute type Array from ScriptProfileNode.idl.
Replace readonly attribute [CustomGetter] Array children; 
To         readonly attribute sequence<ProfileNode> children;

[Vineet Chaudhary (vineetc)]

(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;

[Vineet Chaudhary (vineetc)]

Created attachment 133230 [details]
Patch

Patch for review.

[Kentaro Hara]

Looks OK. Would you change r to r? if you want a review?

[Vineet Chaudhary (vineetc)]

Comment on attachment 133230 [details]
Patch

Thanks for quick review. :)

[Kentaro Hara]

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.

[Vineet Chaudhary (vineetc)]

(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?

[Kentaro Hara]

(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.

[Kentaro Hara]

(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.

[Vineet Chaudhary (vineetc)]

Created attachment 133242 [details]
Updated Patch

Sorry for misunderstanding I was not aware of the new style in Changelogs.

[Kentaro Hara]

Comment on attachment 133242 [details]
Updated Patch

OK. Thanks!

[WebKit Review Bot]

Comment on attachment 133242 [details]
Updated Patch

Clearing flags on attachment: 133242

Committed r111692: <http://trac.webkit.org/changeset/111692>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.