Bug 167293

Summary: [WebRTC][Mac] Activate libwebrtc
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, agouaillard, ap, benjamin, bfulgham, buildbot, cdumez, cmarcelo, commit-queue, dbates, eric.carlson, ggaren, gp, joepeck, jonlee, keith_miller, lepinski, mark.lam, mitz, msaboff, rniwa, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=168010
https://bugs.webkit.org/show_bug.cgi?id=168617
https://bugs.webkit.org/show_bug.cgi?id=168634
Bug Depends on: 168647    
Bug Blocks: 167289    
Attachments:
Description Flags
Patch
none
Patch
achristensen: review-
Misc patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Trying the dylib way
none
Adding some more exports
none
Archive of layout-test-results from ews117 for mac-elcapitan
none
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews101 for mac-elcapitan
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
achristensen: review+, buildbot: commit-queue-
Archive of layout-test-results from ews100 for mac-elcapitan none

Description youenn fablet 2017-01-22 10:39:39 PST
LibWebRTC endpoint should be implemented behind a compilation flag until the implementation can actually be deployed.
This will allow not put burden on others to compile/link libwebrtc in this early phase
Comment 1 youenn fablet 2017-01-22 11:02:53 PST
Created attachment 299472 [details]
Patch
Comment 2 youenn fablet 2017-01-22 13:14:38 PST
Created attachment 299481 [details]
Patch
Comment 3 Alex Christensen 2017-01-23 09:49:27 PST
Comment on attachment 299481 [details]
Patch

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

I think this should be USE(LIBWEBRTC) instead.

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:168
>  ENABLE_WEB_RTC = ENABLE_WEB_RTC;
> +ENABLE_LIBWEBRTC_ENDPOINT = ;

It is nice to have these two together, but I think the intent is that this list be alphabetically sorted.
Comment 4 youenn fablet 2017-01-29 21:57:28 PST
Created attachment 300087 [details]
Misc patch
Comment 5 youenn fablet 2017-02-07 17:32:51 PST
Created attachment 300865 [details]
Patch
Comment 6 Geoffrey Garen 2017-02-08 11:24:00 PST
<rdar://problem/30401864>
Comment 7 youenn fablet 2017-02-08 11:35:30 PST
Created attachment 300930 [details]
Patch
Comment 8 Alexey Proskuryakov 2017-02-08 11:43:28 PST
Comment on attachment 300930 [details]
Patch

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

> Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:434
>  (with-filter (extension "com.apple.webkit.microphone")
>      (allow device-microphone))
>  
> +(allow device-microphone)

This doesn't look right. Why do we allow microphone both conditionally and unconditionally?

Also, what tracks removing the unconditional rule?
Comment 9 Brent Fulgham 2017-02-08 11:49:36 PST
Comment on attachment 300930 [details]
Patch

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

> Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:201
> +(allow network*)

Yikes!

>> Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:434
>> +(allow device-microphone)
> 
> This doesn't look right. Why do we allow microphone both conditionally and unconditionally?
> 
> Also, what tracks removing the unconditional rule?

