Fix leak with binding Framebuffers directly.

src/libANGLE/Context.cpp

@@ -686,24 +686,16 @@ void Context::bindTexture(GLenum target, GLuint handle)
     mState.setSamplerTexture(target, texture);
 }
 
-void Context::bindReadFramebuffer(GLuint framebuffer)
+void Context::bindReadFramebuffer(GLuint framebufferHandle)
 {
-    if (!getFramebuffer(framebuffer))
-    {
-        mFramebufferMap[framebuffer] = new Framebuffer(mCaps, mRenderer, framebuffer);
-    }
-
-    mState.setReadFramebufferBinding(getFramebuffer(framebuffer));
+    Framebuffer *framebuffer = checkFramebufferAllocation(framebufferHandle);
+    mState.setReadFramebufferBinding(framebuffer);
 }
 
-void Context::bindDrawFramebuffer(GLuint framebuffer)
+void Context::bindDrawFramebuffer(GLuint framebufferHandle)
 {
-    if (!getFramebuffer(framebuffer))
-    {
-        mFramebufferMap[framebuffer] = new Framebuffer(mCaps, mRenderer, framebuffer);
-    }
-
-    mState.setDrawFramebufferBinding(getFramebuffer(framebuffer));
+    Framebuffer *framebuffer = checkFramebufferAllocation(framebufferHandle);
+    mState.setDrawFramebufferBinding(framebuffer);
 }
 
 void Context::bindRenderbuffer(GLuint renderbuffer)
@@ -887,16 +879,8 @@ Error Context::getQueryObjectui64v(GLuint id, GLenum pname, GLuint64 *params)
 
 Framebuffer *Context::getFramebuffer(unsigned int handle) const
 {
-    FramebufferMap::const_iterator framebuffer = mFramebufferMap.find(handle);
-
-    if (framebuffer == mFramebufferMap.end())
-    {
-        return NULL;
-    }
-    else
-    {
-        return framebuffer->second;
-    }
+    auto framebufferIt = mFramebufferMap.find(handle);
+    return ((framebufferIt == mFramebufferMap.end()) ? nullptr : framebufferIt->second);
 }
 
 FenceNV *Context::getFenceNV(unsigned int handle)
@@ -1685,6 +1669,7 @@ EGLenum Context::getRenderBuffer() const
 
 void Context::checkVertexArrayAllocation(GLuint vertexArray)
 {
+    // Only called after a prior call to Gen.
     if (!getVertexArray(vertexArray))
     {
         VertexArray *vertexArrayObject =
@@ -1695,6 +1680,7 @@ void Context::checkVertexArrayAllocation(GLuint vertexArray)
 
 void Context::checkTransformFeedbackAllocation(GLuint transformFeedback)
 {
+    // Only called after a prior call to Gen.
     if (!getTransformFeedback(transformFeedback))
     {
         TransformFeedback *transformFeedbackObject =
@@ -1704,6 +1690,27 @@ void Context::checkTransformFeedbackAllocation(GLuint transformFeedback)
     }
 }
 
+Framebuffer *Context::checkFramebufferAllocation(GLuint framebuffer)
+{
+    // Can be called from Bind without a prior call to Gen.
+    auto framebufferIt = mFramebufferMap.find(framebuffer);
+    bool neverCreated = framebufferIt == mFramebufferMap.end();
+    if (neverCreated || framebufferIt->second == nullptr)
+    {
+        Framebuffer *newFBO = new Framebuffer(mCaps, mRenderer, framebuffer);
+        if (neverCreated)
+        {
+            mFramebufferHandleAllocator.reserve(framebuffer);
+            mFramebufferMap[framebuffer] = newFBO;
+            return newFBO;
+        }
+
+        framebufferIt->second = newFBO;
+    }
+
+    return framebufferIt->second;
+}
+
 bool Context::isVertexArrayGenerated(GLuint vertexArray)
 {
     return mVertexArrayMap.find(vertexArray) != mVertexArrayMap.end();

src/libANGLE/Context.h

@@ -111,8 +111,8 @@ class Context final : public ValidationContext
     void bindArrayBuffer(GLuint buffer);
     void bindElementArrayBuffer(GLuint buffer);
     void bindTexture(GLenum target, GLuint handle);
-    void bindReadFramebuffer(GLuint framebuffer);
-    void bindDrawFramebuffer(GLuint framebuffer);
+    void bindReadFramebuffer(GLuint framebufferHandle);
+    void bindDrawFramebuffer(GLuint framebufferHandle);
     void bindRenderbuffer(GLuint renderbuffer);
     void bindVertexArray(GLuint vertexArray);
     void bindSampler(GLuint textureUnit, GLuint sampler);
@@ -319,6 +319,7 @@ class Context final : public ValidationContext
   private:
     void checkVertexArrayAllocation(GLuint vertexArray);
     void checkTransformFeedbackAllocation(GLuint transformFeedback);
+    Framebuffer *checkFramebufferAllocation(GLuint framebufferHandle);
 
     void detachBuffer(GLuint buffer);
     void detachTexture(GLuint texture);
@@ -391,6 +392,6 @@ class Context final : public ValidationContext
     ResourceManager *mResourceManager;
 };
 
-}
+}  // namespace gl
 
 #endif   // LIBANGLE_CONTEXT_H_

