Bug 175472 – WebRTC video does not resume receiving when switching back to Safari 11 on iOS

• Component: WebRTC
• Status: RESOLVED FIXED

[Andrew Morris]

1. Join the same room on https://safari.opentokrtc.com on two devices, one that is iOS Safari, the other doesn't matter
2. Wait for both sides to send and receive video
3. On iOS, switch apps so Safari is in the background
4. Switch back to Safari

Result: Remote video is frozen or black
Expected: Remote video should resume flowing

I have tried calling .play() on the video to no avail. This also happens P2P, and I suspect that it is unrelated to the OpenTok stack, but I can produce a minimal repro app if needed. The tricky bit in setting this up though is that you must use multiple devices. I have done a similar test at https://webrtc.github.io/samples/src/content/peerconnection/pc1/ but video resumes flowing correctly, and I suspect this is because the video being sent is being affected by the same device.

Tested on iPhone with iOS beta 4 and iPad with iOS beta 5.

[Radar WebKit Bug Importer]

<rdar://problem/33860863>

[youenn fablet]

I tried reproducing this issue and was in most cases unable to do so.
I was able to reproduce it once but reloading the other page (not the iOS device one) fixed the issue so this probably does not count.

Some sysdiagnose might be useful since the issue might be below WebKit.

[Andrew Morris]

@youenn Hmm. Are you using an internal build that's newer than beta 5? If not that would be strange because I was getting this reliably across multiple devices.

[Andrew Morris]

Created attachment 318209 [details]
sysdiagnose

[Andrew Morris]

Sysdiagnose attached

2017-08-16 UTC+10 AEST
10:54:10 - switch apps
10:54:15 - switch back to safari; remote video frozen
10:54:18 - siri activated
10:54:20 - siri dismissed; remote video black (local video did not go black this time)

[youenn fablet]

It seems that the decoder has some issues and mediaserverd is returning kVTVideoDecoderMalfunctionErr as soon as WebKit is backgrounded.
And it does not seem to be able to recover.

In opentokrtc SFU, how often are key frames being sent by default?
Is this something you would be able to play to check whether that would have an impact?

[Andrew Morris]

@youenn. I believe it's once per minute. I'll inquire about experimenting with this for you. However, please note this also occurred for P2P.

[youenn fablet]

