Created attachment 299474 [details] Patch

Created attachment 299555 [details] Patch

Attachment 299555 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 299556 [details] Adding PageConfiguration integration

Attachment 299556 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 299556 [details] Adding PageConfiguration integration View in context: https://bugs.webkit.org/attachment.cgi?id=299556&action=review > Source/WebCore/page/Page.cpp:205 > + , m_libWebRTCClient(*pageConfiguration.libWebRTCClient) When the PageConfiguration goes out of scope, this will be pointing to freed memory. > Source/WebCore/page/Page.h:642 > + LibWebRTCClient& m_libWebRTCClient; Let's have the Page keep a UniqueRef<PeerConnectionProvider> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCClient.h:42 > +class WEBCORE_EXPORT LibWebRTCClient { > +public: > + virtual rtc::scoped_refptr<webrtc::PeerConnectionInterface> createPeerConnection(webrtc::PeerConnectionObserver&) = 0; I think this should be called a PeerConnectionProvider. It should be inside of ENABLE(WEBRTC) except the createPeerConnection function, which should be inside a USE(LIBWEBRTC)

Created attachment 299603 [details] Patch

Attachment 299603 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 299603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299603&action=review > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCClient.h:40 > +class WEBCORE_EXPORT LibWebRTCClient { My review feedback was not addressed.

Created attachment 299647 [details] Patch

Attachment 299647 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/page/PageConfiguration.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 299648 [details] Patch

Attachment 299648 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/page/PageConfiguration.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 299651 [details] Patch

Attachment 299651 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/page/PageConfiguration.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 299704 [details] Patch

Attachment 299704 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/page/PageConfiguration.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 2 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 299704 [details] Patch Attachment 299704 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2947146 New failing tests: imported/w3c/web-platform-tests/html/browsers/history/the-location-interface/location-protocol-setter-non-broken.html

Created attachment 299709 [details] Archive of layout-test-results from ews101 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-elcapitan Platform: Mac OS X 10.11.6

Created attachment 299711 [details] Trying to fix EFL/GTK builds

Attachment 299711 [details] did not pass style-queue: ERROR: Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:59: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/page/PageConfiguration.cpp:53: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] Total errors found: 2 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 299711 [details] Trying to fix EFL/GTK builds View in context: https://bugs.webkit.org/attachment.cgi?id=299711&action=review > Source/WebKit/mac/ChangeLog:3 > + [WebRTC] Add support for WK1 libwebrtc endpoint Duplicate ChangeLog entry. > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:38 > + virtual rtc::scoped_refptr<webrtc::PeerConnectionInterface> createPeerConnection(webrtc::PeerConnectionObserver&) = 0; If we make this not purely virtual, then we can probably get rid of LibWebRTCEmptyProvider and the WebLibWebRTCProvider.h in Source/WebKit. > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:43 > +class LibWebRTCEmptyProvider final : public LibWebRTCProvider { I think this should be in EmptyClients.h/cpp like createEmptyEditorClient > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCUtils.cpp:60 > + Function<void()> function; Maybe this should be called callback to be consistent. > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:379 > + , makeUniqueRef<WebCore::LibWebRTCEmptyProvider>() I assume the next step is to make this return a LibWebRTCProvider that returns something that makes connection proxies, right?

Created attachment 299718 [details] Patch

Comment on attachment 299718 [details] Patch This version removes many of the USE(LIBWEBRTC) macros. If you don't use libwebrtc, you will have a pointer on the Page to an object that does nothing. I think that's worth having much cleaner code everywhere.

Created attachment 299720 [details] Patch

(In reply to comment #26) > http://trac.webkit.org/r211161 It broke the Apple Mac cmake build: /Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebKit/mac/WebView/WebView.mm:160:9: fatal error: 'WebCore/LibWebRTCProvider.h' file not found #import <WebCore/LibWebRTCProvider.h> ^ 1 warning and 1 error generated. I don't know how Apple Mac cmake build generates this magic forwarding headers, I let you guys to fix the build you broke yourself.

I found it, here you are - https://trac.webkit.org/changeset/211208

Thanks, Ossy!