Prevent more script-observable cases of HTMLMediaElement GC

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 a709141682132ddc3829fd251ecc4500baed28bc..5d8bdfb0ddaa645c120f81e14aca23b09bcdba21 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -382,18 +382,7 @@ HTMLMediaElement::~HTMLMediaElement()
     WTF_LOG(Media, "HTMLMediaElement::~HTMLMediaElement(%p)", this);
 
 #if ENABLE(OILPAN)
-    // If the HTMLMediaElement dies with the document we are not

[Mads Ager (chromium), 2014/09/15 07:01:18]

From an Oilpan perspective it is really nice to get rid of this!

One verifying question: loading on an HTMLMediaElement can only be started from
JavaScript? So we know that there is a JavaScript wrapper for the
HTMLMediaElement when m_shouldDelayLoadEvent is set to true? If that is the
case, this looks correct to me. If there is a wrapper when loading starts the
hasPendingActivity check will keep the wrapper (and therefore the
HTMLMediaElement) alive until the load is completed.

[philipj_slow, 2014/09/15 13:07:44]

No, loading can start without a wrapper, all you need for that to happen is
<video src="video.webm"> and no JavaScript. (m_shouldDelayLoadEvent corresponds
to the spec's delaying-the-load-event flag, which can be triggered by the HTML
parser inserting elements.)

-    // allowed to touch the document to adjust delay load event counts
-    // because the document could have been already
-    // destructed. However, if the HTMLMediaElement dies with the
-    // document there is no need to change the delayed load counts
-    // because no load event will fire anyway. If the document is
-    // still alive we do have to decrement the load delay counts. We
-    // determine if the document is alive via the ActiveDOMObject
-    // which is a context lifecycle observer. If the Document has been
-    // destructed ActiveDOMObject::executionContext() returns 0.
-    if (ActiveDOMObject::executionContext())
-        setShouldDelayLoadEvent(false);
+    ASSERT(!m_shouldDelayLoadEvent);
 #else
     // HTMLMediaElement and m_asyncEventQueue always become unreachable
     // together. So HTMLMediaElemenet and m_asyncEventQueue are destructed in
@@ -3468,7 +3457,28 @@ void HTMLMediaElement::stop()
 
 bool HTMLMediaElement::hasPendingActivity() const
 {
-    return (hasAudio() && isPlaying()) || m_asyncEventQueue->hasPendingEvents();
+    if (!document().isActive())
+        return false;
+
+    if (m_shouldDelayLoadEvent)
+        return true;
+
+    if (m_networkState == NETWORK_LOADING)
+        return true;
+
+    if (potentiallyPlaying())
+        return true;
+
+    if (m_seeking)
+        return true;
+
+    if (m_asyncEventQueue->hasPendingEvents())
+        return true;
+
+    if (m_mediaSource)
+        return true;
+
+    return false;
 }
 
 void HTMLMediaElement::contextDestroyed()