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

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 352 matching lines @@
         audioSourceProvider()->setClient(0);
 #endif
     document().decrementLoadEventDelayCount();
 }
 
 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)
         removeElementFromDocumentMap(this, oldDocument);
 
     addElementToDocumentMap(this, &document());
 
     // FIXME: This is a temporary fix to prevent this object from causing the
     // MediaPlayer to dereference Frame and FrameLoader pointers from the
     // previous document. A proper fix would provide a mechanism to allow this
     // object to refresh the MediaPlayer's Frame and FrameLoader references on
     // 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);
 }
 
 bool HTMLMediaElement::hasCustomFocusLogic() const
 {
     return true;
 }
 
 bool HTMLMediaElement::supportsFocus() const
 {
@@ skipping 3597 matching lines @@
 {
     m_restrictions = NoRestrictions;
 }
 
 void HTMLMediaElement::mediaPlayerScheduleLayerUpdate()
 {
     scheduleLayerUpdate();
 }
 
 }