[Oilpan]: Make StylePropertySet fully garbage collected.

Source/core/css/resolver/MatchedPropertiesCache.h

Index: Source/core/css/resolver/MatchedPropertiesCache.h
diff --git a/Source/core/css/resolver/MatchedPropertiesCache.h b/Source/core/css/resolver/MatchedPropertiesCache.h
index d86bf01f856cfc35b29ce941a2c0e18c80581dd8..aa3382f798dad087bab202d8e2103a0318d87b42 100644
--- a/Source/core/css/resolver/MatchedPropertiesCache.h
+++ b/Source/core/css/resolver/MatchedPropertiesCache.h
@@ -23,9 +23,10 @@
 #ifndef MatchedPropertiesCache_h
 #define MatchedPropertiesCache_h
 
+#include "core/css/StylePropertySet.h"
 #include "core/css/resolver/MatchResult.h"
-
 #include "platform/Timer.h"
+#include "platform/heap/Handle.h"
 #include "wtf/Forward.h"
 #include "wtf/HashMap.h"
 #include "wtf/Noncopyable.h"
@@ -35,17 +36,55 @@ namespace WebCore {
 class RenderStyle;
 class StyleResolverState;
 
-struct CachedMatchedProperties {
-    Vector<MatchedProperties> matchedProperties;
+class CachedMatchedProperties : public NoBaseWillBeGarbageCollectedFinalized<CachedMatchedProperties> {

[zerny-chromium, 2014/06/25 06:19:02]

Nit: FINAL

[wibling-chromium, 2014/06/25 09:03:28]

Done.

+
+public:
+    WillBeHeapVector<MatchedProperties> matchedProperties;
     MatchRanges ranges;
     RefPtr<RenderStyle> renderStyle;
     RefPtr<RenderStyle> parentRenderStyle;
 
     void set(const RenderStyle*, const RenderStyle* parentStyle, const MatchResult&);
     void clear();
+    void trace(Visitor* visitor) { visitor->trace(matchedProperties); }
 };
 
+// Specialize the HashTraits for CachedMatchedProperties to check for dead
+// entries in the MatchedPropertiesCache.

[Mads Ager (chromium), 2014/06/25 06:31:48]

MatchedPropertiesCache -> matchedProperties vector?

[wibling-chromium, 2014/06/25 09:03:27]

I was thinking about it being the entries in the cache that I am checking. The
check involves running through the matchedProperties vector, but the trait is
for the value element in the MatchedProperties' m_cache.

+#if ENABLE(OILPAN)
+struct CachedMatchedPropertiesHashTraits : HashTraits<Member<CachedMatchedProperties> > {
+    static const WTF::WeakHandlingFlag weakHandlingFlag = WTF::WeakHandlingInCollections;
+    static bool traceInCollection(Visitor* visitor, Member<CachedMatchedProperties>& cachedProperties, WTF::ShouldWeakPointersBeMarkedStrongly strongify)
+    {
+        // The below loop serves two purposes. It either marks the MatchedProperties'
+        // "properties" field strong if "strongify" is set to WeakPointersActStrong.
+        // Otherwise it checks if any of the MatchedProperties has a "properties"
+        // field that is dead and if so the entire entry in m_cache is removed
+        // (by returning isDead == true).
+        HeapVector<MatchedProperties>::iterator it = cachedProperties->matchedProperties.begin();
+        HeapVector<MatchedProperties>::iterator end = cachedProperties->matchedProperties.end();
+        bool isDead = false;
+        while (it != end) {

[zerny-chromium, 2014/06/25 06:19:02]

Nit: for(; it != end; ++it)

[wibling-chromium, 2014/06/25 09:03:27]

Done.

+            if (strongify == WTF::WeakPointersActStrong) {
+                visitor->traceInCollection(it->properties, strongify);
+            } else if (!visitor->isAlive(it->properties)) {
+                isDead = true;

[zerny-chromium, 2014/06/25 06:19:02]

It looks like this might as well be:

  return true;

and the code after the loop specialized to isDead == false.

[wibling-chromium, 2014/06/25 09:03:27]

Done.

+                break;
+            }
+            ++it;
+        }
+
+        // If none of the entries in the matchedProperties vector had a dead weak
+        // member we trace CachedMatchedProperties normally.
+        if (!isDead)
+            visitor->trace(cachedProperties);
+        return isDead;
+    }
+};
+#endif
+
 class MatchedPropertiesCache {
+    DISALLOW_ALLOCATION();
     WTF_MAKE_NONCOPYABLE(MatchedPropertiesCache);
 public:
     MatchedPropertiesCache();
@@ -58,7 +97,12 @@ public:
 
     static bool isCacheable(const Element*, const RenderStyle*, const RenderStyle* parentStyle);
 
+    void trace(Visitor*);
+
 private:
+#if ENABLE(OILPAN)
+    typedef HeapHashMap<unsigned, Member<CachedMatchedProperties>, DefaultHash<unsigned>::Hash, HashTraits<unsigned>, CachedMatchedPropertiesHashTraits > Cache;
+#else
     // Every N additions to the matched declaration cache trigger a sweep where entries holding
     // the last reference to a style declaration are garbage collected.
     void sweep(Timer<MatchedPropertiesCache>*);
@@ -66,9 +110,9 @@ private:
     unsigned m_additionsSinceLastSweep;
 
     typedef HashMap<unsigned, OwnPtr<CachedMatchedProperties> > Cache;
-    Cache m_cache;
-
     Timer<MatchedPropertiesCache> m_sweepTimer;
+#endif
+    Cache m_cache;
 };
 
 }