Chromium Code Reviews
Help | Chromium Project | Sign in
(92)

Issue 13126002: TextureLayer: clear texture id when clearing client (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by piman
Modified:
1 year ago
Reviewers:
danakj
CC:
chromium-reviews_chromium.org, cc-bugs_chromium.org
Visibility:
Public.

Description

TextureLayer: clear texture id when clearing client.

This is to avoid having the impl side accessing a texture that's been deleted.

BUG=224308
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=191268

Patch Set 1 #

Patch Set 2 : Added test #

Total comments: 2

Patch Set 3 : count used textures instead of relying on asserts #

Total comments: 4

Patch Set 4 : add assert #

Patch Set 5 : add SetNeedsDisplay #

Patch Set 6 : add SetNeedsDisplay #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -2 lines) Lint Patch
M cc/layers/texture_layer.h View 1 chunk +1 line, -1 line 0 comments 0 errors Download
M cc/layers/texture_layer.cc View 1 1 chunk +5 lines, -0 lines 0 comments 0 errors Download
M cc/layers/texture_layer_unittest.cc View 1 2 3 4 2 chunks +112 lines, -0 lines 0 comments 1 errors Download
M webkit/plugins/ppapi/ppapi_plugin_instance.cc View 1 1 chunk +0 lines, -1 line 0 comments ? errors Download
Commit:

Messages

Total messages: 11
piman
1 year ago #1
danakj
Can you explain the problem this fixes in the description?
1 year ago #2
danakj
Also, unit test?
1 year ago #3
piman
On 2013/03/28 00:05:03, danakj wrote: > Also, unit test? Done, and Done.
1 year ago #4
danakj
Thanks! LGTM with one question about the test. https://codereview.chromium.org/13126002/diff/7001/cc/layers/texture_layer_unittest.cc File cc/layers/texture_layer_unittest.cc (right): https://codereview.chromium.org/13126002/diff/7001/cc/layers/texture_layer_unittest.cc#newcode532 cc/layers/texture_layer_unittest.cc:532: // ...
1 year ago #5
piman
PTAL https://codereview.chromium.org/13126002/diff/7001/cc/layers/texture_layer_unittest.cc File cc/layers/texture_layer_unittest.cc (right): https://codereview.chromium.org/13126002/diff/7001/cc/layers/texture_layer_unittest.cc#newcode532 cc/layers/texture_layer_unittest.cc:532: // We want to delete the texture, so ...
1 year ago #6
danakj
LGTM! https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc File cc/layers/texture_layer_unittest.cc (right): https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc#newcode536 cc/layers/texture_layer_unittest.cc:536: texture_layer_->ClearClient(); This should also damage the layer so ...
1 year ago #7
danakj
https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc File cc/layers/texture_layer_unittest.cc (right): https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc#newcode565 cc/layers/texture_layer_unittest.cc:565: bool result) OVERRIDE { Alternatively, or additionally, EXPECT_TRUE(result) would ...
1 year ago #8
piman
https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc File cc/layers/texture_layer_unittest.cc (right): https://codereview.chromium.org/13126002/diff/15001/cc/layers/texture_layer_unittest.cc#newcode536 cc/layers/texture_layer_unittest.cc:536: texture_layer_->ClearClient(); On 2013/03/28 03:57:07, danakj wrote: > This should ...
1 year ago #9
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/piman@chromium.org/13126002/27001
1 year ago #10
I haz the power (commit-bot)
1 year ago #11
Message was sent while issue was closed.
Change committed as 191268
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434