Bug 159269 – [Mac] Crash registering AVFoundation media engine

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

r199326 made it possible for HTMLMediaElement::originsInMediaCache, HTMLMediaElement::clearMediaCache and HTMLMediaElement::clearMediaCacheForOrigins to be called from multiple threads, but the media engine registration code isn't thread safe.

[Eric Carlson]

<rdar://problem/27017656>

[Eric Carlson]

Created attachment 282365 [details]
Proposed patch

[Build Bot]

Comment on attachment 282365 [details]
Proposed patch

Attachment 282365 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1596164

New failing tests:
animations/multiple-backgrounds.html

[Build Bot]

Created attachment 282371 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4

[Brent Fulgham]

Comment on attachment 282365 [details]
Proposed patch

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

Looks great! Would it be safer to initialize haveVector to false so that it's always in a known state?

> Source/WebCore/platform/graphics/MediaPlayer.cpp:189
> +    static bool haveVector;

Should this be initialized to false?

[Brent Fulgham]

(In reply to comment #3)
> Comment on attachment 282365 [details]
> Proposed patch
> 
> Attachment 282365 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: http://webkit-queues.webkit.org/results/1596164
> 
> New failing tests:
> animations/multiple-backgrounds.html

I don't believe that this test failure can be attributed to these AVFoundation Media Engine changes, especially when that test does not use a media element.

[Eric Carlson]

(In reply to comment #5)
> Comment on attachment 282365 [details]
> Proposed patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282365&action=review
> 
> Looks great! Would it be safer to initialize haveVector to false so that
> it's always in a known state?
> 
> > Source/WebCore/platform/graphics/MediaPlayer.cpp:189
> > +    static bool haveVector;
> 
> Should this be initialized to false?

It is a static, so the default value is false

[Eric Carlson]

Committed r202678: https://trac.webkit.org/r202678