Oilpan: Use transition types for HTMLMediaElement and make AudioSourceProviderClient a GC mixin.

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 266 matching lines @@
     , m_tracksAreReady(true)
     , m_haveVisibleTextTrack(false)
     , m_processingPreferenceChange(false)
 #if ENABLE(OILPAN)
     , m_isFinalizing(false)
 #endif
     , m_lastTextTrackUpdateTime(-1)
     , m_textTracks(nullptr)
     , m_ignoreTrackDisplayUpdate(0)
 #if ENABLE(WEB_AUDIO)
-    , m_audioSourceNode(0)
+    , m_audioSourceNode(nullptr)
 #endif
 {
     ASSERT(RuntimeEnabledFeatures::mediaEnabled());
 
     WTF_LOG(Media, "HTMLMediaElement::HTMLMediaElement");
     ScriptWrappable::init(this);
 
     if (document.settings() && document.settings()->mediaPlaybackRequiresUserGesture())
         m_userGestureRequiredForPlay = true;
 
@@ skipping 67 matching lines @@
     // supplementable table.)  Handled for now by entering an
     // is-finalizing state, which is explicitly checked for if the
     // player tries to access the media element during shutdown.
     //
     // FIXME: Oilpan: move the media player to the heap instead and
     // avoid having to finalize it from here; this whole #if block
     // could then be removed (along with the state bit it depends on.)
     // crbug.com/378229
     m_isFinalizing = true;
 #endif
-    clearMediaPlayerAndAudioSourceProviderClient();
+
+#if !ENABLE(OILPAN)
+    ASSERT(!m_audioSourceNode);
+#endif
+    clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();

[acolwell GONE FROM CHROMIUM, 2014/06/10 17:07:17]

This makes me nervous. This is safe because we can't get here if MEASN is still
holding a reference to this object and MEASN won't get destructed unless it is
detached from the WebAudio node graph an therefore it isn't possible for a call
to come in from the audio processing thread? 

Assuming what I've said here is correct, it would be nice to have a comment here
outlining why it is ok to not lock.

[zerny-chromium, 2014/06/10 19:42:29]

Yes. The HTMLMediaElement can only die after the MEASN is gone, so we can assert
that in the non-oilpan build. With oilpan, it can be the case that both objects
are being finalized in the same GC round. Therefor, the destructor can not
access m_audioSourceNode which is potentially already finalized or will be soon.

I've added a comment to that effect.

 
 #if !ENABLE(OILPAN)
     document().decrementLoadEventDelayCount();
 #endif
 }
 
 void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument)
 {
     WTF_LOG(Media, "HTMLMediaElement::didMoveToNewDocument");
 
@@ skipping 2726 matching lines @@
 
     // 6 - Abort the overall resource selection algorithm.
     m_currentSourceNode = nullptr;
 
     // Reset m_readyState since m_player is gone.
     m_readyState = HAVE_NOTHING;
     updateMediaController();
     updateActiveTextTrackCues(0);
 }
 
+void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClientWithoutLocking()
+{
+#if ENABLE(WEB_AUDIO)
+    if (audioSourceProvider())
+        audioSourceProvider()->setClient(0);
+#endif
+    m_player.clear();
+}
+
 void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClient()

[acolwell GONE FROM CHROMIUM, 2014/06/10 17:07:17]

nit: How about just moving the body of this function into clearMediaPlayer()
since that appears to be the only call site?

[zerny-chromium, 2014/06/10 19:42:29]

Done.

 {
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         m_audioSourceNode->lock();
-
-    if (audioSourceProvider())
-        audioSourceProvider()->setClient(0);
 #endif
 
-    m_player.clear();
+    clearMediaPlayerAndAudioSourceProviderClientWithoutLocking();
 
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         m_audioSourceNode->unlock();
 #endif
 }
 
 void HTMLMediaElement::clearMediaPlayer(int flags)
 {
     forgetResourceSpecificTracks();
@@ skipping 506 matching lines @@
     HTMLElement::defaultEventHandler(event);
 }
 
 void HTMLMediaElement::trace(Visitor* visitor)
 {
     visitor->trace(m_error);
     visitor->trace(m_currentSourceNode);
     visitor->trace(m_nextChildNodeToConsider);
     visitor->trace(m_textTracks);
     visitor->trace(m_textTracksWhenResourceSelectionBegan);
+    visitor->registerWeakMembers<HTMLMediaElement, &HTMLMediaElement::clearWeakMembers>(this);
     WillBeHeapSupplementable<HTMLMediaElement>::trace(visitor);
     HTMLElement::trace(visitor);
 }
 
+void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
+{
+    if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
+        audioSourceProvider()->setClient(0);
 }
+
+}