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

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 264 matching lines @@
     , m_sentStalledEvent(false)
     , m_sentEndEvent(false)
     , m_pausedInternal(false)
     , m_closedCaptionsVisible(false)
     , m_completelyLoaded(false)
     , m_havePreparedToPlay(false)
     , m_delayingLoadForPreloadNone(false)
     , 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)
 #endif
 {
     ASSERT(RuntimeEnabledFeatures::mediaEnabled());
 
     WTF_LOG(Media, "HTMLMediaElement::HTMLMediaElement");
@@ skipping 25 matching lines @@
     // removed from the map during weak processing.
     if (documentToElementSetMap().contains(&document()))
         setShouldDelayLoadEvent(false);
 #else
     setShouldDelayLoadEvent(false);
 #endif
 
 #if !ENABLE(OILPAN)
     if (m_textTracks)
         m_textTracks->clearOwner();
-#endif
 
     if (m_mediaController) {
         m_mediaController->removeMediaElement(this);
         m_mediaController = nullptr;
     }
+#endif
 
     closeMediaSource();
 
 #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.
     // Setting m_completelyLoaded ensures that such a call will not cause
     // us to dispatch an abort event, which would result in a crash.
     // See http://crbug.com/233654 for more details.
     m_completelyLoaded = true;
 
     // 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
     // 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)
-    // 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();
+#else
+    // If the media element is finalized at the same time as
+    // the media player, the latter cannot be accessed and
+    // detached from the destructor. The web layer can be
+    // cleared out safely, however.
+    if (m_player)
+        mediaPlayerSetWebLayer(0);
 #endif
 }
 
 void HTMLMediaElement::didMoveToNewDocument(Document& oldDocument)
 {
     WTF_LOG(Media, "HTMLMediaElement::didMoveToNewDocument");
 
     if (m_shouldDelayLoadEvent) {
         document().incrementLoadEventDelayCount();
         // Note: Keeping the load event delay count increment on oldDocument that was added
@@ skipping 2737 matching lines @@
 void HTMLMediaElement::clearMediaPlayerAndAudioSourceProviderClient()
 {
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         m_audioSourceNode->lock();
 
     if (audioSourceProvider())
         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)
     if (m_audioSourceNode)
         m_audioSourceNode->unlock();
 #endif
 }
 
 void HTMLMediaElement::clearMediaPlayer(int flags)
 {
@@ skipping 277 matching lines @@
 void* HTMLMediaElement::preDispatchEventHandler(Event* event)
 {
     if (event && event->type() == EventTypeNames::webkitfullscreenchange)
         configureMediaControls();
 
     return 0;
 }
 
 void HTMLMediaElement::createMediaPlayer()
 {
+    closeMediaSource();
+
+    clearMediaPlayerAndAudioSourceProviderClient();

[acolwell GONE FROM CHROMIUM, 2014/06/03 19:23:08]

Why is this done here? I believe this needs to occur after the
m_audioSourceNode->lock() to avoid the audio thread from accessing the audio
source provider during destruction.

[sof, 2014/06/03 19:52:44]

"this" == closeMediaSource() ? clearMediaPlayerAndAudioSourceProviderClient()
has its own lock-unlock pairing.

It mirrors the ordering for clearMediaPlayer(), where closeMediaSource() is
called prior to clear...Client(); and there's no explicit lock there first.

IOW, I thought the lock call here was to control/protect the
MediaPlayer::create(this) call.

+
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode)
         m_audioSourceNode->lock();
 #endif
 
-    closeMediaSource();
-
     m_player = MediaPlayer::create(this);
 
 #if ENABLE(WEB_AUDIO)
     if (m_audioSourceNode) {
         // When creating the player, make sure its AudioSourceProvider knows about the client.
         if (audioSourceProvider())
             audioSourceProvider()->setClient(m_audioSourceNode);
 
         m_audioSourceNode->unlock();
     }
@@ skipping 59 matching lines @@
 
     // Otherwise, let controller be a newly created MediaController.
     setControllerInternal(MediaController::create(Node::executionContext()));
 }
 
 MediaController* HTMLMediaElement::controller() const
 {
     return m_mediaController.get();
 }
 
-void HTMLMediaElement::setController(PassRefPtr<MediaController> controller)
+void HTMLMediaElement::setController(PassRefPtrWillBeRawPtr<MediaController> controller)
 {
     // 4.8.10.11.2 Media controllers: controller attribute.
     // On setting, it must first remove the element's mediagroup attribute, if any,
     removeAttribute(mediagroupAttr);
     // and then set the current media controller to the given value.
     setControllerInternal(controller);
 }
 
-void HTMLMediaElement::setControllerInternal(PassRefPtr<MediaController> controller)
+void HTMLMediaElement::setControllerInternal(PassRefPtrWillBeRawPtr<MediaController> controller)
 {
     if (m_mediaController)
         m_mediaController->removeMediaElement(this);
 
     m_mediaController = controller;
 
     if (m_mediaController)
         m_mediaController->addMediaElement(this);
 }
 
@@ skipping 83 matching lines @@
     if (webLayer == m_webLayer)
         return;
 
     // If either of the layers is null we need to enable or disable compositing. This is done by triggering a style recalc.
     if (!m_webLayer || !webLayer)
         setNeedsCompositingUpdate();
 
     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)
 {
     m_mediaSource->setWebMediaSourceAndOpen(adoptPtr(webMediaSource));
 }
 
 bool HTMLMediaElement::isInteractiveContent() const
 {
     return fastHasAttribute(controlsAttr);
 }
 
 void HTMLMediaElement::defaultEventHandler(Event* event)
 {
     if (event->type() == EventTypeNames::focusin) {
         if (hasMediaControls())
             mediaControls()->mediaElementFocused();
     }
     HTMLElement::defaultEventHandler(event);
 }
 
 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);
+    visitor->trace(m_mediaController);
     WillBeHeapSupplementable<HTMLMediaElement>::trace(visitor);
+    MediaPlayerClient::trace(visitor);
     HTMLElement::trace(visitor);
 }
 
 }