Reviewed by Dave Hyatt.
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Aug 2007 02:30:30 +0000 (02:30 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Aug 2007 02:30:30 +0000 (02:30 +0000)
        Refactored live decoded resource eviction to be more modular /
        encapsulated.

        This fixes one known place where we forgot to hook into the live
        decoded eviction mechanism -- canvas. There might be other, unknown
        places. In a canvas test page, which I broke off from the Safari
        pageout test, I saw an RPRVT reduction of ~10MB.

        A few renames:
        - "m_lastLiveAccessTime" => "m_lastDecodedAccessTime" because the data
        point we're recording is access to the resource in decoded form.

        - "liveResourceAccessed" => "didAccessDecodedData" for the same reason.

        - "pruneAllResources" => "pruneDeadResources" because this function
        does not prune live resources.

        And the fix:
        Instead of updating cache metadata at the call site whenver drawing an
        image, just have an image notify its observer whenever it draws. The
        observer, which is a CachedResource, can then update the metadata.

        * loader/Cache.cpp: Renames
        * loader/Cache.h: Removed stale declarations, updated comments
        * loader/CachedImage.cpp:
        (WebCore::CachedImage::didDraw): Implemented didDraw to update cache
        metadata whenever our image draws.
        * loader/CachedImage.h: Grouped parts of the ImageObserver interface.
        * loader/CachedResource.cpp:
        (WebCore::CachedResource::CachedResource):
        (WebCore::CachedResource::deref):
        (WebCore::CachedResource::didAccessDecodedData): Made this function
        slightly more modular by allowing the caller to provide a time stamp.
        In theory, not all CachedResources will necessarily want to use the
        current paint time stamp.
        * platform/graphics/cg/ImageCG.cpp:
        (WebCore::BitmapImage::draw): Notify our observer that we drew.
        (WebCore::Image::drawPattern): ditto
        * platform/graphics/cg/PDFDocumentImage.cpp:
        (WebCore::PDFDocumentImage::draw): ditto
        * platform/graphics/svg/SVGImage.cpp:
        (WebCore::SVGImage::draw): ditto

        Removed old code at image drawing call sites:

        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::paintBackgroundExtended):
        * rendering/RenderImage.cpp:
        (WebCore::RenderImage::paint):
        * rendering/RenderListMarker.cpp:
        (WebCore::RenderListMarker::paint):
        * rendering/RenderObject.cpp:
        (WebCore::RenderObject::paintBorderImage):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@24970 268f45cc-cd09-0410-ab3c-d52691b4dbfc

16 files changed:
WebCore/ChangeLog
WebCore/html/CanvasPattern.cpp
WebCore/loader/Cache.cpp
WebCore/loader/Cache.h
WebCore/loader/CachedImage.cpp
WebCore/loader/CachedImage.h
WebCore/loader/CachedResource.cpp
WebCore/loader/CachedResource.h
WebCore/platform/graphics/ImageObserver.h
WebCore/platform/graphics/cg/ImageCG.cpp
WebCore/platform/graphics/cg/PDFDocumentImage.cpp
WebCore/platform/graphics/svg/SVGImage.cpp
WebCore/rendering/RenderBox.cpp
WebCore/rendering/RenderImage.cpp
WebCore/rendering/RenderListMarker.cpp
WebCore/rendering/RenderObject.cpp

