[Oilpan]: Make StylePropertySet fully garbage collected.

Source/core/dom/PresentationAttributeStyle.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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 38 matching lines @@
     Vector<std::pair<StringImpl*, AtomicString>, 3> attributesAndValues;
 };
 
 static bool operator!=(const PresentationAttributeCacheKey& a, const PresentationAttributeCacheKey& b)
 {
     if (a.tagName != b.tagName)
         return true;
     return a.attributesAndValues != b.attributesAndValues;
 }
 
-struct PresentationAttributeCacheEntry {
-    WTF_MAKE_FAST_ALLOCATED;
+struct PresentationAttributeCacheEntry : public NoBaseWillBeGarbageCollectedFinalized<PresentationAttributeCacheEntry> {
+    WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
 public:
+    void trace(Visitor* visitor) { visitor->trace(value); }
+
     PresentationAttributeCacheKey key;
-    RefPtr<StylePropertySet> value;
+    RefPtrWillBeMember<StylePropertySet> value;
 };
 
-typedef HashMap<unsigned, OwnPtr<PresentationAttributeCacheEntry>, AlreadyHashed> PresentationAttributeCache;
+typedef WillBeHeapHashMap<unsigned, OwnPtrWillBeMember<PresentationAttributeCacheEntry>, AlreadyHashed> PresentationAttributeCache;
 static PresentationAttributeCache& presentationAttributeCache()
 {
+#if ENABLE(OILPAN)
+    DEFINE_STATIC_LOCAL(Persistent<PresentationAttributeCache>, cache, (new PresentationAttributeCache()));
+    return *cache;
+#else
     DEFINE_STATIC_LOCAL(PresentationAttributeCache, cache, ());
     return cache;
+#endif // ENABLE(OILPAN)
 }
 
 class PresentationAttributeCacheCleaner {
     WTF_MAKE_NONCOPYABLE(PresentationAttributeCacheCleaner); WTF_MAKE_FAST_ALLOCATED;
 public:
     PresentationAttributeCacheCleaner()
         : m_hitCount(0)
         , m_cleanTimer(this, &PresentationAttributeCacheCleaner::cleanCache)
     {
     }
@@ skipping 64 matching lines @@
 
 static unsigned computePresentationAttributeCacheHash(const PresentationAttributeCacheKey& key)
 {
     if (!key.tagName)
         return 0;
     ASSERT(key.attributesAndValues.size());
     unsigned attributeHash = StringHasher::hashMemory(key.attributesAndValues.data(), key.attributesAndValues.size() * sizeof(key.attributesAndValues[0]));
     return WTF::pairIntHash(key.tagName->existingHash(), attributeHash);
 }
 
-PassRefPtr<StylePropertySet> computePresentationAttributeStyle(Element& element)
+PassRefPtrWillBeRawPtr<StylePropertySet> computePresentationAttributeStyle(Element& element)
 {
     DEFINE_STATIC_LOCAL(PresentationAttributeCacheCleaner, cacheCleaner, ());
 
     ASSERT(element.isStyledElement());
 
     PresentationAttributeCacheKey cacheKey;
     makePresentationAttributeCacheKey(element, cacheKey);
 
     unsigned cacheHash = computePresentationAttributeCacheHash(cacheKey);
 
-    PresentationAttributeCache::ValueType* cacheValue;
+    PresentationAttributeCacheEntry* cacheEntry;
     if (cacheHash) {
-        cacheValue = presentationAttributeCache().add(cacheHash, nullptr).storedValue;
-        if (cacheValue->value && cacheValue->value->key != cacheKey)
+        cacheEntry = presentationAttributeCache().get(cacheHash);
+        if (cacheEntry && cacheEntry->key != cacheKey)

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

Why did you change this? Why doesn't the Oilpan type using the same API as the
WTF type?

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

I am not sure I follow? The reason for changing the type above is since we do a
get and get a hash map value back and not an entry which is (confusingly) named
ValueType.

             cacheHash = 0;
     } else {
-        cacheValue = 0;
+        cacheEntry = 0;
     }
 
-    RefPtr<StylePropertySet> style;
-    if (cacheHash && cacheValue->value) {
-        style = cacheValue->value->value;
+    RefPtrWillBeRawPtr<StylePropertySet> style;
+    if (cacheHash && cacheEntry) {
+        style = cacheEntry->value;
         cacheCleaner.didHitPresentationAttributeCache();
     } else {
         style = MutableStylePropertySet::create(element.isSVGElement() ? SVGAttributeMode : HTMLAttributeMode);
         unsigned size = element.attributeCount();
         for (unsigned i = 0; i < size; ++i) {
             const Attribute& attribute = element.attributeItem(i);
             element.collectStyleForPresentationAttribute(attribute.name(), attribute.value(), toMutableStylePropertySet(style));
         }
     }
 
-    if (!cacheHash || cacheValue->value)
+    if (!cacheHash || cacheEntry)
         return style.release();
 
-    OwnPtr<PresentationAttributeCacheEntry> newEntry = adoptPtr(new PresentationAttributeCacheEntry);
+    OwnPtrWillBeRawPtr<PresentationAttributeCacheEntry> newEntry = adoptPtrWillBeNoop(new PresentationAttributeCacheEntry);
     newEntry->key = cacheKey;
     newEntry->value = style;
 
     static const unsigned presentationAttributeCacheMaximumSize = 4096;
     if (presentationAttributeCache().size() > presentationAttributeCacheMaximumSize) {
         // FIXME: Discarding the entire cache when it gets too big is probably bad
         // since it creates a perf "cliff". Perhaps we should use an LRU?
         presentationAttributeCache().clear();
-        presentationAttributeCache().set(cacheHash, newEntry.release());
-    } else {
-        cacheValue->value = newEntry.release();
     }
+    presentationAttributeCache().set(cacheHash, newEntry.release());

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

This is again wrong, you make us do two hash lookups instead of one.

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

Yes, this is similar to the previous case where the allocation of the newEntry
could trigger a GC which could cause the backing of the hash map to move. I will
talk to Erik and Mads to see if we can find a way to avoid the additional
lookup.

 
     return style.release();
 }
 
 } // namespace WebCore