Oilpan: move custom element objects to the heap.

Source/core/dom/custom/CustomElementObserver.cpp

 /*
  * Copyright (C) 2013 Google 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
@@ skipping 13 matching lines @@
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/dom/custom/CustomElementObserver.h"
 
+#include "core/dom/Element.h"
+
 namespace WebCore {
 
-CustomElementObserver::ElementObserverMap& CustomElementObserver::elementObservers()
+// Maps elements to the observer watching them. At most one per
+// element at a time.
+typedef WillBeHeapHashMap<RawPtrWillBeWeakMember<Element>, RawPtrWillBeMember<CustomElementObserver> > ElementObserverMap;

[haraken, 2014/05/26 15:40:23]

I think you need to remove the following lines from Element's destructor:

Element::~Element() {
  ...;
  if (isCustomElement())
    CustomElement::wasDestroyed(this);
  ...;
}

CustomElement::wasDestroyed calls
CustomElementUpgradeCandidateMap::elementWasDestroyed and thus touches on-heap
objects. I guess this will cause an ASan issue.

[sof, 2014/05/26 16:41:08]

Very good catch; made it conditional on !ENABLE(OILPAN).

The weak reference on the other side will remove/unregister the object during
the same GC.

[sof, 2014/05/26 19:25:37]

Just to continue that thought & looking at details some more.. the weak
reference to the Element is cleared out before the sweep and Element
finalization, so CustomElementObserver::notifyElementWasDestroyed() will return
early and no possibly-finalized heap objects will be touched here.

So, while clearly wrong to trigger the wasDestroyed() call in ~Element, it is
not unsafe, just redundant (with the current implementation.)

+
+static ElementObserverMap& elementObservers()
 {
+#if ENABLE(OILPAN)
+    DEFINE_STATIC_LOCAL(Persistent<ElementObserverMap>, map, (new ElementObserverMap()));
+    return *map;
+#else
     DEFINE_STATIC_LOCAL(ElementObserverMap, map, ());
     return map;
+#endif
 }
 
 void CustomElementObserver::notifyElementDidFinishParsingChildren(Element* element)
 {
     ElementObserverMap::iterator it = elementObservers().find(element);
     if (it == elementObservers().end())
         return;
     it->value->elementDidFinishParsingChildren(element);
 }
 
@@ skipping 11 matching lines @@
     ASSERT_UNUSED(result, result.isNewEntry);
 }
 
 void CustomElementObserver::unobserve(Element* element)
 {
     CustomElementObserver* observer = elementObservers().take(element);
     ASSERT_UNUSED(observer, observer == this);
 }
 
 } // namespace WebCore