Bug 188745 - Update libwebrtc up to 984f1a80c0
Summary: Update libwebrtc up to 984f1a80c0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-20 08:54 PDT by youenn fablet
Modified: 2018-09-07 12:47 PDT (History)
8 users (show)

See Also:


Attachments
WIP (29.71 MB, patch)
2018-08-20 10:40 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP (29.43 MB, patch)
2018-08-20 10:57 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP (62.97 MB, patch)
2018-08-20 11:01 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP (62.36 MB, patch)
2018-08-20 16:41 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP (62.97 MB, patch)
2018-08-20 21:21 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP (62.60 MB, patch)
2018-08-21 13:26 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
patch (56.02 MB, patch)
2018-08-21 14:35 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
patch (56.02 MB, patch)
2018-08-21 17:06 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
patch (56.04 MB, patch)
2018-08-21 22:50 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
WIP GTK compilation patch (44.75 KB, patch)
2018-08-22 08:20 PDT, Alejandro G. Castro
no flags Details | Formatted Diff | Diff
patch (56.08 MB, patch)
2018-08-22 09:40 PDT, youenn fablet
eric.carlson: review+
Details | Formatted Diff | Diff
reduced patch that contains only the changes after resyncing libwebrtc (844.46 KB, patch)
2018-08-22 13:35 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 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.
Comment 1 youenn fablet 2018-08-20 10:40:36 PDT
Created attachment 347508 [details]
WIP
Comment 2 youenn fablet 2018-08-20 10:57:38 PDT
Created attachment 347510 [details]
WIP
Comment 3 youenn fablet 2018-08-20 11:01:57 PDT
Created attachment 347512 [details]
WIP
Comment 4 youenn fablet 2018-08-20 16:41:59 PDT
Created attachment 347568 [details]
WIP
Comment 5 EWS Watchlist 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.
Comment 6 Radar WebKit Bug Importer 2018-08-20 18:22:59 PDT
<rdar://problem/43539177>
Comment 7 youenn fablet 2018-08-20 21:21:03 PDT
Created attachment 347607 [details]
WIP
Comment 8 youenn fablet 2018-08-21 13:26:11 PDT
Created attachment 347679 [details]
WIP
Comment 9 youenn fablet 2018-08-21 14:35:07 PDT
Created attachment 347698 [details]
patch
Comment 10 EWS Watchlist 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.
Comment 11 youenn fablet 2018-08-21 17:06:30 PDT
Created attachment 347735 [details]
patch
Comment 12 EWS Watchlist 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.
Comment 13 youenn fablet 2018-08-21 22:50:26 PDT
Created attachment 347769 [details]
patch
Comment 14 EWS Watchlist 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.
Comment 15 Alejandro G. Castro 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.
Comment 16 youenn fablet 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!
Comment 17 youenn fablet 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.
Comment 18 youenn fablet 2018-08-22 09:40:00 PDT
Created attachment 347808 [details]
patch
Comment 19 youenn fablet 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...
Comment 20 EWS Watchlist 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.
Comment 21 youenn fablet 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
Comment 22 Alejandro G. Castro 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
Comment 23 Alejandro G. Castro 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.
Comment 24 Alejandro G. Castro 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)
Comment 25 youenn fablet 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.
Comment 26 youenn fablet 2018-08-23 10:58:34 PDT
https://trac.webkit.org/r235230
Comment 27 Alejandro G. Castro 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.
Comment 28 Truitt Savell 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"
Comment 29 youenn fablet 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.