Prevent more script-observable cases of HTMLMediaElement GC

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 375 matching lines @@
     // 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);

[acolwell GONE FROM CHROMIUM, 2014/09/11 16:33:40]

You might be able to remove this and the one below now that
m_shouldDelayLoadEvent being true prevents this object from getting destroyed.

 #else
     // HTMLMediaElement and m_asyncEventQueue always become unreachable
     // together. So HTMLMediaElemenet and m_asyncEventQueue are destructed in
     // the same GC. We don't need to close it explicitly in Oilpan.
     m_asyncEventQueue->close();
 
     setShouldDelayLoadEvent(false);
 
     if (m_textTracks)
         m_textTracks->clearOwner();
@@ skipping 3054 matching lines @@
         renderer()->updateFromElement();
 
     stopPeriodicTimers();
     cancelPendingEventsAndCallbacks();
 
     m_asyncEventQueue->close();
 }
 
 bool HTMLMediaElement::hasPendingActivity() const
 {
-    return (hasAudio() && isPlaying()) || m_asyncEventQueue->hasPendingEvents();
+    if (m_shouldDelayLoadEvent)
+        return true;
+
+    if (m_networkState == NETWORK_LOADING)
+        return true;
+
+    if (potentiallyPlaying())

[Ken Russell (switch to Gerrit), 2014/09/11 18:20:56]

Consider the case where a video is set to loop and then to play. Then a page
navigation occurs and ActiveDOMObject::stop is called against the video element.
Will that cause potentiallyPlaying() to start returning false so that the
HTMLMediaElement will ultimately be GC'd? (Or do ActiveDOMObjects have some
other mechanism to prevent them from leaking once the page that contained them
is destroyed?)

[acolwell GONE FROM CHROMIUM, 2014/09/11 19:02:24]

I believe it we add an m_readyStateMaximum = HAVE_NOTHING to userCancelledLoad()
near the m_readyState assignment we'll cover most of the cases. I'm still trying
to figure out what the right thing to do is if m_completelyLoaded is true
because that assignment is skipped in that case.

[philipj_slow, 2014/09/12 14:05:20]

I'll add "if (!document().isActive()) return false" at the top of
hasPendingActivity() as a catch-all for this kind of thing. Sound OK?

+        return true;
+
+    if (m_seeking)
+        return true;
+
+    if (m_asyncEventQueue->hasPendingEvents())
+        return true;
+
+    return false;
 }
 
 void HTMLMediaElement::contextDestroyed()
 {
     // With Oilpan the ExecutionContext is weakly referenced from the media
     // controller and so it will clear itself on destruction.
 #if !ENABLE(OILPAN)
     if (m_mediaController)
         m_mediaController->clearExecutionContext();
 #endif
@@ skipping 501 matching lines @@
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
 {
     if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
         audioSourceProvider()->setClient(0);
 }
 #endif
 
 }