Oilpan: Remove guardRef and guardDeref from TreeScope.

Source/core/dom/Document.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) Research In Motion Limited 2010-2011. All rights reserved.
@@ skipping 360 matching lines @@
 };
 
 DocumentVisibilityObserver::DocumentVisibilityObserver(Document& document)
     : m_document(0)
 {
     registerObserver(document);
 }
 
 DocumentVisibilityObserver::~DocumentVisibilityObserver()
 {
+#if !ENABLE(OILPAN)
     unregisterObserver();
+#endif
 }
 
 void DocumentVisibilityObserver::unregisterObserver()
 {
     if (m_document) {
         m_document->unregisterVisibilityObserver(this);
         m_document = 0;
     }
 }
 
@@ skipping 124 matching lines @@
     // m_fetcher.
     m_styleEngine = StyleEngine::create(*this);
 }
 
 Document::~Document()
 {
     ASSERT(!renderView());
     ASSERT(m_ranges.isEmpty());
     ASSERT(!parentTreeScope());
     ASSERT(!hasGuardRefCount());
+#if !ENABLE(OILPAN)
+    // With oilpan the visibility obeservers and the document can die together

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

This comment is a bit misreading. Document can outlive visibility observers.

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

Yes, either the document outlives the visibility observers or they die in the
same GC round. When they die in the same GC round the list of
visibilityObservers is not empty.

Update comment, thanks!

+    // at which point the weak collection of observers will not be empty.
     ASSERT(m_visibilityObservers.isEmpty());
+#endif
 
     if (m_templateDocument)
         m_templateDocument->m_templateDocumentHost = 0; // balanced in ensureTemplateDocument().
 
     m_scriptRunner.clear();
 
     removeAllEventListenersRecursively();
 
     // Currently we believe that Document can never outlive the parser.
     // Although the Document may be replaced synchronously, DocumentParsers
     // generally keep at least one reference to an Element which would in turn
     // has a reference to the Document.  If you hit this ASSERT, then that
     // assumption is wrong.  DocumentParser::detach() should ensure that even
     // if the DocumentParser outlives the Document it won't cause badness.
     ASSERT(!m_parser || m_parser->refCount() == 1);
     detachParser();
 
     if (this == topDocument())
         clearAXObjectCache();
 
+#if !ENABLE(OILPAN)
     if (m_styleSheetList)
         m_styleSheetList->detachFromDocument();
+#endif
 
     if (m_importsController) {
         m_importsController->wasDetachedFrom(*this);
         m_importsController = 0;
     }
 
     m_timeline->detachFromDocument();
     m_transitionTimeline->detachFromDocument();
 
     // We need to destroy CSSFontSelector before destroying m_fetcher.
@@ skipping 869 matching lines @@
     return pageVisibilityState() != PageVisibilityStateVisible;
 }
 
 void Document::didChangeVisibilityState()
 {
     dispatchEvent(Event::create(EventTypeNames::visibilitychange));
     // Also send out the deprecated version until it can be removed.
     dispatchEvent(Event::create(EventTypeNames::webkitvisibilitychange));
 
     PageVisibilityState state = pageVisibilityState();
-    HashSet<DocumentVisibilityObserver*>::const_iterator observerEnd = m_visibilityObservers.end();
-    for (HashSet<DocumentVisibilityObserver*>::const_iterator it = m_visibilityObservers.begin(); it != observerEnd; ++it)
+    WillBeHeapHashSet<RawPtrWillBeWeakMember<DocumentVisibilityObserver> >::const_iterator observerEnd = m_visibilityObservers.end();
+    for (WillBeHeapHashSet<RawPtrWillBeWeakMember<DocumentVisibilityObserver> >::const_iterator it = m_visibilityObservers.begin(); it != observerEnd; ++it)
         (*it)->didChangeVisibilityState(state);
 }
 
 void Document::registerVisibilityObserver(DocumentVisibilityObserver* observer)
 {
     ASSERT(!m_visibilityObservers.contains(observer));
     m_visibilityObservers.add(observer);
 }
 
 void Document::unregisterVisibilityObserver(DocumentVisibilityObserver* observer)
@@ skipping 3382 matching lines @@
     m_ranges.add(range);
 }
 
 void Document::detachRange(Range* range)
 {
     // We don't ASSERT m_ranges.contains(range) to allow us to call this
     // unconditionally to fix: https://bugs.webkit.org/show_bug.cgi?id=26044
     m_ranges.remove(range);
 }
 
