Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(629)

Unified Diff: Source/core/dom/Document.cpp

Issue 235113002: Oilpan: Remove guardRef and guardDeref from TreeScope. (Closed) Base URL: svn://svn.chromium.org/blink/trunk
Patch Set: Address review comments. Created 6 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: Source/core/dom/Document.cpp
diff --git a/Source/core/dom/Document.cpp b/Source/core/dom/Document.cpp
index 023399805da574f2d514b2d8e243b7ab860decb7..5579b7a8b0210c33a9d2b5a5fd1a48217d83fe13 100644
--- a/Source/core/dom/Document.cpp
+++ b/Source/core/dom/Document.cpp
@@ -378,7 +378,9 @@ DocumentVisibilityObserver::DocumentVisibilityObserver(Document& document)
DocumentVisibilityObserver::~DocumentVisibilityObserver()
{
+#if !ENABLE(OILPAN)
unregisterObserver();
+#endif
}
void DocumentVisibilityObserver::unregisterObserver()
@@ -522,8 +524,14 @@ Document::~Document()
ASSERT(!renderView());
ASSERT(m_ranges.isEmpty());
ASSERT(!parentTreeScope());
+#if !ENABLE(OILPAN)
ASSERT(!hasGuardRefCount());
+ // With Oilpan, either the document outlives the visibility observers
+ // or the visibility observers and the document die in the same GC round.
+ // When they die in the same GC round, the list of visibility observers
+ // will not be empty on Document destruction.
Erik Corry 2014/04/24 13:53:16 Is it worth checking that they are all dead at thi
Mads Ager (chromium) 2014/04/24 14:12:14 I don't think so, no.
ASSERT(m_visibilityObservers.isEmpty());
+#endif
if (m_templateDocument)
m_templateDocument->m_templateDocumentHost = 0; // balanced in ensureTemplateDocument().
@@ -544,8 +552,10 @@ Document::~Document()
if (this == topDocument())
clearAXObjectCache();
+#if !ENABLE(OILPAN)
if (m_styleSheetList)
m_styleSheetList->detachFromDocument();
+#endif
if (m_importsController) {
m_importsController->wasDetachedFrom(*this);
@@ -1435,8 +1445,8 @@ void Document::didChangeVisibilityState()
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();
Erik Corry 2014/04/24 13:53:16 I think a typedef would be good here.
Mads Ager (chromium) 2014/04/24 14:12:14 Done.
+ for (WillBeHeapHashSet<RawPtrWillBeWeakMember<DocumentVisibilityObserver> >::const_iterator it = m_visibilityObservers.begin(); it != observerEnd; ++it)
(*it)->didChangeVisibilityState(state);
}
@@ -4839,7 +4849,7 @@ void Document::detachRange(Range* range)
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));
@@ -5042,10 +5052,32 @@ void Document::decrementLoadEventDelayCount()
ASSERT(m_loadEventDelayCount);
--m_loadEventDelayCount;
- if (frame() && !m_loadEventDelayCount && !m_loadEventDelayTimer.isActive())
+ if (!m_loadEventDelayCount)
Erik Corry 2014/04/24 13:53:16 Style guide was just revised so you can write == 0
Mads Ager (chromium) 2014/04/24 14:12:14 OK, not going to bother with that right now. The r
+ 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.
Erik Corry 2014/04/24 13:53:16 The "With Oilpan" part of the comment is obsolete
Mads Ager (chromium) 2014/04/24 14:12:14 Yes, true. Done.
+ // This way we don't have to explicitly delay load events via
+ // incrementLoadEventDelayCount and decrementLoadEventDelayCount in
+ // Node destructors.
+ if (!m_loadEventDelayCount && ThreadState::current()->isSweepInProgress()) {
+ checkLoadEventSoon();
Erik Corry 2014/04/24 13:53:16 Just because we asked whether load events are bein
Mads Ager (chromium) 2014/04/24 14:12:14 Good point. Done. If we are sweeping, we check for
+ return true;
+ }
+#endif
+ return m_loadEventDelayCount;
Erik Corry 2014/04/24 13:53:16 != 0
Mads Ager (chromium) 2014/04/24 14:12:14 Nah, not in this change. The rest of this code use
+}
+
+
void Document::loadEventDelayTimerFired(Timer<Document>*)
{
if (frame())
@@ -5599,9 +5631,33 @@ void Document::invalidateNodeListCaches(const QualifiedName* attrName)
(*it)->invalidateCacheForAttribute(attrName);
}
+void Document::clearWeakMembers(Visitor* visitor)
+{
+ if (m_axObjectCache)
+ m_axObjectCache->clearWeakMembers(visitor);
+
+ if (m_markers)
+ m_markers->clearWeakMembers(visitor);
+
+ // 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);
}

Powered by Google App Engine
This is Rietveld 408576698