index 9358cd5..d6f653e 100644 (file)
@@ -1,3 +1,61 @@
+2007-08-09  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Dave Hyatt.
+        
+        Refactored live decoded resource eviction to be more modular / 
+        encapsulated. 
+        
+        This fixes one known place where we forgot to hook into the live 
+        decoded eviction mechanism -- canvas. There might be other, unknown 
+        places. In a canvas test page, which I broke off from the Safari 
+        pageout test, I saw an RPRVT reduction of ~10MB.
+        
+        A few renames:
+        - "m_lastLiveAccessTime" => "m_lastDecodedAccessTime" because the data
+        point we're recording is access to the resource in decoded form.
+        
+        - "liveResourceAccessed" => "didAccessDecodedData" for the same reason.
+
+        - "pruneAllResources" => "pruneDeadResources" because this function 
+        does not prune live resources.
+        
+        And the fix:
+        Instead of updating cache metadata at the call site whenver drawing an 
+        image, just have an image notify its observer whenever it draws. The 
+        observer, which is a CachedResource, can then update the metadata.
+        
+        * loader/Cache.cpp: Renames
+        * loader/Cache.h: Removed stale declarations, updated comments
+        * loader/CachedImage.cpp:
+        (WebCore::CachedImage::didDraw): Implemented didDraw to update cache
+        metadata whenever our image draws.
+        * loader/CachedImage.h: Grouped parts of the ImageObserver interface.
+        * loader/CachedResource.cpp:
+        (WebCore::CachedResource::CachedResource):
+        (WebCore::CachedResource::deref):
+        (WebCore::CachedResource::didAccessDecodedData): Made this function
+        slightly more modular by allowing the caller to provide a time stamp.
+        In theory, not all CachedResources will necessarily want to use the 
+        current paint time stamp.
+        * platform/graphics/cg/ImageCG.cpp:
+        (WebCore::BitmapImage::draw): Notify our observer that we drew.
+        (WebCore::Image::drawPattern): ditto
+        * platform/graphics/cg/PDFDocumentImage.cpp:
+        (WebCore::PDFDocumentImage::draw): ditto
+        * platform/graphics/svg/SVGImage.cpp:
+        (WebCore::SVGImage::draw): ditto
+        
+        Removed old code at image drawing call sites:
+        
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::paintBackgroundExtended):
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paint):
+        * rendering/RenderListMarker.cpp:
+        (WebCore::RenderListMarker::paint):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::paintBorderImage):
+
 2007-08-10  Holger Hans Peter Freyther  <zecke@selfish.org>
 
         Reviewed by Adam.
index 730a2d2..d645fb1 100644 (file)
@@ -121,6 +121,10 @@ static void patternCallback(void* info, CGContextRef context)
 
     if (image->getCGImageRef()) {
         CGContextDrawImage(context, rect, image->getCGImageRef());
+        // FIXME: We should refactor this code to use the platform-independent 
+        // drawing API in all cases. Then, this didDraw call will happen 
+        // automatically, and we can remove it.
+        cachedImage->didDraw(image);
         return;
     }
 
index 6495505..dc2cf33 100644 (file)
@@ -172,7 +172,7 @@ void Cache::pruneLiveResources()
         ASSERT(current->referenced());
         if (current->isLoaded()) {
             // Check to see if the remaining resources are too new to prune.
-            double elapsedTime = currentTime - current->m_lastLiveAccessTime;
+            double elapsedTime = currentTime - current->m_lastDecodedAccessTime;
             if (elapsedTime < cMinDelayBeforeLiveDecodedPrune)
                 return;
 
@@ -190,7 +190,7 @@ void Cache::pruneLiveResources()
     }
 }
 
-void Cache::pruneAllResources()
+void Cache::pruneDeadResources()
 {
     // No need to prune if all of our objects fit.
     if (m_currentSize <= m_maximumSize)
@@ -254,7 +254,7 @@ void Cache::pruneAllResources()
 void Cache::setMaximumSize(unsigned bytes)
 {
     m_maximumSize = bytes;
-    pruneAllResources();
+    pruneDeadResources();
 }
 
 void Cache::remove(CachedResource* resource)
index 31f4e75..537b199 100644 (file)
 
 namespace WebCore  {
 
-class CachedCSSStyleSheet;
-class CachedImage;
 class CachedResource;
-class CachedScript;
-class CachedXSLStyleSheet;
 class DocLoader;
-class Image;
 class KURL;
 
 // This cache holds subresources used by Web pages: images, scripts, stylesheets, etc.
@@ -101,10 +96,10 @@ public:
     // Remove an existing cache entry from both the resource map and from the LRU list.
     void remove(CachedResource*);
 
-    // Flush the cache.  Any resources still referenced by Web pages will not be removed by this call.
-    void pruneAllResources();
+    // Flush dead resources (resources not referenced by Web pages).
+    void pruneDeadResources();
 
-    // Flush live resources only.
+    // Flush live resources.
     void pruneLiveResources();
 
     void addDocLoader(DocLoader*);
index bed4e1a..384d243 100644 (file)
@@ -29,7 +29,9 @@
 #include "CachedResourceClient.h"
 #include "CachedResourceClientWalker.h"
 #include "DocLoader.h"
+#include "Frame.h"
 #include "Request.h"
+#include "SystemTime.h"
 #include <wtf/Vector.h>
 
 #if PLATFORM(CG)
@@ -242,6 +244,18 @@ void CachedImage::decodedSizeChanged(const Image* image, int delta)
     }
 }
 
