Fixing GraphicsContext state checks to support oilpan

Source/core/html/canvas/CanvasRenderingContext2D.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2007 Alp Toker <alp@atoker.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008 Dirk Schulze <krit@webkit.org>
  * Copyright (C) 2010 Torch Mobile (Beijing) Co. Ltd. All rights reserved.
  * Copyright (C) 2012, 2013 Intel Corporation. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
@@ skipping 84 matching lines @@
     , m_dispatchContextLostEventTimer(this, &CanvasRenderingContext2D::dispatchContextLostEvent)
     , m_dispatchContextRestoredEventTimer(this, &CanvasRenderingContext2D::dispatchContextRestoredEvent)
     , m_tryRestoreContextEventTimer(this, &CanvasRenderingContext2D::tryRestoreContextEvent)
 {
     m_stateStack.append(adoptPtrWillBeNoop(new State()));
     ScriptWrappable::init(this);
 }
 
 void CanvasRenderingContext2D::unwindStateStack()
 {
-    // Ensure that the state stack in the ImageBuffer's context
-    // is cleared before destruction, to avoid assertions in the
-    // GraphicsContext dtor.
-    if (size_t stackSize = m_stateStack.size()) {

[f(malita), 2014/05/27 20:28:22]

Since we're asserting that m_stateStack.size() == context->saveCount() below,
doesn't this old approach still work? We could avoid adding
GC::unwindStateStack() if it did.

[Justin Novosad, 2014/05/27 22:25:25]

Good point. GC::unwindStateStack was my fist attempt at fixing this bug, but on
its own it was not enough to ensure the coherence of the state stack.  I will
delete it.

-        if (GraphicsContext* context = canvas()->existingDrawingContext()) {
-            while (--stackSize)
-                context->restore();
-        }
+    if (GraphicsContext* context = canvas()->existingDrawingContext()) {
+        context->unwindStateStack();
     }
 }
 
 CanvasRenderingContext2D::~CanvasRenderingContext2D()
 {
-#if !ENABLE(OILPAN)
+}
+
+void CanvasRenderingContext2D::validateStateStack()
+{
 #if !ASSERT_DISABLED
-    unwindStateStack();
-#endif
+    GraphicsContext* context = canvas()->existingDrawingContext();
+    if (context && !context->contextDisabled())
+        ASSERT(context->saveCount() == m_stateStack.size());
 #endif
 }
 
 bool CanvasRenderingContext2D::isAccelerated() const
 {
     if (!canvas()->hasImageBuffer())
         return false;
     GraphicsContext* context = drawingContext();
     return context && context->isAccelerated();
 }
@@ skipping 87 matching lines @@
     reset();
     m_isContextLost = false;
     if (contextLostRestoredEventsEnabled()) {
         RefPtrWillBeRawPtr<Event> event(Event::create(EventTypeNames::contextrestored));
         canvas()->dispatchEvent(event);
     }
 }
 
 void CanvasRenderingContext2D::reset()
 {
+    validateStateStack();
     unwindStateStack();
+    // Replace the save() held by HTMLCanvasElement::m_contextStateSaver

[Stephen White, 2014/05/27 18:11:32]

Maybe it's jetlag, but I don't understand this comment, or the two lines of code
below it. How was this not causing problems before? It seems like we would
previously have had a mismatched number of saved states in GraphicsContext vs
CRC2D, after a reset, no?

If so, could we add a test for that (if it's possible)?

+    if (GraphicsContext* context = canvas()->existingDrawingContext())
+        context->save();
     m_stateStack.resize(1);
     m_stateStack.first() = adoptPtrWillBeNoop(new State());
     m_path.clear();
+    validateStateStack();
 }
 
 // Important: Several of these properties are also stored in GraphicsContext's
 // StrokeData. The default values that StrokeData uses may not the same values
 // that the canvas 2d spec specifies. Make sure to sync the initial state of the
 // GraphicsContext in HTMLCanvasElement::createImageBuffer()!
 CanvasRenderingContext2D::State::State()
     : m_unrealizedSaveCount(0)
     , m_strokeStyle(CanvasStyle::createFromRGBA(Color::black))
     , m_fillStyle(CanvasStyle::createFromRGBA(Color::black))
@@ skipping 98 matching lines @@
 void CanvasRenderingContext2D::State::fontsNeedUpdate(CSSFontSelector* fontSelector)
 {
     ASSERT_ARG(fontSelector, fontSelector == m_font.fontSelector());
     ASSERT(m_realizedFont);
 
     m_font.update(fontSelector);
 }
 
 void CanvasRenderingContext2D::realizeSaves()
 {
+    validateStateStack();
     if (state().m_unrealizedSaveCount) {
         ASSERT(m_stateStack.size() >= 1);
         // Reduce the current state's unrealized count by one now,
         // to reflect the fact we are saving one state.
         m_stateStack.last()->m_unrealizedSaveCount--;
         m_stateStack.append(adoptPtrWillBeNoop(new State(state())));
         // Set the new state's unrealized count to 0, because it has no outstanding saves.
         // We need to do this explicitly because the copy constructor and operator= used
         // by the Vector operations copy the unrealized count from the previous state (in
         // turn necessary to support correct resizing and unwinding of the stack).
         m_stateStack.last()->m_unrealizedSaveCount = 0;
         GraphicsContext* context = drawingContext();
+        ASSERT(context);
         if (context)
             context->save();
+        validateStateStack();
     }
 }
 
 void CanvasRenderingContext2D::restore()
 {
+    validateStateStack();
     if (state().m_unrealizedSaveCount) {
         // We never realized the save, so just record that it was unnecessary.
         --m_stateStack.last()->m_unrealizedSaveCount;
         return;
     }
     ASSERT(m_stateStack.size() >= 1);
     if (m_stateStack.size() <= 1)
         return;
     m_path.transform(state().m_transform);
     m_stateStack.removeLast();
     m_path.transform(state().m_transform.inverse());
     GraphicsContext* c = drawingContext();
     if (c)
         c->restore();
+    validateStateStack();
 }
 
 CanvasStyle* CanvasRenderingContext2D::strokeStyle() const
 {
     return state().m_strokeStyle.get();
 }
 
 void CanvasRenderingContext2D::setStrokeStyle(PassRefPtr<CanvasStyle> prpStyle)
 {
     RefPtr<CanvasStyle> style = prpStyle;
@@ skipping 839 matching lines @@
         if (!saved) {
             context->save();
             saved = true;
         }
         context->setCompositeOperation(CompositeSourceOver);
     }
     context->clearRect(rect);
     if (saved)
         context->restore();
 
+    validateStateStack();
     didDraw(dirtyRect);
 }
 
 void CanvasRenderingContext2D::fillRect(float x, float y, float width, float height)
 {
     if (!validateRectForCanvas(x, y, width, height))
         return;
 
     GraphicsContext* c = drawingContext();
     if (!c)
@@ skipping 275 matching lines @@
 void CanvasRenderingContext2D::drawVideo(HTMLVideoElement* video, FloatRect srcRect, FloatRect dstRect)
 {
     GraphicsContext* c = drawingContext();
     GraphicsContextStateSaver stateSaver(*c);
     c->clip(dstRect);
     c->translate(dstRect.x(), dstRect.y());
     c->scale(FloatSize(dstRect.width() / srcRect.width(), dstRect.height() / srcRect.height()));
     c->translate(-srcRect.x(), -srcRect.y());
     video->paintCurrentFrameInContext(c, IntRect(IntPoint(), IntSize(video->videoWidth(), video->videoHeight())));
     stateSaver.restore();
+    validateStateStack();
 }
 
 void CanvasRenderingContext2D::drawImageFromRect(HTMLImageElement* image,
     float sx, float sy, float sw, float sh,
     float dx, float dy, float dw, float dh,
     const String& compositeOperation)
 {
     if (!image)
         return;
     CompositeOperator op;
@@ skipping 817 matching lines @@
     FloatRect dirtyRect;
     if (!computeDirtyRect(path.strokeBoundingRect(strokeData), &dirtyRect))
         return;
 
     c->save();
     c->setAlphaAsFloat(1.0);
     c->clearShadow();
     c->setCompositeOperation(CompositeSourceOver, blink::WebBlendModeNormal);
     c->drawFocusRing(path, focusRingWidth, focusRingOutline, focusRingColor);
     c->restore();
-
+    validateStateStack();
     didDraw(dirtyRect);
 }
 
 } // namespace WebCore