Fix ASSERT(m_canvas) failures when contextDisabled()

Source/platform/graphics/GraphicsContext.cpp

Index: Source/platform/graphics/GraphicsContext.cpp
diff --git a/Source/platform/graphics/GraphicsContext.cpp b/Source/platform/graphics/GraphicsContext.cpp
index de87ce856ffeed8e9169984eb310490db4742c85..6f6eca41d6599ae890be7f9a5304037f85dd4d92 100644
--- a/Source/platform/graphics/GraphicsContext.cpp
+++ b/Source/platform/graphics/GraphicsContext.cpp
@@ -53,6 +53,7 @@
 #include "third_party/skia/include/effects/SkPictureImageFilter.h"
 #include "third_party/skia/include/gpu/GrRenderTarget.h"
 #include "third_party/skia/include/gpu/GrTexture.h"
+#include "third_party/skia/include/utils/SkNullCanvas.h"
 #include "wtf/Assertions.h"
 #include "wtf/MathExtras.h"
 
@@ -107,6 +108,11 @@ GraphicsContext::GraphicsContext(SkCanvas* canvas, DisplayItemList* displayItemL
     // several here.
     m_paintStateStack.append(GraphicsContextState::create());
     m_paintState = m_paintStateStack.last().get();
+
+    if (contextDisabled()) {
+        DEFINE_STATIC_LOCAL(RefPtr<SkCanvas>, nullCanvas, (adoptRef(SkCreateNullCanvas())));
+        m_canvas = nullCanvas.get();
+    }
 }
 
 GraphicsContext::~GraphicsContext()
