[webkit-reviews] review granted: [Bug 21769] MessagePort should be GC protected if there are messages to be delivered : [Attachment 24538] proposed fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Alexey Proskuryakov <ap at webkit.org>'s
request for review:
Bug 21769: MessagePort should be GC protected if there are messages to be
delivered
https://bugs.webkit.org/show_bug.cgi?id=21769

Attachment 24538: proposed fix
https://bugs.webkit.org/attachment.cgi?id=24538&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+	 // Warning: using the below methods is risky, as the queue state may
change by the time they return. Extra caution is advised.

I like what this comment is warning about, but I'm not fond of "risky" and
"caution" when describing how to use functions. And "queue state" is also
pretty vague. It would be better if this was concrete about what the
consideration is. I think that "be careful" doesn't help as much as specifics.
Maybe a comment more like "the result of isEmpty() is only useful if there's
some other guarantee that no one is going to add messages to the queue on
another thread" would be one good example. I think that's better than talking
about risk and caution. The comment in MessagePort::hasPendingActivity is nice
and clear on this point.

r=me