Oilpan: Remove guardRef and guardDeref from TreeScope.

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 300 matching lines @@
         m_textTracks->clearOwner();
 #endif
 
     if (m_mediaController) {
         m_mediaController->removeMediaElement(this);
         m_mediaController = nullptr;
     }
 
     closeMediaSource();
 
     removeElementFromDocumentMap(this, &document());

[haraken, 2014/04/24 04:18:43]

Shouldn't this be also put in #if !ENABLE(OILPAN) and implement weak processing
for the map?

[Mads Ager (chromium), 2014/04/24 10:57:36]

Yes, thank you! I misread the code and thought that document was a member here.
It isn't, you go though the tree scope to get to the document. That is not safe.
Changed this to use a weak map from document to a weak set of media elements.
:-)

 
     // 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.

[haraken, 2014/04/24 04:18:43]

I don't quite understand this. Do you need to comment out increment/decrement in
the destructor but not need to comment out other increments/decrements in this
file?

[Mads Ager (chromium), 2014/04/24 10:57:36]

Yes, have a look at the bug report that is referenced in the comment below.
These increments/decrements are only here to prevent a load event dispatch
during destruction. That is now handled in document explicitly so that *no*
destructors for objects in the oilpan heap can dispatch a document load event.

+#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
 
     clearMediaPlayerAndAudioSourceProviderClient();
 
+#if !ENABLE(OILPAN)
     document().decrementLoadEventDelayCount();
+#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
         // when m_shouldDelayLoadEvent was set so that destruction of m_player can not
@@ skipping 3298 matching lines @@
 
 void HTMLMediaElement::trace(Visitor* visitor)
 {
     visitor->trace(m_textTracks);
     visitor->trace(m_textTracksWhenResourceSelectionBegan);
     Supplementable<HTMLMediaElement>::trace(visitor);
     HTMLElement::trace(visitor);
 }
 
 }