Fix the DCHECK problem for TextureManager::TextureInfo::SetTarget

Fix the DCHECK problem for TextureManager::TextureInfo::SetTarget

It's because GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM can
clear the target of a streamtexture which is not zero.
It occurs when destorying a html5 video on Android platform because it will
create/destroy streamtextures.

BUG=
TEST=


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

[yongsheng, 2012-11-29 09:48:08]

please help review it. See the callstack to call the crash for debug:
0x0148e5df  gpu::gles2::TextureManager::TextureInfo::SetTarget(unsigned int,
int)
0x0148e709 
gpu::gles2::TextureManager::SetInfoTarget(gpu::gles2::TextureManager::TextureInfo*,
unsigned int)
0x0146a393 
gpu::gles2::GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM(unsigned int,
gpu::gles2::DestroyStreamTextureCHROMIUM const&)

[no sievers, 2012-11-29 18:03:58]

It's possible that the right fix is to remove the
texture_manager()->SetInfoTarget(info, 0); in 
GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().

In fact, I think Min was trying to upstream that change at some point.

Also, see this patch I uploaded a year ago and never landed
https://codereview.chromium.org/8676048/

[no sievers, 2012-11-29 18:11:47]

+gman 

see https://codereview.chromium.org/8669008/

On 2012/11/29 18:03:58, Daniel Sievers wrote:
> It's possible that the right fix is to remove the
> texture_manager()->SetInfoTarget(info, 0); in 
> GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> 
> In fact, I think Min was trying to upstream that change at some point.
> 
> Also, see this patch I uploaded a year ago and never landed
> https://codereview.chromium.org/8676048/

[yongsheng, 2012-11-30 01:00:54]

On 2012/11/29 18:03:58, Daniel Sievers wrote:
> It's possible that the right fix is to remove the
> texture_manager()->SetInfoTarget(info, 0); in 
> GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> 
> In fact, I think Min was trying to upstream that change at some point.
> 
> Also, see this patch I uploaded a year ago and never landed
> https://codereview.chromium.org/8676048/
this is a patch for reuse.
so streamtexture never be reused. then it's better to remove
texture_manager()->SetInforTarget(info, 0)

[greggman, 2012-11-30 03:12:34]

On 2012/11/30 01:00:54, yongsheng wrote:
> On 2012/11/29 18:03:58, Daniel Sievers wrote:
> > It's possible that the right fix is to remove the
> > texture_manager()->SetInfoTarget(info, 0); in 
> > GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> > 
> > In fact, I think Min was trying to upstream that change at some point.
> > 
> > Also, see this patch I uploaded a year ago and never landed
> > https://codereview.chromium.org/8676048/
> this is a patch for reuse.
> so streamtexture never be reused. then it's better to remove
> texture_manager()->SetInfoTarget(info, 0)

You can't change the targets of other textures. In other words this is illegal

  glBindTexture(GL_TEXTURE_2D, tex);
  glBindTexture(GL_TEXTURE_CUBE_MAP, tex);

so i'm leary of allowing changing the target of a texture back to 0 since that
would allow other code to change the target of a texture. 

Is that okay? Maybe the check can be changed so that it's okay to set it to what
it already is?

[yongsheng, 2012-11-30 08:29:10]

On 2012/11/30 03:12:34, greggman wrote:
> On 2012/11/30 01:00:54, yongsheng wrote:
> > On 2012/11/29 18:03:58, Daniel Sievers wrote:
> > > It's possible that the right fix is to remove the
> > > texture_manager()->SetInfoTarget(info, 0); in 
> > > GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> > > 
> > > In fact, I think Min was trying to upstream that change at some point.
> > > 
> > > Also, see this patch I uploaded a year ago and never landed
> > > https://codereview.chromium.org/8676048/
> > this is a patch for reuse.
> > so streamtexture never be reused. then it's better to remove
> > texture_manager()->SetInfoTarget(info, 0)
> 
> You can't change the targets of other textures. In other words this is illegal
> 
>   glBindTexture(GL_TEXTURE_2D, tex);
>   glBindTexture(GL_TEXTURE_CUBE_MAP, tex);
> 
> so i'm leary of allowing changing the target of a texture back to 0 since that
> would allow other code to change the target of a texture. 
> 
> Is that okay? Maybe the check can be changed so that it's okay to set it to
what
> it already is?
so if target_ is 0, then it's ok.
if target_ is not 0 and target_ equals to-be-set target, then it's also ok,
right?

