Bug 85491

Summary: MediaStream API: Make PeerConnection00's API fully compliant with the draft
Product: WebKit Reporter: Tommy Widenflycht <tommyw>
Component: WebCore Misc.Assignee: Tommy Widenflycht <tommyw>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric.carlson, feature-media-reviews, fishd, gustavo, jamesr, ojan, pnormand, tkent+wkapi, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 80589    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Tommy Widenflycht 2012-05-03 06:29:26 PDT
Mainly make the relevant API's use objects (aka Dictionaries) instead of the temporary strings, but also making a few API's exception aware and changing the name of a flag.
Comment 1 Tommy Widenflycht 2012-05-03 06:38:47 PDT
Created attachment 140000 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-03 06:41:50 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Philippe Normand 2012-05-03 06:43:52 PDT
Comment on attachment 140000 [details]
Patch

Attachment 140000 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12610242
Comment 4 Tommy Widenflycht 2012-05-03 07:34:41 PDT
Created attachment 140010 [details]
Patch
Comment 5 Adam Barth 2012-05-03 10:50:36 PDT
Comment on attachment 140010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140010&action=review

> Source/WebCore/Modules/mediastream/PeerConnection00.cpp:260
> +    if (!ec)
> +        return;

Is this backwards?  I thought ec == 0 is success.

> LayoutTests/ChangeLog:1
> +2012-05-03  Tommy Widenflycht  <tommyw@google.com>

Looks like this ChangeLog got corrupted a bit.
Comment 6 Darin Adler 2012-05-03 15:58:22 PDT
Comment on attachment 140010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140010&action=review

>> Source/WebCore/Modules/mediastream/PeerConnection00.cpp:260
>> +        return;
> 
> Is this backwards?  I thought ec == 0 is success.

More importantly, if this is backwards, why didn’t a test catch that fact?
Comment 7 Tommy Widenflycht 2012-05-04 04:14:32 PDT
Created attachment 140188 [details]
Patch
Comment 8 Tommy Widenflycht 2012-05-04 04:17:52 PDT
Comment on attachment 140010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=140010&action=review

>>> Source/WebCore/Modules/mediastream/PeerConnection00.cpp:260
>>> +        return;
>> 
>> Is this backwards?  I thought ec == 0 is success.
> 
> More importantly, if this is backwards, why didn’t a test catch that fact?

Yes, it was backward. And I am ashamed. Test now added. It's so cold in Sweden that my brain froze.

>> LayoutTests/ChangeLog:1
>> +2012-05-03  Tommy Widenflycht  <tommyw@google.com>
> 
> Looks like this ChangeLog got corrupted a bit.

Fixed.
Comment 9 Adam Barth 2012-05-04 10:13:26 PDT
Comment on attachment 140188 [details]
Patch

Thanks.
Comment 10 WebKit Review Bot 2012-05-04 10:54:37 PDT
Comment on attachment 140188 [details]
Patch

Clearing flags on attachment: 140188

Committed r116127: <http://trac.webkit.org/changeset/116127>
Comment 11 WebKit Review Bot 2012-05-04 10:54:43 PDT
All reviewed patches have been landed.  Closing bug.