Oilpan: move the MediaPlayer and MediaPlayerClient objects to the heap.

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 5a413b76a92dd59e447c11ba062f168869783f8c..05da19265e1387fb0cb99429ad02e697c27345f4 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -282,9 +282,6 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , 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)
@@ -341,7 +338,6 @@ HTMLMediaElement::~HTMLMediaElement()
 
 #if !ENABLE(OILPAN)
     removeElementFromDocumentMap(this, &document());
-#endif
 
     // Destroying the player may cause a resource load to be canceled,
     // which could result in userCancelledLoad() being called back.
@@ -353,7 +349,7 @@ HTMLMediaElement::~HTMLMediaElement()
     // With Oilpan load events on the Document are always delayed during
     // sweeping so we don't need to explicitly increment and decrement
     // load event delay counts.
-#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
@@ -361,25 +357,9 @@ HTMLMediaElement::~HTMLMediaElement()
     // object destruction, we use Document::incrementLoadEventDelayCount().
     // See http://crbug.com/275223 for more details.
     document().incrementLoadEventDelayCount();
-#endif
 
-#if ENABLE(OILPAN)
-    // Oilpan: the player must be released, but the player object
-    // cannot safely access this player client any longer as parts of
-    // it may have been finalized already (like the media element's
-    // 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)
     document().decrementLoadEventDelayCount();
 #endif
 }
@@ -3138,6 +3118,21 @@ void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClient()
         audioSourceProvider()->setClient(0);
 #endif
 
+    // With Oilpan, if the media player is explicitly cleared, the
+    // player is detach()ed first to notify dependent objects.
+    // If this media element (and the player's "client") is finalized
+    // at the same time as the media player, no need to explicitly
+    // detach and notify. All heap-residing objects will die at the
+    // same time, and the media player object will handle notification
+    // of the browser process WebMediaPlayer object when finalized.
+    //
+    // To keep code paths regular, we explicitly detach in a non-Oilpan
+    // setting also rather than do it as part of the player's destructor.
+    if (m_player) {
+        // Reset the web layer right away.
+        mediaPlayerSetWebLayer(0);
+        m_player->detach();
+    }
     m_player.clear();
 
 #if ENABLE(WEB_AUDIO)
@@ -3435,13 +3430,15 @@ void* HTMLMediaElement::preDispatchEventHandler(Event* event)
 
 void HTMLMediaElement::createMediaPlayer()
 {
+    closeMediaSource();
+
+    clearMediaPlayerAndAudioSourceProviderClient();
+
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         m_audioSourceNode->lock();
 #endif
 
-    closeMediaSource();
-
     m_player = MediaPlayer::create(this);
 
 #if ENABLE(WEB_AUDIO)
@@ -3634,9 +3631,8 @@ void HTMLMediaElement::mediaPlayerSetWebLayer(blink::WebLayer* webLayer)
     if (m_webLayer)
         GraphicsLayer::unregisterContentsLayer(m_webLayer);
     m_webLayer = webLayer;
-    if (m_webLayer) {
+    if (m_webLayer)
         GraphicsLayer::registerContentsLayer(m_webLayer);
-    }
 }
 
 void HTMLMediaElement::mediaPlayerMediaSourceOpened(blink::WebMediaSource* webMediaSource)
@@ -3663,9 +3659,11 @@ void HTMLMediaElement::trace(Visitor* visitor)
     visitor->trace(m_error);
     visitor->trace(m_currentSourceNode);
     visitor->trace(m_nextChildNodeToConsider);
+    visitor->trace(m_player);
     visitor->trace(m_textTracks);
     visitor->trace(m_textTracksWhenResourceSelectionBegan);
     WillBeHeapSupplementable<HTMLMediaElement>::trace(visitor);
+    MediaPlayerClient::trace(visitor);
     HTMLElement::trace(visitor);
 }