-void Document::getCSSCanvasContext(const String& type, const String& name, int width, int height, bool& is2d, RefPtr<CanvasRenderingContext2D>& context2d, bool& is3d, RefPtr<WebGLRenderingContext>& context3d)
+void Document::getCSSCanvasContext(const String& type, const String& name, int width, int height, bool& is2d, RefPtrWillBeRawPtr<CanvasRenderingContext2D>& context2d, bool& is3d, RefPtrWillBeRawPtr<WebGLRenderingContext>& context3d)
 {
     HTMLCanvasElement& element = getCSSCanvasElement(name);
     element.setSize(IntSize(width, height));
     CanvasRenderingContext* context = element.getContext(type);
     if (!context)
         return;
 
     if (context->is2d()) {
         is2d = true;
         context2d = toCanvasRenderingContext2D(context);
@@ skipping 182 matching lines @@
             return element;
     }
     return 0;
 }
 
 void Document::decrementLoadEventDelayCount()
 {
     ASSERT(m_loadEventDelayCount);
     --m_loadEventDelayCount;
 
-    if (frame() && !m_loadEventDelayCount && !m_loadEventDelayTimer.isActive())
+    if (!m_loadEventDelayCount)
+        checkLoadEventSoon();
+}
+
+void Document::checkLoadEventSoon()
+{
+    if (frame() && !m_loadEventDelayTimer.isActive())
         m_loadEventDelayTimer.startOneShot(0, FROM_HERE);
 }
 
+bool Document::isDelayingLoadEvent()
+{
+#if ENABLE(OILPAN)
+    // With Oilpan, always delay load events until after garbage collection.
+    // This way we don't have to explicitly delay load events via
+    // incrementLoadEventDelayCount and decrementLoadEventDelayCount in
+    // Node destructors.
+    if (!m_loadEventDelayCount && ThreadState::current()->isSweepInProgress()) {

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

I understand this code is needed in order not to let Node's destructor touch the
Document, but it looks a bit too hacky...

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

This is a little complicated, but it is a minor complication to an area that is
pretty complicated and hacky already. :) This is the cleanest solution I can
come up with. The issue is:

HTMLMediaElement's destructor can dispatch a load event. In order to prevent
that the HTMLMediaElement's destructor goes via the document to delay the load
event via explicit calls to increment and decrementLoadEventDelayCount. That is
pretty nasty in itself.

We cannot touch the document in the HTMLMediaElement's destructor. And this is a
more general mechanism that makes sure that no destructor *in the entire system*
can ever dispatch a load event on the document. That seems like a nice property.

I'm open for suggestions. I think this is pretty clean considering the amount of
hacks already in this code. :)

+        checkLoadEventSoon();
+        return true;
+    }
+#endif
+    return m_loadEventDelayCount;
+}
+
+
 void Document::loadEventDelayTimerFired(Timer<Document>*)
 {
     if (frame())
         frame()->loader().checkCompleted();
 }
 
 void Document::loadPluginsSoon()
 {
     // FIXME: Remove this timer once we don't need to compute layout to load plugins.
     if (!m_pluginLoadingTimer.isActive())
@@ skipping 533 matching lines @@
     return false;
 }
 
 void Document::invalidateNodeListCaches(const QualifiedName* attrName)
 {
     HashSet<LiveNodeListBase*>::iterator end = m_listsInvalidatedAtDocument.end();
     for (HashSet<LiveNodeListBase*>::iterator it = m_listsInvalidatedAtDocument.begin(); it != end; ++it)
         (*it)->invalidateCache(attrName);
 }
 
+void Document::clearWeakMembers(Visitor* visitor)
+{
+    if (m_axObjectCache)
+        m_axObjectCache->clearWeakMembers(visitor);
+
+    if (m_markers)
+        m_markers->clearWeakMembers(visitor);

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

This is a clarifying question about a programming pattern in oilpan.

In this CL, you're making a Document a weak reference to the Document itself,
and dispatching clearWeakMember() to objects that need weak processing.

Another way to do this is to make "the objects that need weak processing" hold a
weak reference to the Document. Then you don't need to dispatch the weak
processing.

I'd prefer the latter pattern as long as the number of weak references doesn't
become a performance issue. Conceptually, the objects that need weak processing
should hold a weak reference to the owner object.

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

Right, so here is the thing: This is all weak processing that is related to the
document still being alive. Most of this weak processing should be moved to the
objects in question but that requires those objects to be moved to the heap (and
this change is already large and complicated, so I don't want to do more in this
changelist).

In followup changelists, we should move (for instance) the axObjectCache to the
Oilpan heap. At that point the weak processing of the axObjectCache can happen
from the axObjectCache itself. Similarly for the other objects.

But you are absolutely right that the weak callback should happen on the
individual objects that need it instead of going through document. We will get
there once we move those objects to the oilpan heap!

+
+    // FIXME: Oilpan: Use a weak counted set instead.
+    if (m_touchEventTargets) {
+        Vector<Node*> deadNodes;
+        for (TouchEventTargetSet::iterator it = m_touchEventTargets->begin(); it != m_touchEventTargets->end(); ++it) {
+            if (!visitor->isAlive(it->key))
+                deadNodes.append(it->key);
+        }
+        for (unsigned i = 0; i < deadNodes.size(); ++i)
+            didClearTouchEventHandlers(deadNodes[i]);
+    }
+}
+
 void Document::trace(Visitor* visitor)
 {
+    visitor->trace(m_styleSheetList);
+    visitor->trace(m_visibilityObservers);
+    visitor->registerWeakMembers<Document, &Document::clearWeakMembers>(this);
     Supplementable<Document>::trace(visitor);
+    TreeScope::trace(visitor);
     ContainerNode::trace(visitor);
 }
 
 } // namespace WebCore