Diff
Modified: trunk/Source/WebCore/ChangeLog (94362 => 94363)
--- trunk/Source/WebCore/ChangeLog 2011-09-01 23:41:12 UTC (rev 94362)
+++ trunk/Source/WebCore/ChangeLog 2011-09-01 23:45:02 UTC (rev 94363)
@@ -1,3 +1,39 @@
+2011-08-31 Adrienne Walker <[email protected]>
+
+ [chromium] Remove unsafe raw GraphicsContext3D pointer from ProgramBinding
+ https://bugs.webkit.org/show_bug.cgi?id=67003
+
+ Reviewed by James Robinson.
+
+ ProgramBinding now takes an explicit cleanup call to destroy its
+ resources. This will assert if it is not called and will leak no
+ longer potentially dereference a dead pointer.
+
+ * platform/graphics/chromium/LayerRendererChromium.cpp:
+ (WebCore::LayerRendererChromium::borderProgram):
+ (WebCore::LayerRendererChromium::headsUpDisplayProgram):
+ (WebCore::LayerRendererChromium::renderSurfaceProgram):
+ (WebCore::LayerRendererChromium::renderSurfaceMaskProgram):
+ (WebCore::LayerRendererChromium::tilerProgram):
+ (WebCore::LayerRendererChromium::tilerProgramSwizzle):
+ (WebCore::LayerRendererChromium::tilerProgramAA):
+ (WebCore::LayerRendererChromium::tilerProgramSwizzleAA):
+ (WebCore::LayerRendererChromium::canvasLayerProgram):
+ (WebCore::LayerRendererChromium::pluginLayerProgram):
+ (WebCore::LayerRendererChromium::videoLayerRGBAProgram):
+ (WebCore::LayerRendererChromium::videoLayerYUVProgram):
+ (WebCore::LayerRendererChromium::cleanupSharedObjects):
+ * platform/graphics/chromium/ProgramBinding.cpp:
+ (WebCore::ProgramBindingBase::ProgramBindingBase):
+ (WebCore::ProgramBindingBase::~ProgramBindingBase):
+ (WebCore::ProgramBindingBase::init):
+ (WebCore::ProgramBindingBase::cleanup):
+ (WebCore::ProgramBindingBase::loadShader):
+ (WebCore::ProgramBindingBase::createShaderProgram):
+ * platform/graphics/chromium/ProgramBinding.h:
+ (WebCore::ProgramBinding::ProgramBinding):
+ (WebCore::ProgramBinding::initialize):
+
2011-09-01 Patrick Gansterer <[email protected]>
Don't include unnecessary headers in V8 bindings
Modified: trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp (94362 => 94363)
--- trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2011-09-01 23:41:12 UTC (rev 94362)
+++ trunk/Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp 2011-09-01 23:45:02 UTC (rev 94363)
@@ -1095,7 +1095,7 @@
m_borderProgram = adoptPtr(new LayerChromium::BorderProgram(m_context.get()));
if (!m_borderProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_borderProgram->initialize();
+ m_borderProgram->initialize(m_context.get());
}
return m_borderProgram.get();
}
@@ -1106,7 +1106,7 @@
m_headsUpDisplayProgram = adoptPtr(new CCHeadsUpDisplay::Program(m_context.get()));
if (!m_headsUpDisplayProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_headsUpDisplayProgram->initialize();
+ m_headsUpDisplayProgram->initialize(m_context.get());
}
return m_headsUpDisplayProgram.get();
}
@@ -1116,7 +1116,7 @@
ASSERT(m_renderSurfaceProgram);
if (!m_renderSurfaceProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_renderSurfaceProgram->initialize();
+ m_renderSurfaceProgram->initialize(m_context.get());
}
return m_renderSurfaceProgram.get();
}
@@ -1127,7 +1127,7 @@
m_renderSurfaceMaskProgram = adoptPtr(new CCRenderSurface::MaskProgram(m_context.get()));
if (!m_renderSurfaceMaskProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_renderSurfaceMaskProgram->initialize();
+ m_renderSurfaceMaskProgram->initialize(m_context.get());
}
return m_renderSurfaceMaskProgram.get();
}
@@ -1137,7 +1137,7 @@
ASSERT(m_tilerProgram);
if (!m_tilerProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::tilerProgram::initialize", this, 0);
- m_tilerProgram->initialize();
+ m_tilerProgram->initialize(m_context.get());
}
return m_tilerProgram.get();
}
@@ -1148,7 +1148,7 @@
m_tilerProgramSwizzle = adoptPtr(new CCTiledLayerImpl::ProgramSwizzle(m_context.get()));
if (!m_tilerProgramSwizzle->initialized()) {
TRACE_EVENT("LayerRendererChromium::tilerProgramSwizzle::initialize", this, 0);
- m_tilerProgramSwizzle->initialize();
+ m_tilerProgramSwizzle->initialize(m_context.get());
}
return m_tilerProgramSwizzle.get();
}
@@ -1159,7 +1159,7 @@
m_tilerProgramAA = adoptPtr(new CCTiledLayerImpl::ProgramAA(m_context.get()));
if (!m_tilerProgramAA->initialized()) {
TRACE_EVENT("LayerRendererChromium::tilerProgramAA::initialize", this, 0);
- m_tilerProgramAA->initialize();
+ m_tilerProgramAA->initialize(m_context.get());
}
return m_tilerProgramAA.get();
}
@@ -1170,7 +1170,7 @@
m_tilerProgramSwizzleAA = adoptPtr(new CCTiledLayerImpl::ProgramSwizzleAA(m_context.get()));
if (!m_tilerProgramSwizzleAA->initialized()) {
TRACE_EVENT("LayerRendererChromium::tilerProgramSwizzleAA::initialize", this, 0);
- m_tilerProgramSwizzleAA->initialize();
+ m_tilerProgramSwizzleAA->initialize(m_context.get());
}
return m_tilerProgramSwizzleAA.get();
}
@@ -1181,7 +1181,7 @@
m_canvasLayerProgram = adoptPtr(new CCCanvasLayerImpl::Program(m_context.get()));
if (!m_canvasLayerProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_canvasLayerProgram->initialize();
+ m_canvasLayerProgram->initialize(m_context.get());
}
return m_canvasLayerProgram.get();
}
@@ -1192,7 +1192,7 @@
m_pluginLayerProgram = adoptPtr(new CCPluginLayerImpl::Program(m_context.get()));
if (!m_pluginLayerProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_pluginLayerProgram->initialize();
+ m_pluginLayerProgram->initialize(m_context.get());
}
return m_pluginLayerProgram.get();
}
@@ -1203,7 +1203,7 @@
m_videoLayerRGBAProgram = adoptPtr(new CCVideoLayerImpl::RGBAProgram(m_context.get()));
if (!m_videoLayerRGBAProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_videoLayerRGBAProgram->initialize();
+ m_videoLayerRGBAProgram->initialize(m_context.get());
}
return m_videoLayerRGBAProgram.get();
}
@@ -1214,7 +1214,7 @@
m_videoLayerYUVProgram = adoptPtr(new CCVideoLayerImpl::YUVProgram(m_context.get()));
if (!m_videoLayerYUVProgram->initialized()) {
TRACE_EVENT("LayerRendererChromium::borderProgram::initialize", this, 0);
- m_videoLayerYUVProgram->initialize();
+ m_videoLayerYUVProgram->initialize(m_context.get());
}
return m_videoLayerYUVProgram.get();
}
@@ -1225,6 +1225,32 @@
makeContextCurrent();
m_sharedGeometry.clear();
+
+ if (m_borderProgram)
+ m_borderProgram->cleanup(m_context.get());
+ if (m_headsUpDisplayProgram)
+ m_headsUpDisplayProgram->cleanup(m_context.get());
+ if (m_tilerProgram)
+ m_tilerProgram->cleanup(m_context.get());
+ if (m_tilerProgramSwizzle)
+ m_tilerProgramSwizzle->cleanup(m_context.get());
+ if (m_tilerProgramAA)
+ m_tilerProgramAA->cleanup(m_context.get());
+ if (m_tilerProgramSwizzleAA)
+ m_tilerProgramSwizzleAA->cleanup(m_context.get());
+ if (m_canvasLayerProgram)
+ m_canvasLayerProgram->cleanup(m_context.get());
+ if (m_pluginLayerProgram)
+ m_pluginLayerProgram->cleanup(m_context.get());
+ if (m_renderSurfaceMaskProgram)
+ m_renderSurfaceMaskProgram->cleanup(m_context.get());
+ if (m_renderSurfaceProgram)
+ m_renderSurfaceProgram->cleanup(m_context.get());
+ if (m_videoLayerRGBAProgram)
+ m_videoLayerRGBAProgram->cleanup(m_context.get());
+ if (m_videoLayerYUVProgram)
+ m_videoLayerYUVProgram->cleanup(m_context.get());
+
m_borderProgram.clear();
m_headsUpDisplayProgram.clear();
m_tilerProgram.clear();
Modified: trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.cpp (94362 => 94363)
--- trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.cpp 2011-09-01 23:41:12 UTC (rev 94362)
+++ trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.cpp 2011-09-01 23:45:02 UTC (rev 94363)
@@ -37,86 +37,97 @@
namespace WebCore {
-ProgramBindingBase::ProgramBindingBase(GraphicsContext3D* context)
- : m_context(context)
- , m_program(0)
+ProgramBindingBase::ProgramBindingBase()
+ : m_program(0)
, m_initialized(false)
{
}
ProgramBindingBase::~ProgramBindingBase()
{
- if (m_program)
- GLC(m_context, m_context->deleteProgram(m_program));
+ // If you hit these asserts, you initialized but forgot to call cleanup().
+ ASSERT(!m_program);
+ ASSERT(!m_initialized);
}
-void ProgramBindingBase::init(const String& vertexShader, const String& fragmentShader)
+void ProgramBindingBase::init(GraphicsContext3D* context, const String& vertexShader, const String& fragmentShader)
{
- m_program = createShaderProgram(vertexShader, fragmentShader);
+ m_program = createShaderProgram(context, vertexShader, fragmentShader);
ASSERT(m_program);
}
-unsigned ProgramBindingBase::loadShader(unsigned type, const String& shaderSource)
+void ProgramBindingBase::cleanup(GraphicsContext3D* context)
{
- unsigned shader = m_context->createShader(type);
+ m_initialized = false;
+ if (!m_program)
+ return;
+
+ ASSERT(context);
+ GLC(context, context->deleteProgram(m_program));
+ m_program = 0;
+}
+
+unsigned ProgramBindingBase::loadShader(GraphicsContext3D* context, unsigned type, const String& shaderSource)
+{
+ unsigned shader = context->createShader(type);
if (!shader)
return 0;
String sourceString(shaderSource);
- GLC(m_context, m_context->shaderSource(shader, sourceString));
- GLC(m_context, m_context->compileShader(shader));
+ GLC(context, context->shaderSource(shader, sourceString));
+ GLC(context, context->compileShader(shader));
#ifndef NDEBUG
int compiled = 0;
- GLC(m_context, m_context->getShaderiv(shader, GraphicsContext3D::COMPILE_STATUS, &compiled));
+ GLC(context, context->getShaderiv(shader, GraphicsContext3D::COMPILE_STATUS, &compiled));
if (!compiled) {
- GLC(m_context, m_context->deleteShader(shader));
+ GLC(context, context->deleteShader(shader));
return 0;
}
#endif
return shader;
}
-unsigned ProgramBindingBase::createShaderProgram(const String& vertexShaderSource, const String& fragmentShaderSource)
+unsigned ProgramBindingBase::createShaderProgram(GraphicsContext3D* context, const String& vertexShaderSource, const String& fragmentShaderSource)
{
TRACE_EVENT("ProgramBindingBase::createShaderProgram", this, 0);
- unsigned vertexShader = loadShader(GraphicsContext3D::VERTEX_SHADER, vertexShaderSource);
+ unsigned vertexShader = loadShader(context, GraphicsContext3D::VERTEX_SHADER, vertexShaderSource);
if (!vertexShader) {
LOG_ERROR("Failed to create vertex shader");
return 0;
}
- unsigned fragmentShader = loadShader(GraphicsContext3D::FRAGMENT_SHADER, fragmentShaderSource);
+ unsigned fragmentShader = loadShader(context, GraphicsContext3D::FRAGMENT_SHADER, fragmentShaderSource);
if (!fragmentShader) {
- GLC(m_context, m_context->deleteShader(vertexShader));
+ GLC(context, context->deleteShader(vertexShader));
LOG_ERROR("Failed to create fragment shader");
return 0;
}
- unsigned programObject = m_context->createProgram();
+ unsigned programObject = context->createProgram();
if (!programObject) {
LOG_ERROR("Failed to create shader program");
return 0;
}
- GLC(m_context, m_context->attachShader(programObject, vertexShader));
- GLC(m_context, m_context->attachShader(programObject, fragmentShader));
+ GLC(context, context->attachShader(programObject, vertexShader));
+ GLC(context, context->attachShader(programObject, fragmentShader));
// Bind the common attrib locations.
- GLC(m_context, m_context->bindAttribLocation(programObject, GeometryBinding::positionAttribLocation(), "a_position"));
- GLC(m_context, m_context->bindAttribLocation(programObject, GeometryBinding::texCoordAttribLocation(), "a_texCoord"));
+ GLC(context, context->bindAttribLocation(programObject, GeometryBinding::positionAttribLocation(), "a_position"));
+ GLC(context, context->bindAttribLocation(programObject, GeometryBinding::texCoordAttribLocation(), "a_texCoord"));
- GLC(m_context, m_context->linkProgram(programObject));
+ GLC(context, context->linkProgram(programObject));
#ifndef NDEBUG
int linked = 0;
- GLC(m_context, m_context->getProgramiv(programObject, GraphicsContext3D::LINK_STATUS, &linked));
+ GLC(context, context->getProgramiv(programObject, GraphicsContext3D::LINK_STATUS, &linked));
if (!linked) {
LOG_ERROR("Failed to link shader program");
- GLC(m_context, m_context->deleteProgram(programObject));
+ GLC(context, context->deleteProgram(programObject));
return 0;
}
#endif
- GLC(m_context, m_context->deleteShader(vertexShader));
- GLC(m_context, m_context->deleteShader(fragmentShader));
+ GLC(context, context->deleteShader(vertexShader));
+ GLC(context, context->deleteShader(fragmentShader));
return programObject;
}
Modified: trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.h (94362 => 94363)
--- trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.h 2011-09-01 23:41:12 UTC (rev 94362)
+++ trunk/Source/WebCore/platform/graphics/chromium/ProgramBinding.h 2011-09-01 23:45:02 UTC (rev 94363)
@@ -37,20 +37,20 @@
class ProgramBindingBase {
public:
- explicit ProgramBindingBase(GraphicsContext3D*);
+ ProgramBindingBase();
~ProgramBindingBase();
- void init(const String& vertexShader, const String& fragmentShader);
+ void init(GraphicsContext3D*, const String& vertexShader, const String& fragmentShader);
+ void cleanup(GraphicsContext3D*);
unsigned program() const { return m_program; }
bool initialized() const { return m_initialized; }
protected:
- unsigned loadShader(unsigned type, const String& shaderSource);
- unsigned createShaderProgram(const String& vertexShaderSource, const String& fragmentShaderSource);
+ unsigned loadShader(GraphicsContext3D*, unsigned type, const String& shaderSource);
+ unsigned createShaderProgram(GraphicsContext3D*, const String& vertexShaderSource, const String& fragmentShaderSource);
- GraphicsContext3D* m_context;
unsigned m_program;
bool m_initialized;
};
@@ -59,15 +59,17 @@
class ProgramBinding : public ProgramBindingBase {
public:
explicit ProgramBinding(GraphicsContext3D* context)
- : ProgramBindingBase(context)
{
- ProgramBindingBase::init(m_vertexShader.getShaderString(), m_fragmentShader.getShaderString());
+ ProgramBindingBase::init(context, m_vertexShader.getShaderString(), m_fragmentShader.getShaderString());
}
- void initialize()
+ void initialize(GraphicsContext3D* context)
{
- m_vertexShader.init(m_context, m_program);
- m_fragmentShader.init(m_context, m_program);
+ ASSERT(context);
+ ASSERT(m_program);
+
+ m_vertexShader.init(context, m_program);
+ m_fragmentShader.init(context, m_program);
m_initialized = true;
}
