RESOLVED FIXED 170954
Add NDEBUG and CodeStripping to libwebrtc build system
https://bugs.webkit.org/show_bug.cgi?id=170954
Summary Add NDEBUG and CodeStripping to libwebrtc build system
youenn fablet
Reported 2017-04-18 10:31:32 PDT
Attachments
Patch (1.61 KB, patch)
2017-04-18 10:32 PDT, youenn fablet
no flags
Patch (1.68 KB, patch)
2017-04-18 10:38 PDT, youenn fablet
no flags
Archive of layout-test-results from ews101 for mac-elcapitan (971.76 KB, application/zip)
2017-04-18 11:55 PDT, Build Bot
no flags
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (1.30 MB, application/zip)
2017-04-18 12:16 PDT, Build Bot
no flags
Patch (15.09 KB, patch)
2017-04-18 12:58 PDT, youenn fablet
no flags
Patch (13.37 KB, patch)
2017-04-18 13:07 PDT, youenn fablet
no flags
Archive of layout-test-results from ews114 for mac-elcapitan (1.67 MB, application/zip)
2017-04-18 14:57 PDT, Build Bot
no flags
Patch (11.55 KB, patch)
2017-04-18 16:47 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2017-04-18 10:32:22 PDT
youenn fablet
Comment 2 2017-04-18 10:38:59 PDT
Build Bot
Comment 3 2017-04-18 11:55:08 PDT
Comment on attachment 307394 [details] Patch Attachment 307394 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3558439 New failing tests: webrtc/captureCanvas-webrtc.html
Build Bot
Comment 4 2017-04-18 11:55:09 PDT
Created attachment 307400 [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
Build Bot
Comment 5 2017-04-18 12:16:13 PDT
Comment on attachment 307394 [details] Patch Attachment 307394 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/3558455 New failing tests: webrtc/release-after-getting-track.html webrtc/ephemeral-certificates-and-cnames.html webrtc/video-addTrack.html webrtc/video-with-receiver.html webrtc/video-disabled-black.html webrtc/video-remote-mute.html webrtc/libwebrtc/descriptionGetters.html webrtc/captureCanvas-webrtc.html webrtc/video-with-data-channel.html
Build Bot
Comment 6 2017-04-18 12:16:14 PDT
Created attachment 307401 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
youenn fablet
Comment 7 2017-04-18 12:58:04 PDT
youenn fablet
Comment 8 2017-04-18 13:01:24 PDT
Crashes are probably due to libwebrtc not defining NDEBUG while WebCore does. Then classes from libwebrtc included/instantiated within WebCore no longer have the same definition, size...
youenn fablet
Comment 9 2017-04-18 13:07:18 PDT
Build Bot
Comment 10 2017-04-18 14:57:43 PDT
Comment on attachment 307405 [details] Patch Attachment 307405 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3559120 New failing tests: http/tests/inspector/network/resource-sizes-network.html
Build Bot
Comment 11 2017-04-18 14:57:44 PDT
Created attachment 307420 [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
Alex Christensen
Comment 12 2017-04-18 15:05:58 PDT
Comment on attachment 307405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307405&action=review I like this. Let's split it up and do it cleanly. > Source/WebCore/ChangeLog:10 > + * platform/mediastream/libwebrtc/VideoToolBoxEncoderFactory.cpp: This change is not in this patch. > Source/ThirdParty/libwebrtc/Configurations/Base.xcconfig:75 > +DEAD_CODE_STRIPPING_normal = YES; This cuts almost 1MB of code from release libwebrtc.dylib! > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc:40 > - return new H264VideoToolboxEncoder(codec); > + return CreateSupportedVideoEncoder(codec); This change is unrelated to the rest of the changes in this patch. The other changes should be in a separate patch titled something like "Strip dead code from release libwebrtc.dylib"
youenn fablet
Comment 13 2017-04-18 15:58:25 PDT
(In reply to Alex Christensen from comment #12) > Comment on attachment 307405 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307405&action=review > > I like this. Let's split it up and do it cleanly. > > > Source/WebCore/ChangeLog:10 > > + * platform/mediastream/libwebrtc/VideoToolBoxEncoderFactory.cpp: > > This change is not in this patch. > > > Source/ThirdParty/libwebrtc/Configurations/Base.xcconfig:75 > > +DEAD_CODE_STRIPPING_normal = YES; > > This cuts almost 1MB of code from release libwebrtc.dylib! > > > Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/videotoolboxvideocodecfactory.cc:40 > > - return new H264VideoToolboxEncoder(codec); > > + return CreateSupportedVideoEncoder(codec); > > This change is unrelated to the rest of the changes in this patch. The > other changes should be in a separate patch titled something like "Strip > dead code from release libwebrtc.dylib" Well, this change was triggering crashes. The fact that it no longer crashes on EWS show that we have a fix there. OK to split it now that the fix is validated.
youenn fablet
Comment 14 2017-04-18 16:47:36 PDT
WebKit Commit Bot
Comment 15 2017-04-18 18:51:03 PDT
Comment on attachment 307435 [details] Patch Clearing flags on attachment: 307435 Committed r215494: <http://trac.webkit.org/changeset/215494>
WebKit Commit Bot
Comment 16 2017-04-18 18:51:05 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.