summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwolfbeast <mcwerewolf@wolfbeast.com>2020-01-22 11:49:38 +0100
committerwolfbeast <mcwerewolf@wolfbeast.com>2020-01-22 11:51:19 +0100
commit5a6059feef9d2c7837afa3f87a2a9bfed345bb9d (patch)
tree2dae570a2f1bbaecc250b0f2b67c80e936c3b945
parent8e33fa9c4b5e4fb3ddfa11b7722cc74df3f94eca (diff)
parentfabe21d593bd09e50ffc9932b074305f15d5409a (diff)
downloadUXP-5a6059feef9d2c7837afa3f87a2a9bfed345bb9d.tar
UXP-5a6059feef9d2c7837afa3f87a2a9bfed345bb9d.tar.gz
UXP-5a6059feef9d2c7837afa3f87a2a9bfed345bb9d.tar.lz
UXP-5a6059feef9d2c7837afa3f87a2a9bfed345bb9d.tar.xz
UXP-5a6059feef9d2c7837afa3f87a2a9bfed345bb9d.zip
Issue #1354 - Fix another potential crashing scenario in WebGL.
(merge of gl-work branch)
-rw-r--r--dom/canvas/WebGLContextValidate.cpp7
-rw-r--r--gfx/gl/GLContext.cpp13
-rw-r--r--gfx/gl/GLContext.h6
-rw-r--r--gfx/gl/GLContextCGL.h2
-rw-r--r--gfx/gl/GLContextGLX.h1
-rw-r--r--gfx/gl/GLContextProviderCGL.mm22
-rw-r--r--gfx/gl/GLContextProviderEAGL.mm41
-rw-r--r--gfx/gl/GLContextProviderEGL.cpp47
-rw-r--r--gfx/gl/GLContextProviderGLX.cpp24
-rw-r--r--gfx/gl/GLContextProviderWGL.cpp13
10 files changed, 81 insertions, 95 deletions
diff --git a/dom/canvas/WebGLContextValidate.cpp b/dom/canvas/WebGLContextValidate.cpp
index ebf0aa8c2..30d4c6522 100644
--- a/dom/canvas/WebGLContextValidate.cpp
+++ b/dom/canvas/WebGLContextValidate.cpp
@@ -440,6 +440,13 @@ WebGLContext::InitAndValidateGL(FailureReason* const out_failReason)
{
MOZ_RELEASE_ASSERT(gl, "GFX: GL not initialized");
+ if (!gl->MakeCurrent(true)) {
+ MOZ_ASSERT(false);
+ *out_failReason = { "FEATURE_FAILURE_WEBGL_MAKECURRENT",
+ "Failed to MakeCurrent for init." };
+ return false;
+ }
+
// Unconditionally create a new format usage authority. This is
// important when restoring contexts and extensions need to add
// formats back into the authority.
diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp
index 1515b8627..3fb87822d 100644
--- a/gfx/gl/GLContext.cpp
+++ b/gfx/gl/GLContext.cpp
@@ -578,6 +578,10 @@ GLContext::LoadFeatureSymbols(const char* prefix, bool trygl, const SymLoadStruc
bool
GLContext::InitWithPrefixImpl(const char* prefix, bool trygl)
{
+ // wglGetProcAddress requires a current context.
+ if (!MakeCurrent(true))
+ return false;
+
mWorkAroundDriverBugs = gfxPrefs::WorkAroundDriverBugs();
const SymLoadStruct coreSymbols[] = {
@@ -714,7 +718,6 @@ GLContext::InitWithPrefixImpl(const char* prefix, bool trygl)
////////////////
- MakeCurrent();
MOZ_ASSERT(mProfile != ContextProfile::Unknown);
uint32_t version = 0;
@@ -2253,13 +2256,11 @@ GLContext::MarkDestroyed()
mBlitHelper = nullptr;
mReadTexImageHelper = nullptr;
- if (MakeCurrent()) {
+ mIsDestroyed = true;
+ mSymbols.Zero();
+ if (MakeCurrent(true)) {
mTexGarbageBin->GLContextTeardown();
- } else {
- NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown.");
}
-
- mSymbols.Zero();
}
#ifdef MOZ_GL_DEBUG
diff --git a/gfx/gl/GLContext.h b/gfx/gl/GLContext.h
index 6e3e22207..4c1f4f55c 100644
--- a/gfx/gl/GLContext.h
+++ b/gfx/gl/GLContext.h
@@ -353,6 +353,7 @@ public:
protected:
bool mIsOffscreen;
bool mContextLost;
+ bool mIsDestroyed = false;
/**
* mVersion store the OpenGL's version, multiplied by 100. For example, if
@@ -3265,7 +3266,7 @@ public:
#endif
return MakeCurrentImpl(aForce);
}
-
+
virtual bool Init() = 0;
virtual bool SetupLookupFunction() = 0;
@@ -3273,8 +3274,7 @@ public:
virtual void ReleaseSurface() {}
bool IsDestroyed() {
- // MarkDestroyed will mark all these as null.
- return mSymbols.fUseProgram == nullptr;
+ return mIsDestroyed;
}
GLContext* GetSharedContext() { return mSharedContext; }
diff --git a/gfx/gl/GLContextCGL.h b/gfx/gl/GLContextCGL.h
index 1a29f3d15..23616d861 100644
--- a/gfx/gl/GLContextCGL.h
+++ b/gfx/gl/GLContextCGL.h
@@ -24,7 +24,7 @@ class GLContextCGL : public GLContext
{
friend class GLContextProviderCGL;
- NSOpenGLContext* mContext;
+ NSOpenGLContext* const mContext;
public:
MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(GLContextCGL, override)
diff --git a/gfx/gl/GLContextGLX.h b/gfx/gl/GLContextGLX.h
index 1f2cee08d..0463669e9 100644
--- a/gfx/gl/GLContextGLX.h
+++ b/gfx/gl/GLContextGLX.h
@@ -86,6 +86,7 @@ private:
GLXContext mContext;
Display* mDisplay;
GLXDrawable mDrawable;
+ Maybe<GLXDrawable> mOverrideDrawable;
bool mDeleteDrawable;
bool mDoubleBuffered;
diff --git a/gfx/gl/GLContextProviderCGL.mm b/gfx/gl/GLContextProviderCGL.mm
index ceab3046c..7cc89eac9 100644
--- a/gfx/gl/GLContextProviderCGL.mm
+++ b/gfx/gl/GLContextProviderCGL.mm
@@ -83,26 +83,13 @@ GLContextCGL::GLContextCGL(CreateContextFlags flags, const SurfaceCaps& caps,
GLContextCGL::~GLContextCGL()
{
MarkDestroyed();
-
- if (mContext) {
- if ([NSOpenGLContext currentContext] == mContext) {
- // Clear the current context before releasing. If we don't do
- // this, the next time we call [NSOpenGLContext currentContext],
- // "invalid context" will be printed to the console.
- [NSOpenGLContext clearCurrentContext];
- }
- [mContext release];
- }
-
+ [mContext release];
}
bool
GLContextCGL::Init()
{
- if (!InitWithPrefix("gl", true))
- return false;
-
- return true;
+ return InitWithPrefix("gl", true);
}
CGLContextObj
@@ -114,6 +101,11 @@ GLContextCGL::GetCGLContext() const
bool
GLContextCGL::MakeCurrentImpl(bool aForce)
{
+ if (IsDestroyed()) {
+ [NSOpenGLContext clearCurrentContext];
+ return false;
+ }
+
if (!aForce && [NSOpenGLContext currentContext] == mContext) {
return true;
}
diff --git a/gfx/gl/GLContextProviderEAGL.mm b/gfx/gl/GLContextProviderEAGL.mm
index 507616e2f..11c7cce77 100644
--- a/gfx/gl/GLContextProviderEAGL.mm
+++ b/gfx/gl/GLContextProviderEAGL.mm
@@ -36,35 +36,24 @@ GLContextEAGL::GLContextEAGL(CreateContextFlags flags, const SurfaceCaps& caps,
GLContextEAGL::~GLContextEAGL()
{
- MakeCurrent();
-
- if (mBackbufferFB) {
- fDeleteFramebuffers(1, &mBackbufferFB);
- }
-
- if (mBackbufferRB) {
- fDeleteRenderbuffers(1, &mBackbufferRB);
+ if (MakeCurrent()) {
+ if (mBackbufferFB) {
+ fDeleteFramebuffers(1, &mBackbufferFB);
+ }
+ if (mBackbufferRB) {
+ fDeleteRenderbuffers(1, &mBackbufferRB);
+ }
}
+ mLayer = nil;
MarkDestroyed();
-
- if (mLayer) {
- mLayer = nil;
- }
-
- if (mContext) {
- [EAGLContext setCurrentContext:nil];
- [mContext release];
- }
+ [mContext release];
}
bool
GLContextEAGL::Init()
{
- if (!InitWithPrefix("gl", true))
- return false;
-
- return true;
+ return InitWithPrefix("gl", true);
}
bool
@@ -120,12 +109,12 @@ GLContextEAGL::MakeCurrentImpl(bool aForce)
return true;
}
- if (mContext) {
- if(![EAGLContext setCurrentContext:mContext]) {
- return false;
- }
+ if (IsDestroyed()) {
+ [EAGLContext setCurrentContext:nil];
+ return false;
}
- return true;
+
+ return [EAGLContext setCurrentContext:mContext];
}
bool
diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp
index 7979f3bf0..23fc3472d 100644
--- a/gfx/gl/GLContextProviderEGL.cpp
+++ b/gfx/gl/GLContextProviderEGL.cpp
@@ -150,13 +150,12 @@ is_power_of_two(int v)
}
static void
-DestroySurface(EGLSurface oldSurface) {
- if (oldSurface != EGL_NO_SURFACE) {
- sEGLLibrary.fMakeCurrent(EGL_DISPLAY(),
- EGL_NO_SURFACE, EGL_NO_SURFACE,
- EGL_NO_CONTEXT);
- sEGLLibrary.fDestroySurface(EGL_DISPLAY(), oldSurface);
+DestroySurface(const EGLSurface surf) {
+ if (!surf) {
+ // Nothing to do.
+ return;
}
+ sEGLLibrary.fDestroySurface(EGL_DISPLAY(), surf);
}
static EGLSurface
@@ -223,7 +222,7 @@ GLContextEGL::~GLContextEGL()
sEGLLibrary.fDestroyContext(EGL_DISPLAY(), mContext);
sEGLLibrary.UnsetCachedCurrentContext();
- mozilla::gl::DestroySurface(mSurface);
+ DestroySurface(mSurface);
}
bool
@@ -247,12 +246,7 @@ GLContextEGL::Init()
if (!InitWithPrefix("gl", true))
return false;
- bool current = MakeCurrent();
- if (!current) {
- gfx::LogFailure(NS_LITERAL_CSTRING(
- "Couldn't get device attachments for device."));
- return false;
- }
+ MOZ_ASSERT(IsCurrent());
static_assert(sizeof(GLint) >= sizeof(int32_t), "GLint is smaller than int32_t");
mMaxTextureImageSize = INT32_MAX;
@@ -303,7 +297,10 @@ GLContextEGL::ReleaseTexImage()
}
void
-GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) {
+GLContextEGL::SetEGLSurfaceOverride(const EGLSurface surf) {
+
+ MOZ_ASSERT(!surf || surf != mSurface);
+
if (Screen()) {
/* Blit `draw` to `read` if we need to, before we potentially juggle
* `read` around. If we don't, we might attach a different `read`,
@@ -314,12 +311,17 @@ GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) {
}
mSurfaceOverride = surf;
- DebugOnly<bool> ok = MakeCurrent(true);
- MOZ_ASSERT(ok);
+ MOZ_ALWAYS_TRUE(MakeCurrent(true));
}
bool
GLContextEGL::MakeCurrentImpl(bool aForce) {
+ if (IsDestroyed()) {
+ //Clear and exit
+ sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), nullptr, nullptr, nullptr);
+ return false;
+ }
+
bool succeeded = true;
// Assume that EGL has the same problem as WGL does,
@@ -373,7 +375,7 @@ GLContextEGL::IsCurrent() {
}
bool
-GLContextEGL::RenewSurface(nsIWidget* aWidget) {
+GLContextEGL::RenewSurface(nsIWidget* const aWidget) {
if (!mOwnsContext) {
return false;
}
@@ -389,13 +391,12 @@ GLContextEGL::RenewSurface(nsIWidget* aWidget) {
void
GLContextEGL::ReleaseSurface() {
- if (mOwnsContext) {
- mozilla::gl::DestroySurface(mSurface);
- }
- if (mSurface == mSurfaceOverride) {
- mSurfaceOverride = EGL_NO_SURFACE;
+ if (!mOwnsContext) {
+ return;
}
- mSurface = EGL_NO_SURFACE;
+
+ DestroySurface(mSurface);
+ mSurface = nullptr;
}
bool
diff --git a/gfx/gl/GLContextProviderGLX.cpp b/gfx/gl/GLContextProviderGLX.cpp
index 539520a8c..c44b1a9f0 100644
--- a/gfx/gl/GLContextProviderGLX.cpp
+++ b/gfx/gl/GLContextProviderGLX.cpp
@@ -894,20 +894,12 @@ GLContextGLX::~GLContextGLX()
return;
}
- // see bug 659842 comment 76
-#ifdef DEBUG
- bool success =
-#endif
- mGLX->xMakeCurrent(mDisplay, X11None, nullptr);
- MOZ_ASSERT(success,
- "glXMakeCurrent failed to release GL context before we call "
- "glXDestroyContext!");
-
mGLX->xDestroyContext(mDisplay, mContext);
if (mDeleteDrawable) {
mGLX->xDestroyPixmap(mDisplay, mDrawable);
}
+ MOZ_ASSERT(!mOverrideDrawable);
}
@@ -944,6 +936,10 @@ GLContextGLX::MakeCurrentImpl(bool aForce)
// DRI2InvalidateBuffers event before drawing. See bug 1280653.
Unused << XPending(mDisplay);
}
+ if (IsDestroyed()) {
+ MOZ_ALWAYS_TRUE(mGLX->xMakeCurrent(mDisplay, X11None, nullptr));
+ return false; // We did not MakeCurrent mContext, but that's what we wanted!
+ }
succeeded = mGLX->xMakeCurrent(mDisplay, mDrawable, mContext);
NS_ASSERTION(succeeded, "Failed to make GL context current!");
@@ -1017,16 +1013,18 @@ GLContextGLX::GetWSIInfo(nsCString* const out) const
bool
GLContextGLX::OverrideDrawable(GLXDrawable drawable)
{
- if (Screen())
+ if (Screen()) {
Screen()->AssureBlitted();
- Bool result = mGLX->xMakeCurrent(mDisplay, drawable, mContext);
- return result;
+ }
+ mOverrideDrawable = Some(drawable);
+ return MakeCurrent(true);
}
bool
GLContextGLX::RestoreDrawable()
{
- return mGLX->xMakeCurrent(mDisplay, mDrawable, mContext);
+ mOverrideDrawable = Nothing();
+ return MakeCurrent(true);
}
GLContextGLX::GLContextGLX(
diff --git a/gfx/gl/GLContextProviderWGL.cpp b/gfx/gl/GLContextProviderWGL.cpp
index 35957259d..da8c93d10 100644
--- a/gfx/gl/GLContextProviderWGL.cpp
+++ b/gfx/gl/GLContextProviderWGL.cpp
@@ -314,20 +314,17 @@ GLContextWGL::Init()
if (!mDC || !mContext)
return false;
- // see bug 929506 comment 29. wglGetProcAddress requires a current context.
- if (!sWGLLib.fMakeCurrent(mDC, mContext))
- return false;
-
SetupLookupFunction();
- if (!InitWithPrefix("gl", true))
- return false;
-
- return true;
+ return InitWithPrefix("gl", true);
}
bool
GLContextWGL::MakeCurrentImpl(bool aForce)
{
+ if (IsDestroyed()) {
+ MOZ_ALWAYS_TRUE(sWGLLib.fMakeCurrent(0, 0));
+ }
+
BOOL succeeded = true;
// wglGetCurrentContext seems to just pull the HGLRC out