Bug 85491 - MediaStream API: Make PeerConnection00's API fully compliant with the draft
Summary: MediaStream API: Make PeerConnection00's API fully compliant with the draft
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Tommy Widenflycht
URL:
Keywords:
Depends on:
Blocks: 80589
  Show dependency tree / graph
 
Reported: 2012-05-03 06:29 PDT by Tommy Widenflycht
Modified: 2012-05-04 10:54 PDT (History)
12 users (show)

See Also:

Attachments
Patch (16.62 KB, patch)
2012-05-03 06:38 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Patch (19.50 KB, patch)
2012-05-03 07:34 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Patch (22.97 KB, patch)
2012-05-04 04:14 PDT, Tommy Widenflycht 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.