@@ -550,6 +556,8 @@ bool GraphicsContext::isRecording() const
 
 void GraphicsContext::drawPicture(const SkPicture* picture)
 {
+    if (contextDisabled())

[f(malita), 2015/03/10 19:34:24]

Already checked below. Probably just need to relocate the assert.

+        return;
     ASSERT(m_canvas);
 
     // FIXME: SP currently builds empty-bounds pictures in some cases. This is a temp
@@ -564,9 +572,9 @@ void GraphicsContext::drawPicture(const SkPicture* picture)
 
 void GraphicsContext::compositePicture(SkPicture* picture, const FloatRect& dest, const FloatRect& src, SkXfermode::Mode op)
 {
-    ASSERT(m_canvas);
     if (contextDisabled() || !picture)
         return;
+    ASSERT(m_canvas);
 
     SkPaint picturePaint;
     picturePaint.setXfermodeMode(op);
@@ -760,9 +768,9 @@ void GraphicsContext::drawInnerShadow(const FloatRoundedRect& rect, const Color&
 
 void GraphicsContext::drawLine(const IntPoint& point1, const IntPoint& point2)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     StrokeStyle penStyle = strokeStyle();
     if (penStyle == NoStroke)
@@ -1096,16 +1104,19 @@ void GraphicsContext::drawImageBuffer(ImageBuffer* image, const FloatRect& dest,
 
 void GraphicsContext::writePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes, int x, int y)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->writePixels(info, pixels, rowBytes, x, y);
 }
 
 void GraphicsContext::drawBitmap(const SkBitmap& bitmap, SkScalar left, SkScalar top, const SkPaint* paint)

[f(malita), 2015/03/10 19:34:24]

I don't think this method is used at all - we should probably get rid of it.

[Xianzhu, 2015/03/11 16:45:10]

I'd leave this to another change.

 {
+    if (contextDisabled())

[f(malita), 2015/03/10 19:34:24]

Already checked below. Relocate the assert?

[Xianzhu, 2015/03/11 16:45:10]

Done.

+        return;
     ASSERT(m_canvas);
+
     // Textures are bound to the blink main-thread GrContext, which can not be
     // used on the compositor raster thread.
     // FIXME: Mailbox support would make this possible in the GPU-raster case.
@@ -1119,7 +1130,10 @@ void GraphicsContext::drawBitmap(const SkBitmap& bitmap, SkScalar left, SkScalar
 void GraphicsContext::drawBitmapRect(const SkBitmap& bitmap, const SkRect* src,
     const SkRect& dst, const SkPaint* paint)
 {
+    if (contextDisabled())

[f(malita), 2015/03/10 19:34:24]

Note that some of these methods are "internal" entry points (they are only ever
reached by calling other GC methods) - so the check is redundant. In this case,
I believe we always go through drawImage() first.

(it used to be that Skia types in the signature were a good indication of
internal/guarded API, but not so much recently)

+        return;
     ASSERT(m_canvas);
+
     // Textures are bound to the blink main-thread GrContext, which can not be
     // used on the compositor raster thread.
     // FIXME: Mailbox support would make this possible in the GPU-raster case.
@@ -1135,54 +1149,54 @@ void GraphicsContext::drawBitmapRect(const SkBitmap& bitmap, const SkRect* src,
 
 void GraphicsContext::drawImage(const SkImage* image, SkScalar left, SkScalar top, const SkPaint* paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);

[f(malita), 2015/03/10 19:34:24]

Since m_disabledState is effectively const for the lifetime of a GC (we should
probably mark it as such too), we could assert once in the ctor instead of
everywhere.

Although the latter may be desirable for documentation purposes, so up to you.

[Xianzhu, 2015/03/11 16:45:10]

This assert is also for m_disabledState==false. m_canvas can change during the
lifetime of GC on beginRecording/endRecording/resetCanvas.

 
     m_canvas->drawImage(image, left, top, paint);
 }
 
 void GraphicsContext::drawImageRect(const SkImage* image, const SkRect* src, const SkRect& dst, const SkPaint* paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawImageRect(image, src, dst, paint);
 }
 
 void GraphicsContext::drawOval(const SkRect& oval, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawOval(oval, paint);
 }
 
 void GraphicsContext::drawPath(const SkPath& path, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawPath(path, paint);
 }
 
 void GraphicsContext::drawRect(const SkRect& rect, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawRect(rect, paint);
 }
 
 void GraphicsContext::drawRRect(const SkRRect& rrect, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawRRect(rrect, paint);
 }
@@ -1190,9 +1204,9 @@ void GraphicsContext::drawRRect(const SkRRect& rrect, const SkPaint& paint)
 void GraphicsContext::drawPosText(const void* text, size_t byteLength,
     const SkPoint pos[], const SkRect& textRect, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawPosText(text, byteLength, pos, paint);
     didDrawTextInRect(textRect);
@@ -1201,9 +1215,9 @@ void GraphicsContext::drawPosText(const void* text, size_t byteLength,
 void GraphicsContext::drawPosTextH(const void* text, size_t byteLength,
     const SkScalar xpos[], SkScalar constY, const SkRect& textRect, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawPosTextH(text, byteLength, xpos, constY, paint);
     didDrawTextInRect(textRect);
@@ -1211,9 +1225,9 @@ void GraphicsContext::drawPosTextH(const void* text, size_t byteLength,
 
 void GraphicsContext::drawTextBlob(const SkTextBlob* blob, const SkPoint& origin, const SkPaint& paint)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->drawTextBlob(blob, origin.x(), origin.y(), paint);
 
@@ -1263,9 +1277,9 @@ void GraphicsContext::fillRect(const FloatRect& rect, const Color& color)
 void GraphicsContext::fillBetweenRoundedRects(const FloatRect& outer, const FloatSize& outerTopLeft, const FloatSize& outerTopRight, const FloatSize& outerBottomLeft, const FloatSize& outerBottomRight,
     const FloatRect& inner, const FloatSize& innerTopLeft, const FloatSize& innerTopRight, const FloatSize& innerBottomLeft, const FloatSize& innerBottomRight, const Color& color)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     SkVector outerRadii[4];
     SkVector innerRadii[4];
@@ -1292,9 +1306,9 @@ void GraphicsContext::fillBetweenRoundedRects(const FloatRoundedRect& outer, con
 void GraphicsContext::fillRoundedRect(const FloatRect& rect, const FloatSize& topLeft, const FloatSize& topRight,
     const FloatSize& bottomLeft, const FloatSize& bottomRight, const Color& color)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     if (topLeft.width() + topRight.width() > rect.width()
             || bottomLeft.width() + bottomRight.width() > rect.width()
@@ -1449,45 +1463,45 @@ void GraphicsContext::clipOutRoundedRect(const FloatRoundedRect& rect)
 
 void GraphicsContext::clipRect(const SkRect& rect, AntiAliasingMode aa, SkRegion::Op op)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->clipRect(rect, op, aa == AntiAliased);
 }
 
 void GraphicsContext::clipPath(const SkPath& path, AntiAliasingMode aa, SkRegion::Op op)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->clipPath(path, op, aa == AntiAliased);
 }
 
 void GraphicsContext::clipRRect(const SkRRect& rect, AntiAliasingMode aa, SkRegion::Op op)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->clipRRect(rect, op, aa == AntiAliased);
 }
 
 void GraphicsContext::rotate(float angleInRadians)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     m_canvas->rotate(WebCoreFloatToSkScalar(angleInRadians * (180.0f / 3.14159265f)));
 }
 
 void GraphicsContext::translate(float x, float y)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     if (!x && !y)
         return;
@@ -1497,9 +1511,9 @@ void GraphicsContext::translate(float x, float y)
 
 void GraphicsContext::scale(float x, float y)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     if (x == 1.0f && y == 1.0f)
         return;
@@ -1509,9 +1523,9 @@ void GraphicsContext::scale(float x, float y)
 
 void GraphicsContext::setURLForRect(const KURL& link, const IntRect& destRect)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     SkAutoDataUnref url(SkData::NewWithCString(link.string().utf8().data()));
     SkAnnotateRectWithURL(m_canvas, destRect, url.get());
@@ -1519,9 +1533,9 @@ void GraphicsContext::setURLForRect(const KURL& link, const IntRect& destRect)
 
 void GraphicsContext::setURLFragmentForRect(const String& destName, const IntRect& rect)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     SkAutoDataUnref skDestName(SkData::NewWithCString(destName.utf8().data()));
     SkAnnotateLinkToDestination(m_canvas, rect, skDestName.get());
@@ -1529,9 +1543,9 @@ void GraphicsContext::setURLFragmentForRect(const String& destName, const IntRec
 
 void GraphicsContext::addURLTargetAtPoint(const String& name, const IntPoint& pos)
 {
-    ASSERT(m_canvas);
     if (contextDisabled())
         return;
+    ASSERT(m_canvas);
 
     SkAutoDataUnref nameData(SkData::NewWithCString(name.utf8().data()));
     SkAnnotateNamedDestination(m_canvas, SkPoint::Make(pos.x(), pos.y()), nameData);