Skip to content

Commit

Permalink
GPU process - parent GL context does not delete the textures it share…
Browse files Browse the repository at this point in the history
…s with child contexts.

The child contexts create them and are then responsible for deleting them. This fixed a crash where the child contexts were passing an invalid texture ID to glBlitFramebufferANGLE.

TEST=can't reproduce bug locally anymore, trybots
BUG=75661
Review URL: http://codereview.chromium.org/6670074

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@78642 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
apatrick@chromium.org committed Mar 18, 2011
1 parent 33301e7 commit 4874aae
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 6 deletions.
3 changes: 2 additions & 1 deletion gpu/command_buffer/service/gles2_cmd_decoder.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -1972,6 +1972,7 @@ bool GLES2DecoderImpl::Initialize(
GLuint service_id = offscreen_saved_color_texture_->id();
TextureManager::TextureInfo* info =
parent_->CreateTextureInfo(parent_client_texture_id, service_id);
info->SetNotOwned();
parent_->texture_manager()->SetInfoTarget(info, GL_TEXTURE_2D);
}

Expand Down
4 changes: 2 additions & 2 deletions gpu/command_buffer/service/texture_manager.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -67,7 +67,7 @@ void TextureManager::Destroy(bool have_context) {
while (!texture_infos_.empty()) {
if (have_context) {
TextureInfo* info = texture_infos_.begin()->second;
if (!info->IsDeleted()) {
if (!info->IsDeleted() && info->owned_) {
GLuint service_id = info->service_id();
glDeleteTextures(1, &service_id);
info->MarkAsDeleted();
Expand Down
13 changes: 11 additions & 2 deletions gpu/command_buffer/service/texture_manager.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -40,7 +40,8 @@ class TextureManager {
max_level_set_(-1),
texture_complete_(false),
cube_complete_(false),
npot_(false) {
npot_(false),
owned_(true) {
}

// True if this texture meets all the GLES2 criteria for rendering.
Expand Down Expand Up @@ -109,6 +110,10 @@ class TextureManager {
return target() && !IsDeleted();
}

void SetNotOwned() {
owned_ = false;
}

private:
friend class TextureManager;
friend class base::RefCounted<TextureInfo>;
Expand Down Expand Up @@ -217,6 +222,10 @@ class TextureManager {
// Whether this texture has ever been bound.
bool has_been_bound_;

// Whether the associated context group owns this texture and should delete
// it.
bool owned_;

DISALLOW_COPY_AND_ASSIGN(TextureInfo);
};

Expand Down
24 changes: 23 additions & 1 deletion gpu/command_buffer/service/texture_manager_unittest.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2010 The Chromium Authors. All rights reserved.
// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Expand Down Expand Up @@ -114,6 +114,28 @@ TEST_F(TextureManagerTest, Destroy) {
ASSERT_TRUE(info1 == NULL);
}

TEST_F(TextureManagerTest, DestroyUnowned) {
const GLuint kClient1Id = 1;
const GLuint kService1Id = 11;
EXPECT_FALSE(manager_.HaveUnrenderableTextures());
// Check we can create texture.
TextureManager::TextureInfo* created_info =
manager_.CreateTextureInfo(&feature_info_, kClient1Id, kService1Id);
created_info->SetNotOwned();

// Check texture got created.
TextureManager::TextureInfo* info1 = manager_.GetTextureInfo(kClient1Id);
ASSERT_TRUE(info1 != NULL);
EXPECT_CALL(*gl_, DeleteTextures(4, _))
.Times(1)
.RetiresOnSaturation();

// Check that it is not freed if it is not owned.
manager_.Destroy(true);
info1 = manager_.GetTextureInfo(kClient1Id);
ASSERT_TRUE(info1 == NULL);
}

TEST_F(TextureManagerTest, MaxValues) {
// Check we get the right values for the max sizes.
EXPECT_EQ(kMax2dLevels, manager_.MaxLevelsForTarget(GL_TEXTURE_2D));
Expand Down

0 comments on commit 4874aae

Please sign in to comment.