+void CachedImage::didDraw(const Image* image)
+{
+    if (image != m_image)
+        return;
+    
+    double timeStamp = Frame::currentPaintTimeStamp();
+    if (!timeStamp) // If didDraw is called outside of a Frame paint.
+        timeStamp = currentTime();
+    
+    CachedResource::didAccessDecodedData(timeStamp);
+}
+
 bool CachedImage::shouldPauseAnimation(const Image* image)
 {
     if (image != m_image)
index 6d81ff5..112b023 100644 (file)
@@ -35,6 +35,8 @@ class Cache;
 class Image;
 
 class CachedImage : public CachedResource, public ImageObserver {
+    friend class Cache;
+
 public:
     CachedImage(DocLoader*, const String& url, bool forCache);
     CachedImage(Image*);
@@ -65,21 +67,21 @@ public:
     
     virtual unsigned decodedSize() const;
 
-    virtual void decodedSizeChanged(const Image* image, int delta);
-
-    virtual bool shouldPauseAnimation(const Image* image);
-    virtual void animationAdvanced(const Image* image);
-
     bool stillNeedsLoad() const { return !m_errorOccurred && m_status == Unknown && m_loading == false; }
     void load();
 
+    // ImageObserver
+    virtual void decodedSizeChanged(const Image* image, int delta);
+    virtual void didDraw(const Image*);
+
+    virtual bool shouldPauseAnimation(const Image*);
+    virtual void animationAdvanced(const Image*);
+
 private:
     void createImage();
     void notifyObservers();
 
     Image* m_image;
-
-    friend class Cache;
 };
 
 }
index 337c560..217711f 100644 (file)
@@ -36,7 +36,7 @@
 namespace WebCore {
 
 CachedResource::CachedResource(const String& URL, Type type, bool forCache, bool sendResourceLoadCallbacks)
-    : m_lastLiveAccessTime(0)
+    : m_lastDecodedAccessTime(0)
     , m_sendResourceLoadCallbacks(sendResourceLoadCallbacks)
     , m_inCache(forCache)
     , m_docLoader(0)
@@ -115,7 +115,7 @@ void CachedResource::deref(CachedResourceClient *c)
         cache()->removeFromLiveResourcesSize(this);
         cache()->removeFromLiveDecodedResourcesList(this);
         allReferencesRemoved();
-        cache()->pruneAllResources();
+        cache()->pruneDeadResources();
     }
 }
 
@@ -146,11 +146,9 @@ void CachedResource::setEncodedSize(unsigned size)
     }
 }
 
