Attachment 340952 Details for Bug 185757 – Patch v3

[patch] Patch v3

bug-185757-20180521215332.patch (text/plain), 7.60 KB, created by David Kilzer (:ddkilzer) on 2018-05-21 21:53:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: David Kilzer (:ddkilzer)

Created: 2018-05-21 21:53:33 PDT

Size: 7.60 KB

patch

obsolete

>Subversion Revision: 231940
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 385dd61ab755a5d049b333bcb2ce9f0b359a9764..197ecc223473d6c4d2d93820cfd141384b044af2 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+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!).
>+
>+        Remove WebCore::subimageCache(), then make getSubimage() and
>+        clearImage() static class methods of SubimageCacheWithTimer to
>+        encapsulate the logic to create the SubimageCacheWithTimer
>+        singleton on demand in getSubimage(), and return from
>+        clearImage() if it hasn't been created yet.
>+
>+        * platform/graphics/cg/GraphicsContextCG.cpp:
>+        (WebCore::GraphicsContext::drawNativeImage): Use new
>+        SubimageCacheWithTimer::getSubimage() static method.
>+        * platform/graphics/cg/NativeImageCG.cpp:
>+        (WebCore::clearNativeImageSubimages): Use new
>+        SubimageCacheWithTimer::clearImage() static method which returns
>+        early if the subimage cache has not been created yet.  This
>+        fixes the bug.
>+        * platform/graphics/cg/SubimageCacheWithTimer.cpp:
>+        (WebCore::SubimageCacheWithTimer::getSubimage): Make static
>+        class method.  Create subimage cache singleton if it doesn't
>+        already exist.  Use `auto` after renaming SubimageCache typedef
>+        to SubimageCacheHashSet.
>+        (WebCore::SubimageCacheWithTimer::clearImage): Make static class
>+        method.  Return early if subimage cache singleton doesn't exist
>+        yet.  Modernize loops.
>+        (WebCore::subimageCache): Delete.  Functionality inlined into
>+        SubimageCacheWithTimer::getSubimage() and clearImage().
>+        * platform/graphics/cg/SubimageCacheWithTimer.h:
>+
> 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..0b89d20c0fe4b06fa20426d503753955a8d919fa 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 = SubimageCacheWithTimer::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..987536bc88307506bee224292ef79ffad88b5938 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());
>+        SubimageCacheWithTimer::clearImage(image.get());
> #endif
> }
> 
>diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp
>index 33923ec49011ac34ebf9d0751c9708617c138aaa..b3842761e3838db698d6b1d485e473fc79762f56 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;
> 
>@@ -75,44 +77,45 @@ void SubimageCacheWithTimer::invalidateCacheTimerFired()
> 
> RetainPtr<CGImageRef> SubimageCacheWithTimer::getSubimage(CGImageRef image, const FloatRect& rect)
> {
>-    m_timer.restart();
>-    if (m_cache.size() == maxSubimageCacheSize) {
>-        SubimageCacheEntry entry = *m_cache.begin();
>-        m_images.remove(entry.image.get());
>-        m_cache.remove(entry);
>+    if (!s_cache)
>+        s_cache = new SubimageCacheWithTimer;
>+
>+    SubimageCacheWithTimer& subimageCache = *s_cache;
>+    subimageCache.m_timer.restart();
>+    if (subimageCache.m_cache.size() == maxSubimageCacheSize) {
>+        SubimageCacheEntry entry = *subimageCache.m_cache.begin();
>+        subimageCache.m_images.remove(entry.image.get());
>+        subimageCache.m_cache.remove(entry);
>     }
> 
>-    ASSERT(m_cache.size() < maxSubimageCacheSize);
>-    SubimageCache::AddResult result = m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
>+    ASSERT(subimageCache.m_cache.size() < maxSubimageCacheSize);
>+    auto result = subimageCache.m_cache.add<SubimageCacheAdder>(SubimageRequest(image, rect));
>     if (result.isNewEntry)
>-        m_images.add(image);
>+        subimageCache.m_images.add(image);
> 
>     return result.iterator->subimage;
> }
> 
> void SubimageCacheWithTimer::clearImage(CGImageRef image)
> {
>-    if (m_images.contains(image)) {
>+    if (!s_cache)
>+        return;
>+
>+    SubimageCacheWithTimer& subimageCache = *s_cache;
>+    if (subimageCache.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 : subimageCache.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)
>+            subimageCache.m_cache.remove(entry);
> 
>-        m_images.removeAll(image);
>+        subimageCache.m_images.removeAll(image);
>     }
> }
> 
>-SubimageCacheWithTimer& subimageCache()
>-{
>-    static SubimageCacheWithTimer& cache = *new SubimageCacheWithTimer;
>-    return cache;
>-}
>-
> }
> 
> #endif
>diff --git a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>index 9033e797372335afffc712dd36f71ce30593af54..015c3c0f21cd0db8670c8309b9b012b663bd4e6c 100644
>--- a/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>+++ b/Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h
>@@ -80,22 +80,21 @@ 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 RetainPtr<CGImageRef> getSubimage(CGImageRef, const FloatRect&);
>+    static void clearImage(CGImageRef);
> 
> private:
>+    SubimageCacheWithTimer();
>     void invalidateCacheTimerFired();
> 
>     HashCountedSet<CGImageRef> m_images;
>-    SubimageCache m_cache;
>+    SubimageCacheHashSet m_cache;
>     DeferrableOneShotTimer m_timer;
>-};
> 
>-SubimageCacheWithTimer& subimageCache();
>+    static SubimageCacheWithTimer* s_cache;
>+};
> 
> #endif // CACHE_SUBIMAGES
>

Actions: View | Formatted Diff | Diff

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