Bug 186615 – Activate -Wexit-time-destructors -and Wglobal-constructors in libwebrtc

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Activate -Wexit-time-destructors -and Wglobal-constructors in libwebrtc

[youenn fablet]

Created attachment 342722 [details]
Patch

[youenn fablet]

Created attachment 342724 [details]
Patch

[youenn fablet]

Created attachment 342725 [details]
Patch

[Darin Adler]

Comment on attachment 342725 [details]
Patch

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

I don’t understand why there are changes to the project file in the patch.

Patch otherwise looks pretty good; I’m a little concerned that the "never destroyed" idiom is a little awkward to cut and paste 3 or more times, but probably fine.

> Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/logging.cc:128
> +    RTC_EXCLUSIVE_LOCKS_REQUIRED(logCriticalScope()) {

I don’t understand RTC_EXCLUSIVE_LOCKS_REQUIRED well enough to be sure this is correct.

> Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/logging.cc:148
> +    streamList();

Why is this needed? Seems like the first caller to streamList will do the right thing without this change.

[youenn fablet]

Created attachment 342742 [details]
Patch

[youenn fablet]

Created attachment 342748 [details]
Patch

[Igalia-pontevedra EWS]

Comment on attachment 342748 [details]
Patch

Attachment 342748 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.webkit.org/results/8183124

New failing tests:
http/tests/misc/cached-scripts.html

[Igalia-pontevedra EWS]

Created attachment 342754 [details]
Archive of layout-test-results from ltilve-gtk-wk2-ews for gtk-wk2

The attached test failures were seen while running run-webkit-tests on the gtk-wk2-ews.
Bot: ltilve-gtk-wk2-ews  Port: gtk-wk2  Platform: Linux-4.16.0-0.bpo.1-amd64-x86_64-with-debian-9.4

[youenn fablet]

(In reply to Igalia-pontevedra EWS from comment #7)
> Comment on attachment 342748 [details]
> Patch
> 
> Attachment 342748 [details] did not pass gtk-wk2-ews (gtk-wk2):
> Output: http://webkit-queues.webkit.org/results/8183124
> 
> New failing tests:
> http/tests/misc/cached-scripts.html

Nice to see GTK bots actually run tests.
Error is unrelated though since the patch is about WebRTC only.

[Darin Adler]

Comment on attachment 342748 [details]
Patch

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

> Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/logging.cc:148
> -  std::call_once(callLogCriticalScopeOnce,[] { logCriticalScope(); });
> +  std::call_once(callLogCriticalScopeOnce,[] {
> +    logCriticalScope();
> +  });

Seems like this formatting change is not needed.

[youenn fablet]

Created attachment 342769 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 342769 [details]
Patch for landing

Clearing flags on attachment: 342769

Committed r232858: <https://trac.webkit.org/changeset/232858>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41143599>