[Oilpan]: Make StylePropertySet fully garbage collected.

Source/core/css/resolver/MatchedPropertiesCache.cpp

Index: Source/core/css/resolver/MatchedPropertiesCache.cpp
diff --git a/Source/core/css/resolver/MatchedPropertiesCache.cpp b/Source/core/css/resolver/MatchedPropertiesCache.cpp
index 469fa3c9c2fb3b0fc99395faa735e8ff61adf3db..118be4856cdc8b6126d1d6ca3d4a152c6a6a2e54 100644
--- a/Source/core/css/resolver/MatchedPropertiesCache.cpp
+++ b/Source/core/css/resolver/MatchedPropertiesCache.cpp
@@ -56,6 +56,9 @@ void CachedMatchedProperties::clear()
 MatchedPropertiesCache::MatchedPropertiesCache()
     : m_additionsSinceLastSweep(0)
     , m_sweepTimer(this, &MatchedPropertiesCache::sweep)
+#if ENABLE(OILPAN)
+    , m_cacheNeedsCleaning(false)
+#endif
 {
 }
 
@@ -63,6 +66,13 @@ const CachedMatchedProperties* MatchedPropertiesCache::find(unsigned hash, const
 {
     ASSERT(hash);
 
+#if ENABLE(OILPAN)
+    // If we had a GC since the last find we could have entries that are now invalid.

[esprehn, 2014/05/14 20:25:27]

This feels very wrong, why are we allowing dead pointers to live after a GC?
This feels like a use-after-free waiting to happen. We shouldn't switch
something to oilpan until we can ensure that dead pointers don't live after the
GC kills an object.

[wibling-chromium, 2014/05/15 13:15:18]

This is a temporary work around to the issue Haraken is mentioning. Basically we
cannot remove elements from our heap hash maps during the GC weak processing.
The reason being that we don't allow reallocation of the hash map's backing
which doing a GC.
That said there are no dead pointers. The pointers in the HeapVector are cleared
(NULL) during the GC since they are WeakMembers, but due to the above
restriction we cannot remove the containing HashMap entry until we are out of
the GC. Erik is working on enhancing our collections to support this at which
point we will do the entire cleanup during GC.

+    // Sweep the cache to ensure it is in a valid state before doing the find.
+    if (m_cacheNeedsCleaning)
+        sweep(0);
+#endif
+
     Cache::iterator it = m_cache.find(hash);
     if (it == m_cache.end())
         return 0;
@@ -95,7 +105,7 @@ void MatchedPropertiesCache::add(const RenderStyle* style, const RenderStyle* pa
     ASSERT(hash);
     Cache::AddResult addResult = m_cache.add(hash, nullptr);
     if (addResult.isNewEntry)
-        addResult.storedValue->value = adoptPtr(new CachedMatchedProperties);
+        addResult.storedValue->value = adoptPtrWillBeNoop(new CachedMatchedProperties);
 
     CachedMatchedProperties* cacheItem = addResult.storedValue->value.get();
     if (!addResult.isNewEntry)
@@ -122,6 +132,9 @@ void MatchedPropertiesCache::clearViewportDependent()
 
 void MatchedPropertiesCache::sweep(Timer<MatchedPropertiesCache>*)
 {
+#if ENABLE(OILPAN)
+    m_cacheNeedsCleaning = false;
+#endif
     // Look for cache entries containing a style declaration with a single ref and remove them.
     // This may happen when an element attribute mutation causes it to generate a new inlineStyle()
     // or presentationAttributeStyle(), potentially leaving this cache with the last ref on the old one.
@@ -130,9 +143,14 @@ void MatchedPropertiesCache::sweep(Timer<MatchedPropertiesCache>*)
     Cache::iterator end = m_cache.end();
     for (; it != end; ++it) {
         CachedMatchedProperties* cacheItem = it->value.get();
-        Vector<MatchedProperties>& matchedProperties = cacheItem->matchedProperties;
+        WillBeHeapVector<MatchedProperties>& matchedProperties = cacheItem->matchedProperties;
         for (size_t i = 0; i < matchedProperties.size(); ++i) {
-            if (matchedProperties[i].properties->hasOneRef()) {
+#if ENABLE(OILPAN)
+            if (!matchedProperties[i].properties)
+#else
+            if (matchedProperties[i].properties->hasOneRef())
+#endif
+            {
                 toRemove.append(it->key);
                 break;
             }
@@ -166,4 +184,17 @@ bool MatchedPropertiesCache::isCacheable(const Element* element, const RenderSty
     return true;
 }
 
+void MatchedPropertiesCache::trace(Visitor* visitor)
+{
+    visitor->trace(m_cache);
+    visitor->registerWeakMembers<MatchedPropertiesCache, &MatchedPropertiesCache::processWeakMembers>(this);
+}
+
+void MatchedPropertiesCache::processWeakMembers(Visitor*)
+{
+#if ENABLE(OILPAN)
+    m_cacheNeedsCleaning = true;
+#endif
+}
+
 }