Attachment 340863 Details for Bug 185757 – Patch v2

[patch] Patch v2

bug-185757-20180521105046.patch (text/plain), 7.94 KB, created by David Kilzer (:ddkilzer) on 2018-05-21 10:50:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2018-05-21 10:50:47 PDT

Size: 7.94 KB

patch

obsolete

>Subversion Revision: 231940
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 385dd61ab755a5d049b333bcb2ce9f0b359a9764..8f46e5837dbd7fcd7c1ea34e35f79aab048eb761 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,47 @@
>+2018-05-21  David Kilzer  <ddkilzer@apple.com>
>+
>+        Don't create the SubimageCache just to clear an image from it
>+        <https://webkit.org/b/185757>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We replace WebCore::subimageCache() with two helper functions
>+        that get the cache (SubimageCacheWithTimer::getCache) and check
>+        whether the cache exists yet
>+        (SubimageCacheWithTimer::cacheExists).  By making these static
>+        class methods, we can make the constructor for
>+        SubimageCacheWithTimer private.  Finally, we provide two inline
>+        helper functions in a namespade (for syntactic sugar) that
>+        provide easy access to the subimage cache and abstract the
>+        implementation details a bit:  SubimageCache::getSubimage() and
>+        SubimageCache::clearImage().
>+
>+        * platform/graphics/cg/GraphicsContextCG.cpp:
>+        (WebCore::GraphicsContext::drawNativeImage): Use new
>+        SubimageCache::getSubimage() helper function.
>+        * platform/graphics/cg/NativeImageCG.cpp:
>+        (WebCore::clearNativeImageSubimages): Use new
>+        SubimageCache::clearImage() helper function which returns early
>+        if the subimage cache has not been created yet.  This fixes the
>+        bug.
>+        * platform/graphics/cg/SubimageCacheWithTimer.cpp:
>+        (WebCore::SubimageCacheWithTimer::getSubimage): Use `auto` after
>+        renaming SubimageCache typedef to SubimageCacheHashSet.
>+        (WebCore::SubimageCacheWithTimer::clearImage): Ditto.  Modernize
>+        loops.
>+        (WebCore::subimageCache): Delete.  Replaced by
>+        SubimageCacheWithTimer::getCache().
>+        (WebCore::SubimageCacheWithTimer::cacheExists): Add.  Returns
>+        false if the subimage cache has not been created yet.
>+        (WebCore::SubimageCacheWithTimer::getCache): Add.  Creates
>+        subimage cache if it doesn't exist yet, then returns a reference
>+        to the subimage cache.  Replaces WebCore::subimageCache().
>+        * platform/graphics/cg/SubimageCacheWithTimer.h:
>+        (WebCore::SubimageCache::getSubimage): Add.  Inline helper
>+        function that simplifies subimage cache use.
>+        (WebCore::SubimageCache::clearImage): Add.  Ditto.  Also returns
>+        early if the subimage cache hasn't been created yet.
>+
> 2018-05-17  David Kilzer  <ddkilzer@apple.com>
> 
>         Lazily create WebCore::Timer for WebCore::Image
>diff --git a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
>index 895c90ed10c13eaed66558f2ab78dcb9d6e77654..4a780dd1d505a75e150278f7af92c9aa9a4dcaef 100644
>--- a/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
>+++ b/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
>@@ -330,7 +330,7 @@ void GraphicsContext::drawNativeImage(const RetainPtr<CGImageRef>& image, const
>             adjustedDestRect.setHeight(subimageRect.height() / yScale);
> 
> #if CACHE_SUBIMAGES
>-            subImage = subimageCache().getSubimage(subImage.get(), subimageRect);
>+            subImage = SubimageCache::getSubimage(subImage.get(), subimageRect);
> #else
>             subImage = adoptCF(CGImageCreateWithImageInRect(subImage.get(), subimageRect));
> #endif
>diff --git a/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp b/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp
>index b08026bd1d77d2e2c5e4803832f65a1fd4fc68c9..adbecf848ac6ca943fe964c628e00c5d2aba6efe 100644
>--- a/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp
>+++ b/Source/WebCore/platform/graphics/cg/NativeImageCG.cpp
>@@ -86,7 +86,7 @@ void clearNativeImageSubimages(const NativeImagePtr& image)
> {
> #if CACHE_SUBIMAGES
>     if (image)
>-        subimageCache().clearImage(image.get());
>+        SubimageCache::clearImage(image.get());
> #endif
> }
> 
>diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp
>index 33923ec49011ac34ebf9d0751c9708617c138aaa..b29ffd0fa79ba9b47a663497e930e3efe66accb5 100644
>--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp
>+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp
>@@ -34,6 +34,8 @@
> 
> namespace WebCore {
> 
>+SubimageCacheWithTimer* SubimageCacheWithTimer::s_cache;
>+
> static const Seconds subimageCacheClearDelay { 1_s };
> static const int maxSubimageCacheSize = 300;
> 
>@@ -83,7 +85,7 @@ RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, cons
>     }
> 
>     ASSERT(m_cache.size() < maxSubimageCacheSize);
>-    SubimageCache::AddResult result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
>+    auto result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
>     if (result.isNewEntry)
>         m_images.add(image);
> 
>@@ -94,23 +96,28 @@ void SubimageCacheWithTimer::clearImage(CGImageRef image)
> {
>     if (m_images.contains(image)) {
>         Vector<SubimageCacheEntry> toBeRemoved;
>-        SubimageCache::const_iterator end = m_cache.end();
>-        for (SubimageCache::const_iterator it = m_cache.begin(); it != end; ++it) {
>-            if (it->image.get() == image)
>-                toBeRemoved.append(*it);
>+        for (const auto& entry : m_cache) {
>+            if (entry.image.get() == image)
>+                toBeRemoved.append(entry);
>         }
> 
>-        for (Vector<SubimageCacheEntry>::iterator removeIt = toBeRemoved.begin(); removeIt != toBeRemoved.end(); ++removeIt)
>-            m_cache.remove(*removeIt);
>+        for (auto& entry : toBeRemoved)
>+            m_cache.remove(entry);
> 
>         m_images.removeAll(image);
>     }
> }
> 
>-SubimageCacheWithTimer& subimageCache()
>+bool SubimageCacheWithTimer::cacheExists()
>+{
>+    return !!SubimageCacheWithTimer::s_cache;
>+}
>+
>+SubimageCacheWithTimer& SubimageCacheWithTimer::getCache()
> {
>-    static SubimageCacheWithTimer& cache = *new SubimageCacheWithTimer;
>-    return cache;
>+    if (!SubimageCacheWithTimer::s_cache)
>+        SubimageCacheWithTimer::s_cache = new SubimageCacheWithTimer;
>+    return *SubimageCacheWithTimer::s_cache;
> }
> 
> }
>diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>index 9033e797372335afffc712dd36f71ce30593af54..3033628850d724aaa67814994ada437dec1d3447 100644
>--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>@@ -80,22 +80,43 @@ public:
>         static const bool safeToCompareToEmptyOrDeleted = true;
>     };
> 
>-    typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCache;
>+    typedef HashSet<SubimageCacheEntry, SubimageCacheHash, SubimageCacheEntryTraits> SubimageCacheHashSet;
> 
>-public:
>-    SubimageCacheWithTimer();
>     RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&);
>     void clearImage(CGImageRef);
> 
>+    static bool cacheExists();
>+    static SubimageCacheWithTimer& getCache();
>+
> private:
>+    SubimageCacheWithTimer();
>     void invalidateCacheTimerFired();
> 
>     HashCountedSet<CGImageRef> m_images;
>-    SubimageCache m_cache;
>+    SubimageCacheHashSet m_cache;
>     DeferrableOneShotTimer m_timer;
>+
>+    static SubimageCacheWithTimer* s_cache;
> };
> 
>-SubimageCacheWithTimer& subimageCache();
>+namespace SubimageCache {
>+
>+RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&);
>+void clearImage(CGImageRef);
>+
>+}
>+
>+inline RetainPtr<CGImageRef> SubimageCache::getSubimage(CGImageRef image, const FloatRect& rect)
>+{
>+    return SubimageCacheWithTimer::getCache().getSubimage(image, rect);
>+}
>+
>+inline void SubimageCache::clearImage(CGImageRef image)
>+{
>+    if (!SubimageCacheWithTimer::cacheExists())
>+        return;
>+    SubimageCacheWithTimer::getCache().clearImage(image);
>+}
> 
> #endif // CACHE_SUBIMAGES
>

Actions: View | Formatted Diff | Diff

Attachments on bug 185757: 340676 | 340700 | 340863 | 340952 | 341101