RESOLVED FIXED 188745
Update libwebrtc up to 984f1a80c0
https://bugs.webkit.org/show_bug.cgi?id=188745
Summary Update libwebrtc up to 984f1a80c0
youenn fablet
Reported 2018-08-20 08:54:47 PDT
This revision is not stable but has support for all the new features we actually want to use.
Attachments
WIP (29.71 MB, patch)
2018-08-20 10:40 PDT, youenn fablet
no flags
WIP (29.43 MB, patch)
2018-08-20 10:57 PDT, youenn fablet
no flags
WIP (62.97 MB, patch)
2018-08-20 11:01 PDT, youenn fablet
no flags
WIP (62.36 MB, patch)
2018-08-20 16:41 PDT, youenn fablet
no flags
WIP (62.97 MB, patch)
2018-08-20 21:21 PDT, youenn fablet
no flags
WIP (62.60 MB, patch)
2018-08-21 13:26 PDT, youenn fablet
no flags
patch (56.02 MB, patch)
2018-08-21 14:35 PDT, youenn fablet
no flags
patch (56.02 MB, patch)
2018-08-21 17:06 PDT, youenn fablet
no flags
patch (56.04 MB, patch)
2018-08-21 22:50 PDT, youenn fablet
no flags
WIP GTK compilation patch (44.75 KB, patch)
2018-08-22 08:20 PDT, Alejandro G. Castro
no flags
patch (56.08 MB, patch)
2018-08-22 09:40 PDT, youenn fablet
eric.carlson: review+
reduced patch that contains only the changes after resyncing libwebrtc (844.46 KB, patch)
2018-08-22 13:35 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2018-08-20 10:40:36 PDT
youenn fablet
Comment 2 2018-08-20 10:57:38 PDT
youenn fablet
Comment 3 2018-08-20 11:01:57 PDT
youenn fablet
Comment 4 2018-08-20 16:41:59 PDT
EWS Watchlist
Comment 5 2018-08-20 18:22:05 PDT
Attachment 347568 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module> main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main WebKitPatch(os.path.abspath(__file__)).main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main result = command.check_arguments_and_execute(options, args, self) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute return self.execute(options, args, tool) or 0 File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute self._sequence.run_and_handle_errors(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors self._run(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run step(tool, options).run(state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 66, in run self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 135, in run_and_throw_if_fail exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 107, in _run_command_with_teed_output **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 481, in popen return subprocess.Popen(string_args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child raise child_exception OSError: [Errno 7] Argument list too long If any of these errors are false positives, please file a bug against check-webkit-style.
Radar WebKit Bug Importer
Comment 6 2018-08-20 18:22:59 PDT
youenn fablet
Comment 7 2018-08-20 21:21:03 PDT
youenn fablet
Comment 8 2018-08-21 13:26:11 PDT
youenn fablet
Comment 9 2018-08-21 14:35:07 PDT
EWS Watchlist
Comment 10 2018-08-21 15:20:51 PDT
Attachment 347698 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module> main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main WebKitPatch(os.path.abspath(__file__)).main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main result = command.check_arguments_and_execute(options, args, self) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute return self.execute(options, args, tool) or 0 File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute self._sequence.run_and_handle_errors(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors self._run(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run step(tool, options).run(state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 66, in run self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 135, in run_and_throw_if_fail exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 107, in _run_command_with_teed_output **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 481, in popen return subprocess.Popen(string_args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child raise child_exception OSError: [Errno 7] Argument list too long If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 11 2018-08-21 17:06:30 PDT
EWS Watchlist
Comment 12 2018-08-21 17:52:01 PDT
Attachment 347735 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module> main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main WebKitPatch(os.path.abspath(__file__)).main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main result = command.check_arguments_and_execute(options, args, self) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute return self.execute(options, args, tool) or 0 File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute self._sequence.run_and_handle_errors(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors self._run(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run step(tool, options).run(state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 66, in run self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 135, in run_and_throw_if_fail exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 107, in _run_command_with_teed_output **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 481, in popen return subprocess.Popen(string_args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child raise child_exception OSError: [Errno 7] Argument list too long If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 13 2018-08-21 22:50:26 PDT
EWS Watchlist
Comment 14 2018-08-21 23:38:06 PDT
Attachment 347769 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module> main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main WebKitPatch(os.path.abspath(__file__)).main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main result = command.check_arguments_and_execute(options, args, self) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute return self.execute(options, args, tool) or 0 File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute self._sequence.run_and_handle_errors(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors self._run(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run step(tool, options).run(state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 66, in run self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 135, in run_and_throw_if_fail exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 107, in _run_command_with_teed_output **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 481, in popen return subprocess.Popen(string_args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child raise child_exception OSError: [Errno 7] Argument list too long If any of these errors are false positives, please file a bug against check-webkit-style.
Alejandro G. Castro
Comment 15 2018-08-22 08:20:58 PDT
Created attachment 347795 [details] WIP GTK compilation patch I made the compilation work for GTK with this patch. It applies over the 347769 patch, I guess we can merge them.
youenn fablet
Comment 16 2018-08-22 08:59:49 PDT
(In reply to Alejandro G. Castro from comment #15) > Created attachment 347795 [details] > WIP GTK compilation patch > > I made the compilation work for GTK with this patch. It applies over the > 347769 patch, I guess we can merge them. Nice!
youenn fablet
Comment 17 2018-08-22 09:02:00 PDT
Comment on attachment 347795 [details] WIP GTK compilation patch View in context: https://bugs.webkit.org/attachment.cgi?id=347795&action=review > Source/WTF/wtf/DisallowCType.h:43 > +#if !(OS(DARWIN) && PLATFORM(GTK)) && !PLATFORM(WPE) && !PLATFORM(GTK) && !defined(_LIBCPP_VERSION) && defined(__GLIBC__) Is this change needed? PLATFORM(GTK) appears twice also, I guess it could be simplified.
youenn fablet
Comment 18 2018-08-22 09:40:00 PDT
youenn fablet
Comment 19 2018-08-22 09:41:45 PDT
(In reply to youenn fablet from comment #18) > Created attachment 347808 [details] > patch Merged GTK changes, let's see what will happen...
EWS Watchlist
Comment 20 2018-08-22 10:27:40 PDT
Attachment 347808 [details] did not pass style-queue: Traceback (most recent call last): File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module> main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main WebKitPatch(os.path.abspath(__file__)).main() File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main result = command.check_arguments_and_execute(options, args, self) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute return self.execute(options, args, tool) or 0 File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 55, in execute self._sequence.run_and_handle_errors(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors self._run(tool, options, state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run step(tool, options).run(state) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 66, in run self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 135, in run_and_throw_if_fail exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 107, in _run_command_with_teed_output **kwargs) File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 481, in popen return subprocess.Popen(string_args, **kwargs) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__ errread, errwrite) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child raise child_exception OSError: [Errno 7] Argument list too long If any of these errors are false positives, please file a bug against check-webkit-style.
youenn fablet
Comment 21 2018-08-22 13:35:46 PDT
Created attachment 347839 [details] reduced patch that contains only the changes after resyncing libwebrtc Might help doing the review
Alejandro G. Castro
Comment 22 2018-08-23 01:36:32 PDT
Comment on attachment 347839 [details] reduced patch that contains only the changes after resyncing libwebrtc View in context: https://bugs.webkit.org/attachment.cgi?id=347839&action=review > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:33 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Is this required? We should use something that can be used for gcc or control it is just clang using it, it causes a ton of warnings in the gcc compilation. I wonder if it is cleaner for us to modify the include in the library. > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:37 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/RealtimeIncomingAudioSource.h:39 > + > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/RealtimeIncomingVideoSource.h:39 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/RealtimeOutgoingAudioSource.h:39 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/RealtimeOutgoingVideoSource.h:38 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCAudioModule.h:33 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:35 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:39 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:38 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/platform/mediastream/mac/RealtimeOutgoingVideoSourceCocoa.cpp:34 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto > Source/WebCore/testing/MockLibWebRTCPeerConnection.h:32 > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored "-Wunused-parameter" Ditto
Alejandro G. Castro
Comment 23 2018-08-23 02:09:20 PDT
(In reply to youenn fablet from comment #17) > Comment on attachment 347795 [details] > WIP GTK compilation patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=347795&action=review > > > Source/WTF/wtf/DisallowCType.h:43 > > +#if !(OS(DARWIN) && PLATFORM(GTK)) && !PLATFORM(WPE) && !PLATFORM(GTK) && !defined(_LIBCPP_VERSION) && defined(__GLIBC__) > > Is this change needed? > PLATFORM(GTK) appears twice also, I guess it could be simplified. Unfortunately it is needed because stringutils.h inside libwebrtc uses tolower. I wonder why it does not happen for mac ports ... I forgot to review the change I used initially to test the compilation. I think we can add a comment and a more accurate condition like this: diff --git a/Source/WTF/wtf/DisallowCType.h b/Source/WTF/wtf/DisallowCType.h index 30544e729fd..de71b79155d 100644 --- a/Source/WTF/wtf/DisallowCType.h +++ b/Source/WTF/wtf/DisallowCType.h @@ -39,8 +39,9 @@ // Also generates errors on wx on Windows, presumably because these functions // are used from wx headers. On GTK+ for Mac many GTK+ files include <libintl.h> // or <glib/gi18n-lib.h>, which in turn include <xlocale/_ctype.h> which uses -// isacii(). -#if !(OS(DARWIN) && PLATFORM(GTK)) && !defined(_LIBCPP_VERSION) && defined(__GLIBC__) +// isacii(). This breaks the compilation for libwebrtc in GTK and WPE, it +// includes a stringutils.h header that uses tolower. +#if !(OS(DARWIN) && PLATFORM(GTK)) && !(USE(LIBWEBRTC) && (PLATFORM(WPE) || PLATFORM(GTK))) && !defined(_LIBCPP_VERSION) && defined(__GLIBC__) #include <ctype.h> The other option is to remove that function in libwebrtc header stringutils.h, it is not used. Both options are ugly, maybe we can find a different option.
Alejandro G. Castro
Comment 24 2018-08-23 02:29:43 PDT
(In reply to Alejandro G. Castro from comment #23) > > [...] > > The other option is to remove that function in libwebrtc header > stringutils.h, it is not used. > > Both options are ugly, maybe we can find a different option. With removing I meant something like this: diff --git a/Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/stringutils.h b/Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/stringutils.h index d92ba0260f9..883b0584e8e 100644 --- a/Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/stringutils.h +++ b/Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/stringutils.h @@ -50,6 +50,7 @@ bool string_match(const char* target, const char* pattern); } // namespace rtc +#if !defined(WEBRTC_WEBKIT_BUILD) /////////////////////////////////////////////////////////////////////////////// // Rename a few common string functions so they are consistent across platforms. // tolowercase is like tolower, but not compatible with end-of-file value @@ -61,6 +62,7 @@ bool string_match(const char* target, const char* pattern); inline char tolowercase(char c) { return static_cast<char>(tolower(c)); } +#endif #if defined(WEBRTC_WIN)
youenn fablet
Comment 25 2018-08-23 08:36:19 PDT
> > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:33 > > +#pragma clang diagnostic push > > +#pragma clang diagnostic ignored "-Wunused-parameter" > > Is this required? We should use something that can be used for gcc or > control it is just clang using it, it causes a ton of warnings in the gcc > compilation. I wonder if it is cleaner for us to modify the include in the > library. WebCore on Mac/iOS ports is using -Werror and -Wunused-parameter. Previously, we were updating the libwebrtc source files but this is tedious and creates a lot of conflicts every time we update libwebrtc. The ideal solution would be to progressively migrate code including these headers in libwebrtc and create new libwebrtc headers matching WebCore/WebKit constraints. I would like to land it as is and we could improve with a follow-up patch to fix GCC warnings, maybe adding macros to support easily CLANG and GCC. (In reply to Alejandro G. Castro from comment #24) > (In reply to Alejandro G. Castro from comment #23) > > > > [...] > > > > The other option is to remove that function in libwebrtc header > > stringutils.h, it is not used. The ideal solution would be to not include stringutils.h in WebCore/WebKit code. Doing a grep, tolowercase does not seem used at all except in unit tests so it might be the best short term solution. I'll do that.
youenn fablet
Comment 26 2018-08-23 10:58:34 PDT
Alejandro G. Castro
Comment 27 2018-08-24 01:53:43 PDT
(In reply to youenn fablet from comment #25) > > > Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:33 > > > +#pragma clang diagnostic push > > > +#pragma clang diagnostic ignored "-Wunused-parameter" > > > > Is this required? We should use something that can be used for gcc or > > control it is just clang using it, it causes a ton of warnings in the gcc > > compilation. I wonder if it is cleaner for us to modify the include in the > > library. > > WebCore on Mac/iOS ports is using -Werror and -Wunused-parameter. > Previously, we were updating the libwebrtc source files but this is tedious > and creates a lot of conflicts every time we update libwebrtc. > > The ideal solution would be to progressively migrate code including these > headers in libwebrtc and create new libwebrtc headers matching > WebCore/WebKit constraints. > Yep, it seems the solution also for the stringutils.h issue, which we are not including directly it is the main peerconnectioninterface.h in the api that causes the include. > I would like to land it as is and we could improve with a follow-up patch to > fix GCC warnings, maybe adding macros to support easily CLANG and GCC. > Ok, I'll look into this. > (In reply to Alejandro G. Castro from comment #24) > > (In reply to Alejandro G. Castro from comment #23) > > > > > > [...] > > > > > > The other option is to remove that function in libwebrtc header > > > stringutils.h, it is not used. > > The ideal solution would be to not include stringutils.h in WebCore/WebKit > code. > > Doing a grep, tolowercase does not seem used at all except in unit tests so > it might be the best short term solution. > I'll do that. The problem is that we are not including it directly it is the main peerconnectioninterface.h in the api that causes the problem.
Truitt Savell
Comment 28 2018-09-05 09:55:33 PDT
Looks like https://trac.webkit.org/changeset/235230/webkit Has caused test webrtc/video-rotation.html to become flaky the failure is easily reproducible with command: run-webkit-tests webrtc/video-rotation.html --iterations 500 -f When this command is run using r235229 no failures occur, when using r235230 the failure is intermittent. History: http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=webrtc%2Fvideo-rotation.html Diff: --- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/webrtc/video-rotation-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/webrtc/video-rotation-actual.txt @@ -2,5 +2,5 @@ PASS Setting video exchange PASS Track is enabled, video should not be black -PASS Track is enabled and rotated, video should not be black and should change size +FAIL Track is enabled and rotated, video should not be black and should change size promise_test: Unhandled rejection with value: "waitForVideoSize timed out, expected 240x320 but got 320x240"
youenn fablet
Comment 29 2018-09-07 12:47:32 PDT
(In reply to Truitt Savell from comment #28) > Looks like https://trac.webkit.org/changeset/235230/webkit > > Has caused test webrtc/video-rotation.html to become flaky > > the failure is easily reproducible with command: > run-webkit-tests webrtc/video-rotation.html --iterations 500 -f > > When this command is run using r235229 no failures occur, when using r235230 > the failure is intermittent. > > History: > http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=webrtc%2Fvideo-rotation.html > > Diff: > --- > /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/ > webrtc/video-rotation-expected.txt > +++ > /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/ > webrtc/video-rotation-actual.txt > @@ -2,5 +2,5 @@ > > PASS Setting video exchange > PASS Track is enabled, video should not be black > -PASS Track is enabled and rotated, video should not be black and should > change size > +FAIL Track is enabled and rotated, video should not be black and should > change size promise_test: Unhandled rejection with value: "waitForVideoSize > timed out, expected 240x320 but got 320x240" https://bugs.webkit.org/show_bug.cgi?id=189427 should fix this.
Note You need to log in before you can comment on or make changes to this bug.