Created attachment 131035 [details] Patch

Comment on attachment 131035 [details] Patch Attachment 131035 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11906598

Comment on attachment 131035 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131035&action=review > Source/WebCore/Modules/mediastream/IceCandidate.h:54 > + String label(); > + String candidateLine(); > + > + String toSdp(); Should these be "const String&" ? > Source/WebCore/platform/mediastream/IceCandidateDescriptor.h:48 > + String label() { return m_label; } > + String candidateLine() { return m_candidateLine; } const String&

Comment on attachment 131035 [details] Patch Looks like you've got build problem on GTK. This patch generally looks fine, but I'd like to resolve Bug 80692 (or at least come to agreement there with Adam) before we start landing these patches.

Created attachment 131306 [details] Patch

Comment on attachment 131035 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131035&action=review >> Source/WebCore/Modules/mediastream/IceCandidate.h:54 >> + String toSdp(); > > Should these be "const String&" ? toSdp will return a generated string and therefore can't return a const String& but the two accessors have been fixed. >> Source/WebCore/platform/mediastream/IceCandidateDescriptor.h:48 >> + String candidateLine() { return m_candidateLine; } > > const String& Fixed.

Sure thing, just uploaded an uploaded an patch to see if I fixed GTK (which btw I can't build either on my Ubuntu box or Mac for some strange reason) (In reply to comment #4) > (From update of attachment 131035 [details]) > Looks like you've got build problem on GTK. This patch generally looks fine, but I'd like to resolve Bug 80692 (or at least come to agreement there with Adam) before we start landing these patches.

Comment on attachment 131306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131306&action=review > Source/WebCore/platform/mediastream/IceCandidateDescriptor.cpp:58 > + return MediaStreamCenter::instance().doConstructSdp(this); doConstructSdp -> constructSdp (WebKit doesn't use the prefix "do".)

We can move forward with these patches why we give Adam a chance to respond on Bug 80692 as long as we don't have a dependency on the final names for the PeerConnection object.

Created attachment 131602 [details] Patch

Comment on attachment 131306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131306&action=review >> Source/WebCore/platform/mediastream/IceCandidateDescriptor.cpp:58 >> + return MediaStreamCenter::instance().doConstructSdp(this); > > doConstructSdp -> constructSdp (WebKit doesn't use the prefix "do".) Fixed.

Thanks.

Comment on attachment 131602 [details] Patch Rejecting attachment 131602 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: to file Source/WebCore/WebCore.gypi.rej patching file Source/WebCore/platform/mediastream/IceCandidateDescriptor.cpp patching file Source/WebCore/platform/mediastream/IceCandidateDescriptor.h patching file Source/WebCore/platform/mediastream/MediaStreamCenter.cpp patching file Source/WebCore/platform/mediastream/MediaStreamCenter.h Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11942657

Created attachment 131815 [details] Patch

Just merge fixes of the makefiles, no code changes att all.

Comment on attachment 131815 [details] Patch Clearing flags on attachment: 131815 Committed r110735: <http://trac.webkit.org/changeset/110735>

All reviewed patches have been landed. Closing bug.