-void CachedResource::liveResourceAccessed()
+void CachedResource::didAccessDecodedData(double timeStamp)
 {
-    m_lastLiveAccessTime = Frame::currentPaintTimeStamp();
-    if (!m_lastLiveAccessTime) // In liveResourceAccessed is called directly, outside of a Frame paint.
-        m_lastLiveAccessTime = currentTime();
+    m_lastDecodedAccessTime = timeStamp;
     
     if (inCache()) {
         if (m_inLiveDecodedResourcesList) {
index 03f9de0..6be9eab 100644 (file)
@@ -93,8 +93,6 @@ public:
     unsigned accessCount() const { return m_accessCount; }
     void increaseAccessCount() { m_accessCount++; }
 
-    void liveResourceAccessed();
-    
     // Computes the status of an object after loading.  
     // Updates the expire date on the cache entry file
     void finish();
@@ -136,6 +134,7 @@ public:
     
 protected:
     void setEncodedSize(unsigned);
+    void didAccessDecodedData(double timeStamp);
     
     HashCountedSet<CachedResourceClient*> m_clients;
 
@@ -155,7 +154,7 @@ private:
     unsigned m_encodedSize;
     unsigned m_accessCount;
     unsigned m_inLiveDecodedResourcesList;
-    double m_lastLiveAccessTime; // Used as a "thrash guard" in the cache
+    double m_lastDecodedAccessTime; // Used as a "thrash guard" in the cache
     
     bool m_sendResourceLoadCallbacks;
 protected:
index fc140dd..4be83bd 100644 (file)
@@ -30,12 +30,14 @@ namespace WebCore {
 
 class Image;
 
-// This class gets notified when an image advances animation frames.
+// Interface for notification about changes to an image, including decoding,
+// drawing, and animating.
 class ImageObserver {
 protected:
     virtual ~ImageObserver() {}
 public:
     virtual void decodedSizeChanged(const Image*, int delta) = 0;
+    virtual void didDraw(const Image*) = 0;
 
     virtual bool shouldPauseAnimation(const Image*) = 0;
     virtual void animationAdvanced(const Image*) = 0;
index e9992d4..eb0e880 100644 (file)
@@ -31,6 +31,7 @@
 #include "AffineTransform.h"
 #include "FloatRect.h"
 #include "GraphicsContext.h"
+#include "ImageObserver.h"
 #include "PDFDocumentImage.h"
 #include "PlatformString.h"
 #include <ApplicationServices/ApplicationServices.h>
@@ -156,11 +157,13 @@ void BitmapImage::draw(GraphicsContext* ctxt, const FloatRect& dstRect, const Fl
         }
     } else // Draw the whole image.
         CGContextDrawImage(context, ir, image);
-
+        
     ctxt->restore();
     
     startAnimation();
 
+    if (imageObserver())
+        imageObserver()->didDraw(this);
 }
 
 void Image::drawPatternCallback(void* info, CGContextRef context)
@@ -201,6 +204,9 @@ void Image::drawPattern(GraphicsContext* ctxt, const FloatRect& tileRect, const
     
     ctxt->restore();
     CGPatternRelease(pattern);
+
+    if (imageObserver())
+        imageObserver()->didDraw(this);
 }
 
 
index b53fdd6..09c6657 100644 (file)
@@ -30,6 +30,7 @@
 #if PLATFORM(CG)
 
 #include "GraphicsContext.h"
+#include "ImageObserver.h"
 #include <wtf/MathExtras.h>
 
 using namespace std;
@@ -167,8 +168,11 @@ void PDFDocumentImage::draw(GraphicsContext* context, const FloatRect& dstRect,
     // Media box may have non-zero origin which we ignore. Pass 1 for the page number.
     CGContextDrawPDFDocument(context->platformContext(), FloatRect(FloatPoint(), m_mediaBox.size()),
         m_document, m_currentPage + 1);
-
+    
     context->restore();
+
+    if (imageObserver())
+        imageObserver()->didDraw(this);
 }
 
 }
index 5c31015..4024c2e 100644 (file)
@@ -34,6 +34,7 @@
 #include "FrameLoader.h"
 #include "FrameView.h"
 #include "GraphicsContext.h"
+#include "ImageObserver.h"
 #include "NotImplemented.h"
 #include "Page.h"
 #include "ResourceError.h"
@@ -101,6 +102,9 @@ void SVGImage::draw(GraphicsContext* context, const FloatRect& dstRect, const Fl
     context->scale(FloatSize(dstRect.width()/srcRect.width(), dstRect.height()/srcRect.height()));
     m_frame->paint(context, enclosingIntRect(srcRect));
     context->restore();
+
+    if (imageObserver())
+        imageObserver()->didDraw(this);
 }
 
 NativeImagePtr SVGImage::nativeImageForCurrentFrame()
index f8f1987..13477b1 100644 (file)
@@ -684,11 +684,8 @@ void RenderBox::paintBackgroundExtended(GraphicsContext* context, const Color& c
         IntSize tileSize;
 
         calculateBackgroundImageGeometry(bgLayer, tx, ty, w, h, destRect, phase, tileSize);
-        if (!destRect.isEmpty()) {
+        if (!destRect.isEmpty())
             context->drawTiledImage(bg->image(), destRect, phase, tileSize, bgLayer->backgroundComposite());
-            if (!context->paintingDisabled())
-                bg->liveResourceAccessed();
-        }
     }
 
     if (bgLayer->backgroundClip() != BGBORDER)
index 8dcf8b1..05e372b 100644 (file)
@@ -281,8 +281,6 @@ void RenderImage::paint(PaintInfo& paintInfo, int tx, int ty)
         HTMLImageElement* imageElt = (element() && element()->hasTagName(imgTag)) ? static_cast<HTMLImageElement*>(element()) : 0;
         CompositeOperator compositeOperator = imageElt ? imageElt->compositeOperator() : CompositeSourceOver;
         context->drawImage(image(), rect, compositeOperator);
-        if (!context->paintingDisabled())
-            m_cachedImage->liveResourceAccessed();
     }
 
     // draw the selection tint even if the image itself is not available
index c5936e0..9dbb590 100644 (file)
@@ -545,8 +545,6 @@ void RenderListMarker::paint(PaintInfo& paintInfo, int tx, int ty)
             paintCustomHighlight(tx, ty, style()->highlight(), true);
 #endif
         context->drawImage(m_image->image(), marker.location());
-        if (!context->paintingDisabled())
-            m_image->liveResourceAccessed();
         if (selectionState() != SelectionNone)
             context->fillRect(selectionRect(), selectionBackgroundColor());
         return;
index b8bc70e..18d5691 100644 (file)
@@ -1216,9 +1216,6 @@ bool RenderObject::paintBorderImage(GraphicsContext* graphicsContext, int tx, in
     if (clipped)
         graphicsContext->restore();
 
-    if (!graphicsContext->paintingDisabled())
-        borderImage->liveResourceAccessed();
-
     return true;
 }