[webkit-reviews] review granted: [Bug 73339] Add an internal targetOrigin member to MessageEvent : [Attachment 117451] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 1 11:42:00 PST 2011


Darin Adler <darin at apple.com> has granted Karl Koscher
<supersat at chromium.org>'s request for review:
Bug 73339: Add an internal targetOrigin member to MessageEvent
https://bugs.webkit.org/show_bug.cgi?id=73339

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117451&action=review


> Source/WebCore/dom/MessageEvent.cpp:87
>      , m_origin("")
>      , m_lastEventId("")
> +    , m_targetOrigin("")

Passing "" is unnecessarily inefficient. We should use emptyString() instead.

> Source/WebCore/dom/MessageEvent.h:138
> +    // For internal use only -- not exposed to any bindings

I think this is a confusing comment. There are many data members in many
classes with this property.

A more helpful comment would explain the purpose of this, why it exists, and
that might make it clear why it would not be exposed. Generally, comments
should say “why” rather than “what”.


More information about the webkit-reviews mailing list