[webkit-reviews] review requested: [Bug 68462] Update PeerConnection to use WebCore platform interfaces : [Attachment 110603] Patch 5

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 11 16:42:10 PDT 2011


Adam Bergkvist <adam.bergkvist at ericsson.com> has asked	for review:
Bug 68462: Update PeerConnection to use WebCore platform interfaces
https://bugs.webkit.org/show_bug.cgi?id=68462

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

------- Additional Comments from Adam Bergkvist <adam.bergkvist at ericsson.com>
(In reply to comment #6)

> I can understand the aesthetic issue, but we use these various types to
communicate different ownership patterns.  We use this type (somewhat rarely)
to mean that it's essential that the caller explicitly hold a reference to the
object (e.g., on the stack) because this function could trigger a cascade of
dereferences.
> 
> In the end, it's just a style thing and not a big deal, but it's slightly
more consistent with the rest of WebKit to just take a raw pointer here.

Let's go with raw pointers for consistency.
 
> > The initial readyState NEW is not handled in the switch. An alternative
would be to add case NEW and move the ASSERT there.
> 
> Please do.  That's part of why we like that style.  :)

Fixed.

I'll upload the next patch which updates MediaStream/LocalMediaStream in
http://webkit.org/b/68464


More information about the webkit-reviews mailing list