Fixing GraphicsContext state checks to support oilpan

Source/core/html/HTMLCanvasElement.cpp

 /*
  * Copyright (C) 2004, 2006, 2007 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alp Toker <alp@atoker.com>
  * Copyright (C) 2010 Torch Mobile (Beijing) Co. Ltd. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
@@ skipping 79 matching lines @@
 }
 
 HTMLCanvasElement::~HTMLCanvasElement()
 {
     v8::Isolate::GetCurrent()->AdjustAmountOfExternalAllocatedMemory(-m_externallyAllocatedMemory);
     HashSet<CanvasObserver*>::iterator end = m_observers.end();
     for (HashSet<CanvasObserver*>::iterator it = m_observers.begin(); it != end; ++it)
         (*it)->canvasDestroyed(this);
 
 #if !ENABLE(OILPAN)
-    m_context.clear(); // Ensure this goes away before the ImageBuffer.
+    // Ensure these go away before the ImageBuffer.
+    m_contextStateSaver.clear();
+    m_context.clear();
 #endif
 }
 
 void HTMLCanvasElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == widthAttr || name == heightAttr)
         reset();
     HTMLElement::parseAttribute(name, value);
 }
 
@@ skipping 378 matching lines @@
     if (!surface->isValid())
         return;
 
     m_imageBuffer = ImageBuffer::create(surface.release());
     m_imageBuffer->setClient(this);
 
     m_didFailToCreateImageBuffer = false;
 
     updateExternallyAllocatedMemory();
 
     if (is3D()) {

[Mads Ager (chromium), 2014/05/22 05:53:03]

Is it possible to get here where m_context has not been created yet? Line 524
below checks for 'if (m_context)' which indicates that it is possible. Is it OK
to treat this as a 2D CanvasElement if m_context is 0 here? In other words: is
it OK to go through the steps below that sets things up for a 2D CanvasElement
and then later create a m_context which is 3D?

[Justin Novosad, 2014/05/26 17:17:51]

Not supposed to.

Line 524
That check was added very recently in Blink r173363. I would argue that it is
probably not necessary

         // Early out for WebGL canvases
         return;
     }
 
     m_imageBuffer->setClient(this);
     m_imageBuffer->context()->setShouldClampToSourceRect(false);
     m_imageBuffer->context()->disableAntialiasingOptimizationForHairlineImages();
     m_imageBuffer->context()->setImageInterpolationQuality(CanvasDefaultInterpolationQuality);
     // Enabling MSAA overrides a request to disable antialiasing. This is true regardless of whether the
     // rendering mode is accelerated or not. For consistency, we don't want to apply AA in accelerated
     // canvases but not in unaccelerated canvases.
     if (!msaaSampleCount && document().settings() && !document().settings()->antialiased2dCanvasEnabled())
         m_imageBuffer->context()->setShouldAntialias(false);
     // GraphicsContext's defaults don't always agree with the 2d canvas spec.
     // See CanvasRenderingContext2D::State::State() for more information.
     m_imageBuffer->context()->setMiterLimit(10);
     m_imageBuffer->context()->setStrokeThickness(1);
+#if !ASSERT_DISABLED
+    m_imageBuffer->context()->disableDestructionChecks(); // 2D canvas is allowed to leave context in an unfinalized state.

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

If we have exclusive control of the initialization path, I would prefer this to
be a GraphicsContext constructor flag. But it's unclear whether that's feasible.

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

Right no ImageBuffer is not explicitly aware that it is owned by a 2D canvas,
but we could still have a construction flag relayed through ImageBuffer.

+#endif
     m_contextStateSaver = adoptPtr(new GraphicsContextStateSaver(*m_imageBuffer->context()));
 
     if (m_context)
         setNeedsCompositingUpdate();
 }
 
 void HTMLCanvasElement::notifySurfaceInvalid()
 {
     if (m_context && m_context->is2d()) {
         CanvasRenderingContext2D* context2d = toCanvasRenderingContext2D(m_context.get());
@@ skipping 166 matching lines @@
 {
     return !originClean();
 }
 
 FloatSize HTMLCanvasElement::sourceSize() const
 {
     return FloatSize(width(), height());
 }
 
 }