(In reply to Andrew Morris from comment #7)
> @youenn. I believe it's once per minute. I'll inquire about experimenting
> with this for you. However, please note this also occurred for P2P.

Before doing that, can you do a quick test: try to restart playing the black video by calling play() when coming back from Siri.
It might be that we are not restarting properly the autoplayed video that got paused due to backgrounding.

In that case, a temporary workaround would be to catch page visibility events.

[Andrew Morris]

(In reply to youenn fablet from comment #8)
> Before doing that, can you do a quick test: try to restart playing the black
> video by calling play() when coming back from Siri.
> It might be that we are not restarting properly the autoplayed video that
> got paused due to backgrounding.
> 
> In that case, a temporary workaround would be to catch page visibility
> events.

Gave this a go. .play() claims to succeed, but does not actually play the video or result in any visual change.

[Lucas Forschler]

The content of attachment 318209 [details] has been deleted for the following reason:

user request

[youenn fablet]

Just tried and backgrounding does not make it.
Using Siri, the audio stops playing and when coming back to the page, the video is not always playing either.

Doing a video.play seems to trigger back the audio in the tests I did.
Same for audio, audio.play seems to relaunch it.

[Andrew Morris]

Are you on a different build? Because it's 100% reproducible for me. On betas 4, 5, and 6 too, on different devices. I'm uploading a screen recording for you to compare, maybe there's something about what I'm doing that I didn't think to specify.

[Andrew Morris]

Created attachment 318310 [details]
screen-recording-of-repro

[youenn fablet]

Hum, I was able to reproduce it once and video.play() is not working also.
That is different from what I saw this morning with Siri interruption...

Clicking twice on the camera icon at the top-left is making the video going back though. What does the web site in this case?

[Andrew Morris]

Created attachment 318314 [details]
switch-camera-icon

[Andrew Morris]

@youenn: Do you mean this camera icon?
https://bugs.webkit.org/attachment.cgi?id=318314

That one is supposed to switch the local video between the available cameras.

Regarding Siri, that's an additional issue that occurs after getting the frozen remote video. Separate ticket for that here:
https://bugs.webkit.org/show_bug.cgi?id=175473

[youenn fablet]

The camera icon like https://d30y9cdsu7xlg0.cloudfront.net/png/1998-200.png on the top left corner of the remote video

[Andrew Morris]

Created attachment 318316 [details]
video-enabled-icon

[Andrew Morris]

@youenn: Oh actually, I know the one you mean, it only shows over the video if you have focus. This one right?

https://bugs.webkit.org/attachment.cgi?id=318316

Anyway, I think I know what is happening. When switching to the background, decoding stops. That means when you resume, a keyframe is needed. Disabling and enabling the video using the icon above triggers a keyframe and unfreezes the video. If a new participant joins, a keyframe is generated for the new user, and incidentally is sent to everyone, which also unfreezes the video.

I think Safari should send a keyframe request when it resumes decoding, and this will fix it. Wdyt?

[youenn fablet]

(In reply to Andrew Morris from comment #19)
> @youenn: Oh actually, I know the one you mean, it only shows over the video
> if you have focus. This one right?
> 
> https://bugs.webkit.org/attachment.cgi?id=318316
> 
> Anyway, I think I know what is happening. When switching to the background,
> decoding stops. That means when you resume, a keyframe is needed. Disabling
> and enabling the video using the icon above triggers a keyframe and
> unfreezes the video. If a new participant joins, a keyframe is generated for
> the new user, and incidentally is sent to everyone, which also unfreezes the
> video.

This is my hypothesis too.
But, then video should be coming back after 1 minute too, no?

> I think Safari should send a keyframe request when it resumes decoding, and
> this will fix it. Wdyt?

Yes, I think we need to fix that.

[Andrew Morris]

(In reply to youenn fablet from comment #20)
> But, then video should be coming back after 1 minute too, no?

Yeah, I don't understand that either.

Also, I found a way to recover from the black videos when activating Siri - video.srcObject = video.srcObject. Seems to work reliably.

[Andrew Morris]

Could be something about a different code path when a requested keyframe comes through vs when a regular keyframe comes through?

I have actually seen it unfreeze spontaneously a few times. But I've also tried to reproduce this by waiting 1min+ and failed.

[youenn fablet]

Just tried to reproduce this case and was not able to do so with safari.opentokrtc.com.
Andrew, did anything changed there?

[youenn fablet]

Created attachment 321469 [details]
Patch

[Build Bot]

Attachment 321469 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.mm:102:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:99:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 20 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 321488 [details]
Patch

[Build Bot]

Attachment 321488 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:99:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.mm:102:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrew Morris]

@youenn Yep we have a workaround deployed where it disables & re-enables the video on Safari when we get visibility back.

[youenn fablet]

Created attachment 321549 [details]
Patch

[Build Bot]

Attachment 321549 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:99:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.mm:102:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Created attachment 321607 [details]
Patch

[Build Bot]

Attachment 321607 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.mm:102:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:72:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:99:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 321607 [details]
Patch

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

Can’t tell if the SetActive that you are adding to libwebrtc is only for WEBRTC_WEBKIT_BUILD or if it’s supposed to be unconditionally added.

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:118
> +    ASSERT(factoryAndThreads.audioDeviceModule);

This assertion seems a little silly. The line above assigns the result of make_unique to this. It’s not ambiguous.

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:134
> +    auto* decoderFactory = new VideoToolboxVideoDecoderFactory();
> +    auto* encoderFactory = new VideoToolboxVideoEncoderFactory();

No need for the () here.

Strange to put these into raw pointers like this. I would have used unique_ptr. Maybe this is a single global object and we just let these leak?

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:147
> -    if (!staticFactoryAndThreads().factory)
> +    if (!staticFactoryAndThreads().audioDeviceModule)
>          initializePeerConnectionFactoryAndThreads();

Code less logical now than it was before. Wish this could be written in a more straightforward way. Doesn’t seem like checking one specific field is a good way to guard this call. I think we should consider just making staticFactoryAndThreads() take care of initialization itself instead of doing it explicitly.

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:160
> -    if (!factoryAndThreads.factory) {
> +    if (!factoryAndThreads.audioDeviceModule) {
>          staticFactoryAndThreads().networkThreadWithSocketServer = true;
>          initializePeerConnectionFactoryAndThreads();
>      }
>      ASSERT(staticFactoryAndThreads().networkThreadWithSocketServer);

This construction is a bit inelegant and strange. Why go out of our way to set networkThreadWithSocketServer only once? That creates a failure mode if someone calls setPeerConnectionFactory first and then calls createPeerConnection. I would like to see the assertions written in a more straightforward manner. They should focus on the actual risk, the actual bad way to call the functions. Is there some problem with a particular combination of calls, or could we just remove the assertion?

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:83
> +    rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> m_factory;
> +    VideoToolboxVideoDecoderFactory* m_decoderFactory  { nullptr };
> +    VideoToolboxVideoEncoderFactory* m_encoderFactory { nullptr };

Seems strange to use a RefPtr for the factory, but then raw pointers for the decoder and encoder factories. I suggest considering unique_ptr instead or raw pointers for all maybe?

Extra space after m_decoderFactory here.

> Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:39
> +    for (webrtc::H264VideoToolboxDecoder& decoder : m_decoders)

Too bad reference_wrapper forces you to write out the type here to avoid having to write () or get(). I would probably have used auto& and written the () out.

> Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:47
> +    auto* decoder = webrtc::VideoToolboxVideoDecoderFactory::CreateVideoDecoder(type);
> +    if (decoder)
> +        m_decoders.append(static_cast<webrtc::H264VideoToolboxDecoder&>(*decoder));

What makes this cast safe? Seems like a design mistake that it returns a generic type by somehow we know the exact class. Is there something we can do to correct this?

> Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:56
> +    m_decoders.removeFirstMatching([&] (const auto& item) {
> +        return &item.get() == decoder;
> +    });

Why use a Vector if we need the remove operation? I suggest a HashSet instead.

> Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxEncoderFactory.cpp:41
> +    for (H264VideoToolboxEncoder& encoder : m_encoders)

Ditto.

> Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxEncoderFactory.h:48
> +    Vector<std::reference_wrapper<H264VideoToolboxEncoder>> m_encoders;

Ditto.

[youenn fablet]

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321607&action=review
> 
> Can’t tell if the SetActive that you are adding to libwebrtc is only for
> WEBRTC_WEBKIT_BUILD or if it’s supposed to be unconditionally added.

Ideally, libwebrtc should not require its decoder/encoder to have a UIKit dependency and SetActive would be used for all.
For now it is indeed better to keep it to WEBRTC_WEBKIT_BUILD only.

> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:118
> > +    ASSERT(factoryAndThreads.audioDeviceModule);
> 
> This assertion seems a little silly. The line above assigns the result of
> make_unique to this. It’s not ambiguous.

I'll remove it.


> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:134
> > +    auto* decoderFactory = new VideoToolboxVideoDecoderFactory();
> > +    auto* encoderFactory = new VideoToolboxVideoEncoderFactory();
> 
> No need for the () here.
OK

> Strange to put these into raw pointers like this. I would have used
> unique_ptr. Maybe this is a single global object and we just let these leak?

I will use unique_ptr and release them when passing them to the factory
That will clarify how the API is supposed to be used.
Ideally, the factory API would take unique_tr&& as input.

> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:147
> > -    if (!staticFactoryAndThreads().factory)
> > +    if (!staticFactoryAndThreads().audioDeviceModule)
> >          initializePeerConnectionFactoryAndThreads();
> 
> Code less logical now than it was before. Wish this could be written in a
> more straightforward way. Doesn’t seem like checking one specific field is a
> good way to guard this call. I think we should consider just making
> staticFactoryAndThreads() take care of initialization itself instead of
> doing it explicitly.


I'll try to refactor this.

> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:160
> > -    if (!factoryAndThreads.factory) {
> > +    if (!factoryAndThreads.audioDeviceModule) {
> >          staticFactoryAndThreads().networkThreadWithSocketServer = true;
> >          initializePeerConnectionFactoryAndThreads();
> >      }
> >      ASSERT(staticFactoryAndThreads().networkThreadWithSocketServer);
> 
> This construction is a bit inelegant and strange. Why go out of our way to
> set networkThreadWithSocketServer only once? That creates a failure mode if
> someone calls setPeerConnectionFactory first and then calls
> createPeerConnection. I would like to see the assertions written in a more
> straightforward manner. They should focus on the actual risk, the actual bad
> way to call the functions. Is there some problem with a particular
> combination of calls, or could we just remove the assertion?

We should really make networkThreadWithSocketServer a process global variable.
This would be best as a follow_up.

> > Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.h:83
> > +    rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> m_factory;
> > +    VideoToolboxVideoDecoderFactory* m_decoderFactory  { nullptr };
> > +    VideoToolboxVideoEncoderFactory* m_encoderFactory { nullptr };
> 
> Seems strange to use a RefPtr for the factory, but then raw pointers for the
> decoder and encoder factories. I suggest considering unique_ptr instead or
> raw pointers for all maybe?

The factory is keeping the raw pointers alive.

> Extra space after m_decoderFactory here.
> 
> > Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:39
> > +    for (webrtc::H264VideoToolboxDecoder& decoder : m_decoders)
> 
> Too bad reference_wrapper forces you to write out the type here to avoid
> having to write () or get(). I would probably have used auto& and written
> the () out.
> 
> > Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:47
> > +    auto* decoder = webrtc::VideoToolboxVideoDecoderFactory::CreateVideoDecoder(type);
> > +    if (decoder)
> > +        m_decoders.append(static_cast<webrtc::H264VideoToolboxDecoder&>(*decoder));
> 
> What makes this cast safe? Seems like a design mistake that it returns a
> generic type by somehow we know the exact class. Is there something we can
> do to correct this?

To fix that, we would need to be more invasive into libwebrtc code by adding a type-specific method to VideoToolBoxDecoderFactory.

> > Source/WebCore/platform/mediastream/libwebrtc/VideoToolBoxDecoderFactory.cpp:56
> > +    m_decoders.removeFirstMatching([&] (const auto& item) {
> > +        return &item.get() == decoder;
> > +    });
> 
> Why use a Vector if we need the remove operation? I suggest a HashSet
> instead.

OK

[youenn fablet]

Created attachment 321740 [details]
Patch

[youenn fablet]

Created attachment 321742 [details]
Patch

[Build Bot]

Attachment 321742 [details] did not pass style-queue:


ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:71:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:71:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/encoder.h:98:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.mm:102:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:46:  is_active is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/Framework/Classes/VideoToolbox/decoder.h:57:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 7 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

I'll do the Vector -> HashSet as a follow-up, it requires adding a DefaultHash for reference_wrapper

[WebKit Commit Bot]

Comment on attachment 321742 [details]
Patch

Rejecting attachment 321742 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 321742, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
:040000 040000 093903938a20571e7b7d4f513da310b393f68a03 172a1449945fc84a61d1db015da30b56a29cc36c M	WebKit.xcworkspace
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.webkit.org/results/4654987

[WebKit Commit Bot]

Comment on attachment 321742 [details]
Patch

Clearing flags on attachment: 321742

Committed r222478: <http://trac.webkit.org/changeset/222478>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jon Lee]

*** Bug 175473 has been marked as a duplicate of this bug. ***