src/libANGLE/HandleAllocator.cpp

@@ -126,8 +126,9 @@ void HandleAllocator::reserve(GLuint handle)
     }
     if (begin != handle)
     {
+        ASSERT(begin < handle);
         mUnallocatedList.insert(placementIt, HandleRange(begin, handle));
     }
 }
 
-}
+}  // namespace gl

src/libANGLE/HandleAllocator.h

@@ -58,6 +58,6 @@ class HandleAllocator final : angle::NonCopyable
     std::vector<GLuint> mReleasedList;
 };
 
-}
+}  // namespace gl
 
 #endif   // LIBANGLE_HANDLEALLOCATOR_H_

src/tests/angle_end2end_tests.gypi

@@ -46,6 +46,7 @@
             '<(angle_path)/src/tests/gl_tests/PBOExtensionTest.cpp',
             '<(angle_path)/src/tests/gl_tests/PointSpritesTest.cpp',
             '<(angle_path)/src/tests/gl_tests/ProvokingVertexTest.cpp',
+            '<(angle_path)/src/tests/gl_tests/ObjectAllocationTest.cpp',
             '<(angle_path)/src/tests/gl_tests/OcclusionQueriesTest.cpp',
             '<(angle_path)/src/tests/gl_tests/ProgramBinaryTest.cpp',
             '<(angle_path)/src/tests/gl_tests/ReadPixelsTest.cpp',

src/tests/gl_tests/ObjectAllocationTest.cpp

+//
+// Copyright 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// ObjectAllocationTest
+//   Tests for object allocations and lifetimes.
+//
+
+#include "test_utils/ANGLETest.h"
+
+using namespace angle;
+
+namespace
+{
+
+class ObjectAllocationTest : public ANGLETest
+{
+  protected:
+    ObjectAllocationTest() {}
+};
+
+// Test that we don't re-allocate a bound framebuffer ID.
+TEST_P(ObjectAllocationTest, BindFramebufferBeforeGen)
+{
+    glBindFramebuffer(GL_FRAMEBUFFER, 1);
+    GLuint fbo = 0;
+    glGenFramebuffers(1, &fbo);
+    EXPECT_NE(1u, fbo);
+    glDeleteFramebuffers(1, &fbo);
+    EXPECT_GL_NO_ERROR();
+}
+
+// Test that we don't re-allocate a bound framebuffer ID, other pattern.
+TEST_P(ObjectAllocationTest, BindFramebufferAfterGen)
+{
+    GLuint firstFBO = 0;
+    glGenFramebuffers(1, &firstFBO);
+    glBindFramebuffer(GL_FRAMEBUFFER, 1);
+    glDeleteFramebuffers(1, &firstFBO);
+
+    glBindFramebuffer(GL_FRAMEBUFFER, 2);
+    GLuint secondFBOs[2] = {0};
+    glGenFramebuffers(2, secondFBOs);
+    EXPECT_NE(2u, secondFBOs[0]);
+    EXPECT_NE(2u, secondFBOs[1]);
+    glDeleteFramebuffers(2, secondFBOs);
+
+    EXPECT_GL_NO_ERROR();
+}
+
+}  // anonymous namespace
+
+ANGLE_INSTANTIATE_TEST(ObjectAllocationTest, ES3_OPENGL(), ES3_D3D11());
\ No newline at end of file