HTMLMediaElement::clearMediaPlayer should acquire MediaElementAudioSourceNode::lock()

Source/core/html/HTMLMediaElement.cpp

Index: Source/core/html/HTMLMediaElement.cpp
diff --git a/Source/core/html/HTMLMediaElement.cpp b/Source/core/html/HTMLMediaElement.cpp
index 1ec05752e7c4a890ae99b8c952484f91a02a8b54..22cc1fb69490d2a9b4926b75c74430513d5fd409 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -3385,6 +3385,11 @@ void HTMLMediaElement::userCancelledLoad()
 
 void HTMLMediaElement::clearMediaPlayer(int flags)
 {
+#if ENABLE(WEB_AUDIO)
+    if (m_audioSourceNode)
+        m_audioSourceNode->lock();

[Ken Russell (switch to Gerrit), 2013/11/26 20:09:24]

The manual calls to lock() and unlock() are unfortunate. It would be much better
to use a Locker class like in wtf/Locker.h. Unfortunately, that class deals only
with references, not with pointers that might be NULL.

+#endif
+
     removeAllInbandTracks();
 
     closeMediaSource();
@@ -3394,6 +3399,12 @@ void HTMLMediaElement::clearMediaPlayer(int flags)
     if (audioSourceProvider())
         audioSourceProvider()->setClient(0);

[acolwell GONE FROM CHROMIUM, 2013/11/26 16:08:58]

How does this ever get executed? Doesn't audioSourceProvider() return 0 since
m_player is null at this point.

[DaleCurtis, 2013/11/26 18:36:15]

Hah, wow, yeah that's true even in the original CL.  I missed that. The code
should be above the .clear() ? It must introduce enough "hand-waving" to prevent
the race the previous Cl was fixing.

[Ken Russell (switch to Gerrit), 2013/11/26 20:09:24]

This should be fixed, and further, HTMLMediaElement::~HTMLMediaElement has the
same problem. Please refactor this code to share the m_player.clear() and
audioSourceProvider()->setClient(0) calls between these two methods.

[haraken, 2013/11/27 01:10:29]

Done.

 #endif
+
+#if ENABLE(WEB_AUDIO)

[acolwell GONE FROM CHROMIUM, 2013/11/26 16:08:58]

nit: Remove this line and the #endif right above it.

[haraken, 2013/11/27 01:10:29]

Done.

+    if (m_audioSourceNode)
+        m_audioSourceNode->unlock();
+#endif
+
     stopPeriodicTimers();
     m_loadTimer.stop();