[yongsheng, 2012-12-03 00:45:55]

On 2012/11/30 08:29:10, yongsheng wrote:
> On 2012/11/30 03:12:34, greggman wrote:
> > On 2012/11/30 01:00:54, yongsheng wrote:
> > > On 2012/11/29 18:03:58, Daniel Sievers wrote:
> > > > It's possible that the right fix is to remove the
> > > > texture_manager()->SetInfoTarget(info, 0); in 
> > > > GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> > > > 
> > > > In fact, I think Min was trying to upstream that change at some point.
> > > > 
> > > > Also, see this patch I uploaded a year ago and never landed
> > > > https://codereview.chromium.org/8676048/
> > > this is a patch for reuse.
> > > so streamtexture never be reused. then it's better to remove
> > > texture_manager()->SetInfoTarget(info, 0)
> > 
> > You can't change the targets of other textures. In other words this is
illegal
> > 
> >   glBindTexture(GL_TEXTURE_2D, tex);
> >   glBindTexture(GL_TEXTURE_CUBE_MAP, tex);
> > 
> > so i'm leary of allowing changing the target of a texture back to 0 since
that
> > would allow other code to change the target of a texture. 
> > 
> > Is that okay? Maybe the check can be changed so that it's okay to set it to
> what
> > it already is?
> so if target_ is 0, then it's ok.
> if target_ is not 0 and target_ equals to-be-set target, then it's also ok,
> right?

do you think it's reasonable?

[no sievers, 2012-12-03 18:28:39]

On 2012/12/03 00:45:55, yongsheng wrote:
> On 2012/11/30 08:29:10, yongsheng wrote:
> > On 2012/11/30 03:12:34, greggman wrote:
> > > On 2012/11/30 01:00:54, yongsheng wrote:
> > > > On 2012/11/29 18:03:58, Daniel Sievers wrote:
> > > > > It's possible that the right fix is to remove the
> > > > > texture_manager()->SetInfoTarget(info, 0); in 
> > > > > GLES2DecoderImpl::HandleDestroyStreamTextureCHROMIUM().
> > > > > 
> > > > > In fact, I think Min was trying to upstream that change at some point.
> > > > > 
> > > > > Also, see this patch I uploaded a year ago and never landed
> > > > > https://codereview.chromium.org/8676048/
> > > > this is a patch for reuse.
> > > > so streamtexture never be reused. then it's better to remove
> > > > texture_manager()->SetInfoTarget(info, 0)
> > > 
> > > You can't change the targets of other textures. In other words this is
> illegal
> > > 
> > >   glBindTexture(GL_TEXTURE_2D, tex);
> > >   glBindTexture(GL_TEXTURE_CUBE_MAP, tex);
> > > 
> > > so i'm leary of allowing changing the target of a texture back to 0 since
> that
> > > would allow other code to change the target of a texture. 
> > > 
> > > Is that okay? Maybe the check can be changed so that it's okay to set it
to
> > what
> > > it already is?
> > so if target_ is 0, then it's ok.
> > if target_ is not 0 and target_ equals to-be-set target, then it's also ok,
> > right?
> 
> do you think it's reasonable?

yes, just remove the 'texture_manager()->SetInforTarget(info, 0)'

[yongsheng, 2012-12-04 04:55:53]

> yes, just remove the 'texture_manager()->SetInforTarget(info, 0)'
ok, need your lgtm :)

[greggman, 2012-12-04 07:06:18]

lgtm

[commit-bot: I haz the power, 2012-12-04 07:11:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yongsheng.zhu@intel.com/11421148/6002

[commit-bot: I haz the power, 2012-12-04 09:46:32]

Change committed as 170915