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 268 matching lines @@
     , m_completelyLoaded(false)
     , m_havePreparedToPlay(false)
     , m_delayingLoadForPreloadNone(false)
     , m_tracksAreReady(true)
     , m_haveVisibleTextTrack(false)
     , m_processingPreferenceChange(false)
     , 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 56 matching lines @@
 #if !ENABLE(OILPAN)
     // Destroying the player may cause a resource load to be canceled,
     // which could result in Document::dispatchWindowLoadEvent() being
     // called via ResourceFetch::didLoadResource() then
     // FrameLoader::loadDone(). To prevent load event dispatching during
     // object destruction, we use Document::incrementLoadEventDelayCount().
     // See http://crbug.com/275223 for more details.
     document().incrementLoadEventDelayCount();
 #endif
 
+#if ENABLE(OILPAN)
+#if ENABLE(WEB_AUDIO)
+    if (audioSourceProvider())
+        audioSourceProvider()->setClient(0);
+#endif
+    m_player.clear();
+#else
+    ASSERT(!m_audioSourceNode);

[haraken, 2014/05/21 16:35:58]

I'm curious why you added this ASSERT. If m_audioSourceNode is really guaranteed
to be 0 here, we don't need to have the following code in
clearMediaPlayerAndAudioSourceProviderClient.

if (m_audioSourceNode)
  m_audioSourceNode->lock();
...;
if (m_audioSourceNode)
  m_audioSourceNode->unlock();

(I added the lock/unlock to fix some threading issues a long time ago.)

[zerny-chromium, 2014/05/22 05:54:32]

Exactly. That is why I inlined the reduced code in the oilpan build and added
the assert to the non-oilpan build. It really must be the case since the audio
node has a ref-ptr to the element and upon destruction calls
setAudioSourceNode(0).

I could inline clearMediaPlayerAndAudioSourceProviderClient for both builds.
That would probably be more clear.

The reason why the locking is needed in
clearMediaPlayerAndAudioSourceProviderClient is because of its use outside of
the destructor.

[Mads Ager (chromium), 2014/05/22 08:02:56]

Maybe we can split this into two methods:

clearMediaPlayerAndAudioSourceProviderClientWithoutLocking()
    - do the actual clearing

clearMediaPlayerAndAudioSourceProviderClient():
   - lock
   - clearMediaPlayerAndAudioSourceProviderClientWithoutLocking()
   - unlock

The call the one without locking here and have the assert that m_audioSourceNode
is NULL for the non-oilpan build.

     clearMediaPlayerAndAudioSourceProviderClient();
+#endif
 
 #if !ENABLE(OILPAN)
     document().decrementLoadEventDelayCount();
 #endif
 }
 
 void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument)
 {
     WTF_LOG(Media, "HTMLMediaElement::didMoveToNewDocument");
 
@@ skipping 3268 matching lines @@
     return fastHasAttribute(controlsAttr);
 }
 
 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->trace(m_audioSourceNode);
     WillBeHeapSupplementable<HTMLMediaElement>::trace(visitor);
     HTMLElement::trace(visitor);
 }
 
 }