Eliminate static initializers in libwebrtc.dylib
<rdar://problem/41054874>
Created attachment 342615 [details] Patch
Created attachment 342619 [details] Patch
Created attachment 342620 [details] Patch
Comment on attachment 342620 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342620&action=review > Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/logging.cc:110 > + static CriticalSection g_log_crit; It would be nice to eliminate not just static constructors, but also static destructors. Can we do that? Would need something like NeverDestroyed here.
Comment on attachment 342620 [details] Patch Should also turn on the compiler option to make it an error if we have any, so we notice if someone accidentally adds one in the future.
Created attachment 342678 [details] Patch
(In reply to Darin Adler from comment #5) > Comment on attachment 342620 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=342620&action=review > > > Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/logging.cc:110 > > + static CriticalSection g_log_crit; > > It would be nice to eliminate not just static constructors, but also static > destructors. Can we do that? Would need something like NeverDestroyed here. Done > Should also turn on the compiler option to make it an error if we have any, > so we notice if someone accidentally adds one in the future. Let's do that as a follow-up
(In reply to Darin Adler from comment #6) > Should also turn on the compiler option to make it an error if we have any, > so we notice if someone accidentally adds one in the future. The options are -Wexit-time-destructors -and Wglobal-constructors. These are both already specified in ThirdParty/libwebrtc/Configurations/Base.xcconfig, but disabled by libwebrtc.xcconfig, which overrides WARNING_CFLAGS with another warning setting without using $(inherited). It seems like that mistake led to us having these without noticing. It seems that other .xcconfig files in the libwebrtc directories are making the same mistake. Also, there are lots more warnings enabled in WebCore's Base.xconfig and it would be nice to enable more here.
> The options are -Wexit-time-destructors -and Wglobal-constructors. OK > Also, there are lots more warnings enabled in WebCore's > Base.xconfig and it would be nice to enable more here. Sure. We want to stay as close as possible to upstream libwebrtc to limit the cost of bumping revision. If we do that cleaning work once, we need to ensure we will not do that again and again.
Comment on attachment 342678 [details] Patch Clearing flags on attachment: 342678 Committed r232827: <https://trac.webkit.org/changeset/232827>
All reviewed patches have been landed. Closing bug.
There are still static initializers for: LogMessage::streams_ in logging.cc NonlinearBeamformer::kHalfBeamWidthRadians in nonlinear_beamformer.cc PI in virtualsocketserver.cc
I did some follow-up work yesterday at https://bugs.webkit.org/show_bug.cgi?id=186615 to enable these flags. It seems this current set of changes is breaking GTK, https://bugs.webkit.org/show_bug.cgi?id=186619
Comment on attachment 342678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342678&action=review > Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/flags.h:158 > + c_type FLAG_##name = (default); \ I think that "static" is missing here.
(In reply to youenn fablet from comment #14) > It seems this current set of changes is breaking GTK, > https://bugs.webkit.org/show_bug.cgi?id=186619 Maybe because of the "static" thing I commented on above?
(In reply to Darin Adler from comment #16) > (In reply to youenn fablet from comment #14) > > It seems this current set of changes is breaking GTK, > > https://bugs.webkit.org/show_bug.cgi?id=186619 > > Maybe because of the "static" thing I commented on above? Makes sense, I'll fix it asap.
Reopening to attach new patch.
Created attachment 342738 [details] Fix memory error issue
Comment on attachment 342738 [details] Fix memory error issue Clearing flags on attachment: 342738 Committed r232846: <https://trac.webkit.org/changeset/232846>