This shouldn't be needed after Bug 167669.
Comment 10 youenn fablet 2017-02-08 11:55:27 PST
(In reply to comment #8)
> Comment on attachment 300930 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300930&action=review
> 
> > Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:434
> >  (with-filter (extension "com.apple.webkit.microphone")
> >      (allow device-microphone))
> >  
> > +(allow device-microphone)
> 
> This doesn't look right. Why do we allow microphone both conditionally and
> unconditionally?
> 
> Also, what tracks removing the unconditional rule?

This patch is not intended to be landed as is. But it allows testing/demoing WebKit with webrtc.
Comment 11 Brent Fulgham 2017-02-08 11:59:33 PST
Comment on attachment 300930 [details]
Patch

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

>> Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:201
>> +(allow network*)
> 
> Yikes!

This shouldn't be needed after Bug 168010.
Comment 12 Alex Christensen 2017-02-08 15:53:03 PST
Created attachment 300980 [details]
Patch
Comment 13 Brent Fulgham 2017-02-09 12:25:33 PST
Created attachment 301070 [details]
Patch
Comment 14 Alex Christensen 2017-02-09 19:11:25 PST
Created attachment 301119 [details]
Patch
Comment 15 Alex Christensen 2017-02-09 19:41:02 PST
http://trac.webkit.org/r212040
Comment 16 mitz 2017-02-09 20:17:16 PST
Comment on attachment 301119 [details]
Patch

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

> Source/WebKit2/Configurations/WebKit.xcconfig:47
> +OTHER_LDFLAGS = $(inherited) $(UNEXPORTED_SYMBOL_LDFLAGS) $(ASAN_OTHER_LDFLAGS) $(FRAMEWORK_AND_LIBRARY_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM) -framework WebKitLegacy -sub_umbrella WebKitLegacy -lboringssl -lwebrtc;

Why do these static libraries need to be linked into both WebCore and WebKit? WebKit already links against WebCore, and it seems like this either already does or can easily lead to redundancy.
Comment 17 youenn fablet 2017-02-09 20:51:45 PST
(In reply to comment #16)
> Comment on attachment 301119 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301119&action=review
> 
> > Source/WebKit2/Configurations/WebKit.xcconfig:47
> > +OTHER_LDFLAGS = $(inherited) $(UNEXPORTED_SYMBOL_LDFLAGS) $(ASAN_OTHER_LDFLAGS) $(FRAMEWORK_AND_LIBRARY_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM) -framework WebKitLegacy -sub_umbrella WebKitLegacy -lboringssl -lwebrtc;
> 
> Why do these static libraries need to be linked into both WebCore and
> WebKit? WebKit already links against WebCore, and it seems like this either
> already does or can easily lead to redundancy.

We'll work on removing these for WebKit and WebKit2.
Comment 18 Alex Christensen 2017-02-09 21:11:02 PST
Rolled out in https://trac.webkit.org/r212042
Comment 20 Alex Christensen 2017-02-09 21:40:38 PST
And it broke a bunch of internal build stuff.
Comment 21 Alex Christensen 2017-02-09 21:44:30 PST
Comment on attachment 301119 [details]
Patch

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

> Source/WTF/wtf/Platform.h:1225
> +#if PLATFORM(COCOA)

32-bit El Capitan will not work because of ARC.  We should probably disable it for El Capitan, for 32-bit, or for both.
Comment 22 youenn fablet 2017-02-10 10:03:02 PST
(In reply to comment #21)
> Comment on attachment 301119 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301119&action=review
> 
> > Source/WTF/wtf/Platform.h:1225
> > +#if PLATFORM(COCOA)
> 
> 32-bit El Capitan will not work because of ARC.  We should probably disable
> it for El Capitan, for 32-bit, or for both.

We should try to remove all files requiring -fobj-arc for mac builds.
For iOS builds, we might need to keep a few of them.
Comment 23 youenn fablet 2017-02-10 15:06:27 PST
Reopening to attach new patch.
Comment 24 youenn fablet 2017-02-10 15:06:32 PST
Created attachment 301199 [details]
Patch
Comment 25 youenn fablet 2017-02-10 15:31:14 PST
Created attachment 301202 [details]
Patch
Comment 26 Build Bot 2017-02-10 17:12:09 PST
Comment on attachment 301202 [details]
Patch

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

New failing tests:
fast/mediastream/RTCPeerConnection-icecandidate-event.html
Comment 27 Build Bot 2017-02-10 17:12:15 PST
Created attachment 301219 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 28 Build Bot 2017-02-10 17:37:07 PST
Comment on attachment 301202 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
Comment 29 Build Bot 2017-02-10 17:37:13 PST
Created attachment 301221 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 30 Build Bot 2017-02-10 17:45:51 PST
Comment on attachment 301202 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
imported/w3c/web-platform-tests/webrtc/RTCDataChannelEvent-constructor.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
Comment 31 Build Bot 2017-02-10 17:45:57 PST
Created attachment 301226 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 32 youenn fablet 2017-02-15 21:31:18 PST
Created attachment 301702 [details]
Trying the dylib way
Comment 33 youenn fablet 2017-02-15 21:58:45 PST
Created attachment 301708 [details]
Adding some more exports
Comment 34 Alex Christensen 2017-02-15 23:17:33 PST
Comment on attachment 301708 [details]
Adding some more exports

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

> Source/WebCore/Configurations/WebCore.xcconfig:69
> +LIBWEBRTC_LDFLAGS = -lwebrtc -framework CoreMedia -framework VideoToolbox;

Instead of -lwebrtc let's do -weak_library <full path to libwebrtc from macros>
Comment 35 Build Bot 2017-02-15 23:48:47 PST
Comment on attachment 301708 [details]
Adding some more exports

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-idl.html
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
imported/w3c/web-platform-tests/webrtc/RTCDataChannelEvent-constructor.html
imported/w3c/web-platform-tests/webrtc/rtcpeerconnection/rtcpeerconnection-constructor.html
Comment 36 Build Bot 2017-02-15 23:48:54 PST
Created attachment 301718 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 37 Build Bot 2017-02-16 04:00:45 PST
Comment on attachment 301708 [details]
Adding some more exports

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

New failing tests:
fast/mediastream/RTCPeerConnection-icecandidate-event.html
Comment 38 Build Bot 2017-02-16 04:00:53 PST
Created attachment 301738 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 39 Build Bot 2017-02-16 05:47:16 PST
Comment on attachment 301708 [details]
Adding some more exports

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
Comment 40 Build Bot 2017-02-16 05:47:22 PST
Created attachment 301751 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 41 Build Bot 2017-02-16 08:04:05 PST
Comment on attachment 301708 [details]
Adding some more exports

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

New failing tests:
imported/w3c/web-platform-tests/webrtc/datachannel-emptystring.html
imported/w3c/web-platform-tests/webrtc/no-media-call.html
fast/scrolling/ios/scroll-events-back-forward.html
imported/w3c/web-platform-tests/webrtc/promises-call.html
Comment 42 Build Bot 2017-02-16 08:04:11 PST
Created attachment 301766 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 43 youenn fablet 2017-02-16 08:23:18 PST
(In reply to comment #34)
> Comment on attachment 301708 [details]
> Adding some more exports
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301708&action=review
> 
> > Source/WebCore/Configurations/WebCore.xcconfig:69
> > +LIBWEBRTC_LDFLAGS = -lwebrtc -framework CoreMedia -framework VideoToolbox;
> 
> Instead of -lwebrtc let's do -weak_library <full path to libwebrtc from
> macros>

Right.
The question is where should we put the libwebrtc library, at the root folder as currently done or within a Framework subfolder like done for other dylib?
I am not sure I have the right settings currently. Mitz, any idea?
Comment 44 youenn fablet 2017-02-16 08:44:11 PST
Or maybe use -weak-lwebrtc for the linking
Comment 45 mitz 2017-02-16 09:06:33 PST
(In reply to comment #44)
> Or maybe use -weak-lwebrtc for the linking

Yeah, that’d be better.

But independently of that you still have to decide the install path and install name for the dylib in all build configurations.

I think in iOS builds the install path and install name can be
/System/Library/PrivateFrameworks/WebCore.framework/Frameworks/libwebrtc.dylib

In normal macOS builds (when WK_USE_OVERRIDE_FRAMEWORKS_DIR is not set to YES) the install path can be
/System/Library/Frameworks/WebKit.framework/Versions/A/Frameworks/WebCore.framework/Versions/A/Frameworks/libwebrtc.dylib
which can also be the install name.

In macOS builds when WK_USE_OVERRIDE_FRAMEWORKS_DIR is set to YES, the install path can be $(WK_OVERRIDE_FRAMEWORKS_DIR) and the install name can be @loader_path/../../../libwebrtc.dylib (which is the path from the WebCore binary to the dylib), assuming the use of @loader_path is compatible with Apple’s combination of use of restricted entitlements and library validation on the various executables in Safari Technology Preview and in its Safari updates.

The libwebrtc headers can probably go into a webrtc subdirectory of WebCore’s PrivateHeaders directory (WebCore’s configuration computes where that ends up being).
Comment 46 youenn fablet 2017-02-17 12:47:15 PST
Created attachment 301967 [details]
Patch
Comment 47 Alex Christensen 2017-02-17 12:54:47 PST
Comment on attachment 301967 [details]
Patch

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

!

> Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig:23
> -EXCLUDED_SOURCE_FILE_NAMES[sdk=macosx*] = audio_device_ios.mm voice_processing_audio_unit.mm audio_device_not_implemented_ios.mm RTCAudioSessionConfiguration.m RTCAudioSessionDelegateAdapter.mm RTCAudioSession.mm RTCAudioSession+Configuration.mm RTCUIApplication.mm;
> +EXCLUDED_SOURCE_FILE_NAMES[sdk=macosx*] = voice_processing_audio_unit.mm;

I remember excluding these because 32-bit El Capitan can't build with -fobjc-arc.  Are these files needed now?

> Source/WebKit2/UIProcess/WebPreferences.cpp:208
> +    return true;

return false if libwebrtc isn't there?
Comment 48 youenn fablet 2017-02-17 13:12:11 PST
Comment on attachment 301967 [details]
Patch

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

>> Source/ThirdParty/libwebrtc/Configurations/libwebrtc.xcconfig:23
>> +EXCLUDED_SOURCE_FILE_NAMES[sdk=macosx*] = voice_processing_audio_unit.mm;
> 
> I remember excluding these because 32-bit El Capitan can't build with -fobjc-arc.  Are these files needed now?

These files are no longer part of the build system.

>> Source/WebKit2/UIProcess/WebPreferences.cpp:208
>> +    return true;
> 
> return false if libwebrtc isn't there?

If we are not using libwebrtc, we always activate WebRTC.
This should allow taking care of GTK port.
Comment 49 Build Bot 2017-02-17 14:17:41 PST
Comment on attachment 301967 [details]
Patch

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

New failing tests:
editing/spelling/spellcheck-async.html
Comment 50 Build Bot 2017-02-17 14:17:48 PST
Created attachment 301987 [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
Comment 51 youenn fablet 2017-02-17 14:40:58 PST
Comment on attachment 301967 [details]
Patch

Test failure is probably unrelated
Comment 52 Alex Christensen 2017-02-20 21:52:24 PST
http://trac.webkit.org/r212699
Comment 53 Alex Christensen 2017-02-21 00:21:38 PST
And out again in http://trac.webkit.org/r212704
Comment 54 Alex Christensen 2017-02-21 19:03:28 PST
Created attachment 302353 [details]
Patch
Comment 55 mitz 2017-02-21 19:36:36 PST
Comment on attachment 302353 [details]
Patch

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

> Source/WTF/wtf/Platform.h:1218
> +asdf

qwer
Comment 56 Joseph Pecoraro 2017-02-21 19:55:15 PST
Comment on attachment 302353 [details]
Patch

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

> Source/WebKit2/Configurations/WebKit.xcconfig:50
> +LIBWEBRTC_LDFLAGS_ =;

Style: Normally we have a space between `=` and `;` for empty values. Sometimes we just leave it off entirely, but I like seeing it explicitly.

> Tools/Scripts/webkitdirs.pm:2154
> +    prependToEnvironmentVariableList("DYLD_LIBRARY_PATH", $dyldFrameworkPath);

This looks identical to the line above it. Perhaps one of these was meant to be __XPC_DYLD_FRAMEWORK_PATH to cover the same 4 as above?
Comment 57 youenn fablet 2017-02-21 20:07:18 PST
Comment on attachment 302353 [details]
Patch

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

> Source/WTF/wtf/Platform.h:1216
> +#if PLATFORM(COCOA) && ENABLE(WEB_RTC)

If we are going COCOA, then we should probably also link iOS/iOSsimulator webcore/webkit2 to libwebrtc.
That would be useful.

>> Tools/Scripts/webkitdirs.pm:2154
>> +    prependToEnvironmentVariableList("DYLD_LIBRARY_PATH", $dyldFrameworkPath);
> 
> This looks identical to the line above it. Perhaps one of these was meant to be __XPC_DYLD_FRAMEWORK_PATH to cover the same 4 as above?

Right!
Comment 58 Alex Christensen 2017-02-21 20:24:25 PST
Created attachment 302362 [details]
Patch
Comment 59 mitz 2017-02-21 20:32:42 PST
Comment on attachment 302362 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:2153
> +    prependToEnvironmentVariableList("__XPC_DYLD_FRAMEWORK_PATH", $dyldFrameworkPath);

What is this for?
Comment 60 youenn fablet 2017-02-21 20:33:01 PST
Patch has my name on it although Alex updated it so I am not sure whether I can r+.
But anyway, r=me, let's land it and move forward!
Comment 61 Alex Christensen 2017-02-21 21:08:52 PST
Created attachment 302364 [details]
Patch
Comment 62 mitz 2017-02-21 21:10:44 PST
Comment on attachment 302364 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:2153
> +    prependToEnvironmentVariableList("__XPC_DYLD_FRAMEWORK_PATH", $dyldFrameworkPath);

What is this for?
Comment 63 Alex Christensen 2017-02-21 22:13:04 PST
Created attachment 302368 [details]
Patch
Comment 64 Alexey Proskuryakov 2017-02-21 22:39:08 PST
Comment on attachment 302368 [details]
Patch

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

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:171
> +ENABLE_WEB_RTC_Production = ;
> +ENABLE_WEB_RTC_Debug = ENABLE_WEB_RTC;
> +ENABLE_WEB_RTC_Release = ENABLE_WEB_RTC;

This looks unusual. We don't differentiate between release and production builds when running tests, so how can we enable any tests when the feature is disabled in production builds?
Comment 65 Alex Christensen 2017-02-21 22:47:18 PST
Created attachment 302370 [details]
Patch
Comment 66 Alex Christensen 2017-02-21 23:04:04 PST
(In reply to comment #64)
> Comment on attachment 302368 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=302368&action=review
> 
> > Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:171
> > +ENABLE_WEB_RTC_Production = ;
> > +ENABLE_WEB_RTC_Debug = ENABLE_WEB_RTC;
> > +ENABLE_WEB_RTC_Release = ENABLE_WEB_RTC;
> 
> This looks unusual. We don't differentiate between release and production
> builds when running tests, so how can we enable any tests when the feature
> is disabled in production builds?
The point of this patch is to get some of the bots building webrtc.  For the purposes of this patch, we could skip all the webrtc tests in all release/production builds and I think this patch would still be a huge step in the right direction: enabling building on some bots to catch build regressions and running some tests (even if debug only) on some bots.  We can improve from there, but we're having enough trouble getting something building somewhere I think this is the least of our worries.
Comment 67 Alex Christensen 2017-02-21 23:04:47 PST
Created attachment 302372 [details]
Patch
Comment 68 Alex Christensen 2017-02-21 23:13:44 PST
Created attachment 302373 [details]
Patch
Comment 69 Alex Christensen 2017-02-22 00:02:02 PST
Comment on attachment 302373 [details]
Patch

http://trac.webkit.org/r212812
Comment 70 Alex Christensen 2017-02-22 00:52:16 PST
(In reply to comment #64)
> We don't differentiate between release and production
> builds when running tests, so how can we enable any tests when the feature
> is disabled in production builds?
Maybe we should skip all webrtc tests on all production/release builds until we get the production builds up and running.  I think it would be quite useful to run them on debug bots until then.
Comment 71 Build Bot 2017-02-22 01:17:27 PST
Comment on attachment 302373 [details]
Patch

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

New failing tests:
editing/spelling/spellcheck-async.html
Comment 72 Build Bot 2017-02-22 01:17:35 PST
Created attachment 302378 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 73 Alex Christensen 2017-02-22 13:35:18 PST
http://trac.webkit.org/r212849