[webkit-reviews] review denied: [Bug 80200] Add transfer map argument to Intent constructor : [Attachment 134132] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 27 14:46:10 PDT 2012


Adam Barth <abarth at webkit.org> has denied Greg Billock <gbillock at google.com>'s
request for review:
Bug 80200: Add transfer map argument to Intent constructor
https://bugs.webkit.org/show_bug.cgi?id=80200

Attachment 134132: Patch
https://bugs.webkit.org/attachment.cgi?id=134132&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134132&action=review


This looks good, but please add a test that calls these new constructors.

> Source/WebCore/ChangeLog:7
> +	   Add transfer map argument to Intent constructor
> +	   https://bugs.webkit.org/show_bug.cgi?id=80200
> +
> +	   Reviewed by NOBODY (OOPS!).
> +

Can you add more information to this ChangeLog?  There was a thread on
webkit-dev recently requesting that folks put more information in their
ChangeLogs.  For example, you might add a link to the line of the spec you're
implementing and/or explain why adding transfer map argument to Intent
constructor is useful.

> Source/WebCore/Modules/intents/Intent.cpp:66
> +    OwnPtr<MessagePortChannelArray> channels =
MessagePort::disentanglePorts(&ports, ec);

I'm slightly unsure about this line, but Comment #36 seems to indiciate that
this is correct.


More information about the webkit-reviews mailing list