Bug 165736 – Remove uses of Dictionary in WebRTC IDL files

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Darin Adler]

Got this partly done.

[Darin Adler]

Created attachment 296833 [details]
Patch

[Darin Adler]

Created attachment 296835 [details]
Patch

[Build Bot]

Comment on attachment 296835 [details]
Patch

Attachment 296835 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2692776

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

[Build Bot]

Created attachment 296840 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 296835 [details]
Patch

Attachment 296835 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2692783

New failing tests:
fast/mediastream/RTCSessionDescription.html
fast/mediastream/RTCPeerConnection-getConfiguration.html
fast/mediastream/RTCIceCandidate.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
fast/mediastream/RTCPeerConnection.html

[Build Bot]

Created attachment 296841 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 296835 [details]
Patch

Attachment 296835 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2692788

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

[Build Bot]

Created attachment 296842 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Build Bot]

Comment on attachment 296835 [details]
Patch

Attachment 296835 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/2693492

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html

[Build Bot]

Created attachment 296848 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Darin Adler]

Created attachment 296859 [details]
Patch

[Build Bot]

Comment on attachment 296859 [details]
Patch

Attachment 296859 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/2697000

New failing tests:
fast/mediastream/RTCSessionDescription.html
fast/mediastream/RTCPeerConnection-getConfiguration.html
fast/mediastream/RTCIceCandidate.html
fast/mediastream/RTCPeerConnection.html

[Build Bot]

Created attachment 296862 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Sam Weinig]

Comment on attachment 296859 [details]
Patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1364
> +                    # FIXME: Replace with a more general solution, presumably using C++ code in JSDOMConvert rather than more special cases here.
> +                    my $nullCheck = "dictionary.${key}";
> +                    my $value = "*dictionary.${key}";
> +                    if ($codeGenerator->IsStringType($member->type)) {
> +                        $nullCheck = "!dictionary.${key}.isNull()";
> +                        $value = "dictionary.${key}";
> +                    }
> +                    $result .= "    if ($nullCheck) {\n";
> +                    $result .= "        auto ${key}Value = toJS<$IDLType>(state, globalObject, $value);\n";

The general solution is:

                    $result .= "    if (!${IDLType}::isNullValue(dictionary.${key})) {\n";
                    $result .= "        auto ${key}Value = toJS<${IDLType}>(state, globalObject, ${IDLType}::extractValueFromNullable(dictionary.${key}));\n";

[Darin Adler]

(In reply to comment #14)
> The general solution is:

Thanks. I will switch to that.

[Darin Adler]

Created attachment 296869 [details]
Patch

[Darin Adler]

I think this patch is done. The failure on GTk seems to be something wrong with the bot.

[Sam Weinig]

Comment on attachment 296869 [details]
Patch

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

> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:62
> +using MediaDescriptionVector = Vector<PeerMediaDescription>;
> +using RtpTransceiverVector = Vector<RefPtr<RTCRtpTransceiver>>;

I'm not generally a fan of these type aliases that just shorten the type name of a Vector, not sure they add much, but I see you are just moving them.

[Darin Adler]

Comment on attachment 296869 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/MediaEndpointPeerConnection.cpp:62
>> +using RtpTransceiverVector = Vector<RefPtr<RTCRtpTransceiver>>;
> 
> I'm not generally a fan of these type aliases that just shorten the type name of a Vector, not sure they add much, but I see you are just moving them.

In a lot of my recent patches I have been removing these entirely. In this case I was just happy to get them out of the header and didn’t follow through to see how often they were used to get rid of them completely.

[WebKit Commit Bot]

Comment on attachment 296869 [details]
Patch

Clearing flags on attachment: 296869

Committed r209695: <http://trac.webkit.org/changeset/209695>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.