Let SkPictureBuilder be DisplayItemClient

Some objects implement DisplayItemClient just because they are painted on SkPictureBuilder. Let SkPictureBuilder be DisplayItemClient can reduce number of DisplayItemClient implementations. We don't need to bother their implementation of visualRect() and future client-side caching status methods. Review-Url: https://codereview.chromium.org/1931133002 Cr-Commit-Position: refs/heads/master@{#390572}
marcosholgado · Apr 29, 2016 · 6684994 · 6684994
1 parent bd92f38
commit 6684994
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 57 deletions.
diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.cpp b/third_party/WebKit/Source/core/frame/LocalFrame.cpp
@@ -117,7 +117,7 @@ class DragImageBuilder {
         AffineTransform transform;
         transform.scale(deviceScaleFactor, deviceScaleFactor);
         transform.translate(-m_bounds.x(), -m_bounds.y());
-        context().getPaintController().createAndAppend<BeginTransformDisplayItem>(*m_localFrame, transform);
+        context().getPaintController().createAndAppend<BeginTransformDisplayItem>(*m_pictureBuilder, transform);
     }
 
     GraphicsContext& context() { return m_pictureBuilder->context(); }
@@ -126,7 +126,7 @@ class DragImageBuilder {
     {
         if (m_draggedNode && m_draggedNode->layoutObject())
             m_draggedNode->layoutObject()->updateDragState(false);
-        context().getPaintController().endItem<EndTransformDisplayItem>(*m_localFrame);
+        context().getPaintController().endItem<EndTransformDisplayItem>(*m_pictureBuilder);
         RefPtr<const SkPicture> recording = m_pictureBuilder->endRecording();
         RefPtr<SkImage> skImage = adoptRef(SkImage::NewFromPicture(recording.get(),
             SkISize::Make(m_bounds.width(), m_bounds.height()), nullptr, nullptr));

diff --git a/third_party/WebKit/Source/core/frame/LocalFrame.h b/third_party/WebKit/Source/core/frame/LocalFrame.h
@@ -38,7 +38,6 @@
 #include "core/paint/PaintPhase.h"
 #include "platform/Supplementable.h"
 #include "platform/graphics/ImageOrientation.h"
-#include "platform/graphics/paint/DisplayItem.h"
 #include "platform/heap/Handle.h"
 #include "platform/scroll/ScrollTypes.h"
 #include "wtf/HashSet.h"
@@ -78,7 +77,7 @@ class WebFrameHostScheduler;
 class WebFrameScheduler;
 template <typename Strategy> class PositionWithAffinityTemplate;
 
-class CORE_EXPORT LocalFrame : public Frame, public LocalFrameLifecycleNotifier, public Supplementable<LocalFrame>, public DisplayItemClient {
+class CORE_EXPORT LocalFrame : public Frame, public LocalFrameLifecycleNotifier, public Supplementable<LocalFrame> {
     USING_GARBAGE_COLLECTED_MIXIN(LocalFrame);
 public:
     static LocalFrame* create(FrameLoaderClient*, FrameHost*, FrameOwner*, ServiceRegistry* = nullptr);
@@ -171,11 +170,6 @@ class CORE_EXPORT LocalFrame : public Frame, public LocalFrameLifecycleNotifier,
     bool shouldReuseDefaultView(const KURL&) const;
     void removeSpellingMarkersUnderWords(const Vector<String>& words);
 
-    // DisplayItemClient methods
-    String debugName() const final { return "LocalFrame"; }
-    // TODO(chrishtr): fix this.
-    LayoutRect visualRect() const override { return LayoutRect(); }
-
     bool shouldThrottleRendering() const;
 
     // Returns the frame scheduler, creating one if needed.

diff --git a/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp b/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
@@ -79,12 +79,6 @@ SVGImage::~SVGImage()
     ASSERT(!m_chromeClient || !m_chromeClient->image());
 }
 
-LayoutRect SVGImage::visualRect() const
-{
-    // TODO(chrishtr): fix this.
-    return LayoutRect();
-}
-
 bool SVGImage::isInSVGImage(const Node* node)
 {
     ASSERT(node);
@@ -283,8 +277,8 @@ void SVGImage::drawPatternForContainer(GraphicsContext& context, const FloatSize
     spacedTile.expand(FloatSize(repeatSpacing));
 
     SkPictureBuilder patternPicture(spacedTile, nullptr, &context);
-    if (!DrawingRecorder::useCachedDrawingIfPossible(patternPicture.context(), *this, DisplayItem::Type::SVGImage)) {
-        DrawingRecorder patternPictureRecorder(patternPicture.context(), *this, DisplayItem::Type::SVGImage, spacedTile);
+    {
+        DrawingRecorder patternPictureRecorder(patternPicture.context(), patternPicture, DisplayItem::Type::SVGImage, spacedTile);
         // When generating an expanded tile, make sure we don't draw into the spacing area.
         if (tile != spacedTile)
             patternPicture.context().clip(tile);
@@ -352,7 +346,7 @@ void SVGImage::drawInternal(SkCanvas* canvas, const SkPaint& paint, const FloatR
 
     SkPictureBuilder imagePicture(dstRect);
     {
-        ClipRecorder clipRecorder(imagePicture.context(), *this, DisplayItem::ClipNodeImage, LayoutRect(enclosingIntRect(dstRect)));
+        ClipRecorder clipRecorder(imagePicture.context(), imagePicture, DisplayItem::ClipNodeImage, LayoutRect(enclosingIntRect(dstRect)));
 
         // We can only draw the entire frame, clipped to the rect we want. So compute where the top left
         // of the image would be if we were drawing without clipping, and translate accordingly.
@@ -361,7 +355,7 @@ void SVGImage::drawInternal(SkCanvas* canvas, const SkPaint& paint, const FloatR
         FloatPoint destOffset = dstRect.location() - topLeftOffset;
         AffineTransform transform = AffineTransform::translation(destOffset.x(), destOffset.y());
         transform.scale(scale.width(), scale.height());
-        TransformRecorder transformRecorder(imagePicture.context(), *this, transform);
+        TransformRecorder transformRecorder(imagePicture.context(), imagePicture, transform);
 
         view->updateAllLifecyclePhasesExceptPaint();
         view->paint(imagePicture.context(), CullRect(enclosingIntRect(srcRect)));

diff --git a/third_party/WebKit/Source/core/svg/graphics/SVGImage.h b/third_party/WebKit/Source/core/svg/graphics/SVGImage.h
@@ -28,7 +28,6 @@
 #define SVGImage_h
 
 #include "platform/graphics/Image.h"
-#include "platform/graphics/paint/DisplayItemClient.h"
 #include "platform/heap/Handle.h"
 #include "platform/weborigin/KURL.h"
 #include "wtf/Allocator.h"
@@ -42,7 +41,7 @@ class LayoutReplaced;
 class SVGImageChromeClient;
 class SVGImageForContainer;
 
-class SVGImage final : public Image, public DisplayItemClient {
+class SVGImage final : public Image {
 public:
     static PassRefPtr<SVGImage> create(ImageObserver* observer)
     {
@@ -82,10 +81,6 @@ class SVGImage final : public Image, public DisplayItemClient {
     // thus also independent of current zoom level.
     FloatSize concreteObjectSize(const FloatSize& defaultObjectSize) const;
 
-    // DisplayItemClient methods.
-    String debugName() const final { return "SVGImage"; }
-    LayoutRect visualRect() const override;
-
     bool hasIntrinsicDimensions() const;
 
 private:

diff --git a/third_party/WebKit/Source/platform/exported/WebFont.cpp b/third_party/WebKit/Source/platform/exported/WebFont.cpp
@@ -8,7 +8,6 @@
 #include "platform/fonts/FontCache.h"
 #include "platform/fonts/FontDescription.h"
 #include "platform/graphics/GraphicsContext.h"
-#include "platform/graphics/paint/DisplayItemClient.h"
 #include "platform/graphics/paint/DrawingRecorder.h"
 #include "platform/graphics/paint/SkPictureBuilder.h"
 #include "platform/text/TextRun.h"
@@ -25,7 +24,7 @@ WebFont* WebFont::create(const WebFontDescription& description)
     return new WebFont(description);
 }
 
-class WebFont::Impl final : public DisplayItemClient {
+class WebFont::Impl final {
 public:
     explicit Impl(const WebFontDescription& description)
         : m_font(description)
@@ -34,12 +33,6 @@ class WebFont::Impl final : public DisplayItemClient {
     }
 
     const Font& getFont() const { return m_font; }
-    String debugName() const final { return "WebFont::Impl"; }
-    LayoutRect visualRect() const final
-    {
-        // TODO(chrishtr): fix this.
-        return LayoutRect();
-    }
 
 private:
     Font m_font;
@@ -98,9 +91,8 @@ void WebFont::drawText(WebCanvas* canvas, const WebTextRun& run,
     SkPictureBuilder pictureBuilder(intRect);
     GraphicsContext& context = pictureBuilder.context();
 
-    ASSERT(!DrawingRecorder::useCachedDrawingIfPossible(context, *m_private, DisplayItem::WebFont));
     {
-        DrawingRecorder drawingRecorder(context, *m_private, DisplayItem::WebFont, intRect);
+        DrawingRecorder drawingRecorder(context, pictureBuilder, DisplayItem::WebFont, intRect);
         context.save();
         context.setFillColor(color);
         context.clip(textClipRect);

diff --git a/third_party/WebKit/Source/platform/graphics/paint/SkPictureBuilder.h b/third_party/WebKit/Source/platform/graphics/paint/SkPictureBuilder.h
@@ -7,6 +7,7 @@
 
 #include "platform/PlatformExport.h"
 #include "platform/geometry/FloatRect.h"
+#include "platform/graphics/paint/DisplayItemClient.h"
 #include "wtf/Noncopyable.h"
 #include "wtf/OwnPtr.h"
 #include "wtf/PassRefPtr.h"
@@ -21,7 +22,7 @@ class PaintController;
 
 // When slimming paint ships we can remove this SkPicture abstraction and
 // rely on PaintController here.
-class PLATFORM_EXPORT SkPictureBuilder final {
+class PLATFORM_EXPORT SkPictureBuilder final : public DisplayItemClient {
     WTF_MAKE_NONCOPYABLE(SkPictureBuilder);
 public:
     // Constructs a new builder with the given bounds for the resulting recorded picture. If
@@ -37,6 +38,10 @@ class PLATFORM_EXPORT SkPictureBuilder final {
     // construction.
     PassRefPtr<SkPicture> endRecording();
 
+    // DisplayItemClient methods
+    String debugName() const final { return "SkPictureBuilder"; }
+    LayoutRect visualRect() const final { return LayoutRect(); }
+
 private:
     OwnPtr<PaintController> m_paintController;
     OwnPtr<GraphicsContext> m_context;

diff --git a/third_party/WebKit/Source/web/PageWidgetDelegate.cpp b/third_party/WebKit/Source/web/PageWidgetDelegate.cpp
@@ -80,16 +80,15 @@ static void paintInternal(Page& page, WebCanvas* canvas,
 
         AffineTransform scale;
         scale.scale(scaleFactor);
-        TransformRecorder scaleRecorder(paintContext, root, scale);
+        TransformRecorder scaleRecorder(paintContext, pictureBuilder, scale);
 
         IntRect dirtyRect(rect);
         FrameView* view = root.view();
         if (view) {
-            ClipRecorder clipRecorder(paintContext, root, DisplayItem::PageWidgetDelegateClip, LayoutRect(dirtyRect));
-
+            ClipRecorder clipRecorder(paintContext, pictureBuilder, DisplayItem::PageWidgetDelegateClip, LayoutRect(dirtyRect));
             view->paint(paintContext, globalPaintFlags, CullRect(dirtyRect));
-        } else if (!DrawingRecorder::useCachedDrawingIfPossible(paintContext, root, DisplayItem::PageWidgetDelegateBackgroundFallback)) {
-            DrawingRecorder drawingRecorder(paintContext, root, DisplayItem::PageWidgetDelegateBackgroundFallback, dirtyRect);
+        } else {
+            DrawingRecorder drawingRecorder(paintContext, pictureBuilder, DisplayItem::PageWidgetDelegateBackgroundFallback, dirtyRect);
             paintContext.fillRect(dirtyRect, Color::white);
         }
     }

diff --git a/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp b/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
@@ -271,7 +271,7 @@ WebPluginContainerImpl* WebLocalFrameImpl::pluginContainerFromNode(LocalFrame* f
 
 // Simple class to override some of PrintContext behavior. Some of the methods
 // made virtual so that they can be overridden by ChromePluginPrintContext.
-class ChromePrintContext : public PrintContext, public DisplayItemClient {
+class ChromePrintContext : public PrintContext {
     WTF_MAKE_NONCOPYABLE(ChromePrintContext);
 public:
     ChromePrintContext(LocalFrame* frame)
@@ -309,7 +309,7 @@ class ChromePrintContext : public PrintContext, public DisplayItemClient {
         SkPictureBuilder pictureBuilder(pageRect, &skia::GetMetaData(*canvas));
         pictureBuilder.context().setPrinting(true);
 
-        float scale = spoolPage(pictureBuilder.context(), pageNumber);
+        float scale = spoolPage(pictureBuilder, pageNumber);
         pictureBuilder.endRecording()->playback(canvas);
         return scale;
     }
@@ -339,7 +339,7 @@ class ChromePrintContext : public PrintContext, public DisplayItemClient {
 
         // Fill the whole background by white.
         {
-            DrawingRecorder backgroundRecorder(context, *this, DisplayItem::PrintedContentBackground, allPagesRect);
+            DrawingRecorder backgroundRecorder(context, pictureBuilder, DisplayItem::PrintedContentBackground, allPagesRect);
             context.fillRect(FloatRect(0, 0, pageWidth, totalHeight), Color::white);
         }
 
@@ -348,7 +348,7 @@ class ChromePrintContext : public PrintContext, public DisplayItemClient {
             ScopeRecorder scopeRecorder(context);
             // Draw a line for a page boundary if this isn't the first page.
             if (pageIndex > 0) {
-                DrawingRecorder lineBoundaryRecorder(context, *this, DisplayItem::PrintedContentLineBoundary, allPagesRect);
+                DrawingRecorder lineBoundaryRecorder(context, pictureBuilder, DisplayItem::PrintedContentLineBoundary, allPagesRect);
                 context.save();
                 context.setStrokeColor(Color(0, 0, 255));
                 context.setFillColor(Color(0, 0, 255));
@@ -364,42 +364,39 @@ class ChromePrintContext : public PrintContext, public DisplayItemClient {
             float scale = getPageShrink(pageIndex);
             transform.scale(scale, scale);
 #endif
-            TransformRecorder transformRecorder(context, *this, transform);
-            spoolPage(context, pageIndex);
+            TransformRecorder transformRecorder(context, pictureBuilder, transform);
+            spoolPage(pictureBuilder, pageIndex);
 
             currentHeight += pageSizeInPixels.height() + 1;
         }
         pictureBuilder.endRecording()->playback(canvas);
     }
 
-    // DisplayItemClient methods
-    String debugName() const final { return "ChromePrintContext"; }
-    LayoutRect visualRect() const override { return LayoutRect(); }
-
 protected:
     // Spools the printed page, a subrect of frame(). Skip the scale step.
     // NativeTheme doesn't play well with scaling. Scaling is done browser side
     // instead. Returns the scale to be applied.
     // On Linux, we don't have the problem with NativeTheme, hence we let WebKit
     // do the scaling and ignore the return value.
-    virtual float spoolPage(GraphicsContext& context, int pageNumber)
+    virtual float spoolPage(SkPictureBuilder& pictureBuilder, int pageNumber)
     {
         IntRect pageRect = m_pageRects[pageNumber];
         float scale = m_printedPageWidth / pageRect.width();
+        GraphicsContext& context = pictureBuilder.context();
 
         AffineTransform transform;
 #if OS(POSIX) && !OS(MACOSX)
         transform.scale(scale);
 #endif
         transform.translate(static_cast<float>(-pageRect.x()), static_cast<float>(-pageRect.y()));
-        TransformRecorder transformRecorder(context, *this, transform);
+        TransformRecorder transformRecorder(context, pictureBuilder, transform);
 
-        ClipRecorder clipRecorder(context, *this, DisplayItem::ClipPrintedPage, LayoutRect(pageRect));
+        ClipRecorder clipRecorder(context, pictureBuilder, DisplayItem::ClipPrintedPage, LayoutRect(pageRect));
 
         frame()->view()->paintContents(context, GlobalPaintNormalPhase, pageRect);
 
         {
-            DrawingRecorder lineBoundaryRecorder(context, *this, DisplayItem::PrintedContentDestinationLocations, pageRect);
+            DrawingRecorder lineBoundaryRecorder(context, pictureBuilder, DisplayItem::PrintedContentDestinationLocations, pageRect);
             outputLinkedDestinations(context, pageRect);
         }
 
@@ -471,10 +468,10 @@ class ChromePluginPrintContext final : public ChromePrintContext {
     // Spools the printed page, a subrect of frame(). Skip the scale step.
     // NativeTheme doesn't play well with scaling. Scaling is done browser side
     // instead. Returns the scale to be applied.
-    float spoolPage(GraphicsContext& context, int pageNumber) override
+    float spoolPage(SkPictureBuilder& pictureBuilder, int pageNumber) override
     {
         IntRect pageRect = m_pageRects[pageNumber];
-        m_plugin->printPage(pageNumber, context, pageRect);
+        m_plugin->printPage(pageNumber, pictureBuilder.context(), pageRect);
 
         return 1.0;
     }