Block load event dispatching on old document when an HTMLMediaElement is moved between documents.

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 cfe442397c75afbbe97d8d06a79fd678961a8dea..6469002ecc62074f6b06aef0666d012e4ef4f7b1 100644
--- a/Source/core/html/HTMLMediaElement.cpp
+++ b/Source/core/html/HTMLMediaElement.cpp
@@ -370,9 +370,14 @@ void HTMLMediaElement::didMoveToNewDocument(Document* oldDocument)
     LOG(Media, "HTMLMediaElement::didMoveToNewDocument");
 
     if (m_shouldDelayLoadEvent) {
-        if (oldDocument)
-            oldDocument->decrementLoadEventDelayCount();
         document().incrementLoadEventDelayCount();
+        // Note: Keeping the load event delay count increment on oldDocument that was added
+        // when m_shouldDelayLoadEvent was set so that destruction of m_player can not
+        // cause load event dispatching in oldDocument.
+    } else if (oldDocument) {
+        // Incrementing the load event delay count so that destruction of m_player can not
+        // cause load event dispatching in oldDocument.
+        oldDocument->incrementLoadEventDelayCount();
     }
 
     if (oldDocument)
@@ -387,6 +392,11 @@ void HTMLMediaElement::didMoveToNewDocument(Document* oldDocument)
     // document changes so that playback can be resumed properly.
     userCancelledLoad();
 
+    // Decrement the load event delay count on oldDocument now that m_player has been destroyed
+    // and there is no risk of dispatching a load event from within the destructor.

[abarth-chromium, 2013/10/01 19:49:19]

This code looks very fragile.  Is there a better design that's less fragile?

[acolwell GONE FROM CHROMIUM, 2013/10/01 22:02:04]

This increment/decrementLoadEventDelayCount() does seem fragile, but I'm not
sure how else to handle it. I'm essentially doing the same thing that is being
done in the HTMLMediaElement destructor to avoid event dispatch from within
m_player's destructor. The tricky part here though is that I can't use
document() because m_player is still associated with the old document. 

I'm also going to need to merge this fix to multiple branches so I'm trying to
keep the changes minimal.(see bug details) If a better solution requires more
invasive changes, I'd prefer to defer that to a followup CL instead to avoid
merge pain.

+    if (oldDocument)
+        oldDocument->decrementLoadEventDelayCount();
+
     HTMLElement::didMoveToNewDocument(oldDocument);
 }