Fixed failing DCHECK in GpuVideoDecodeAccelerator destructor.

Fixed failing DCHECK in GpuVideoDecodeAccelerator destructor.

BUG=

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

[Victor Starodub, 2014-06-06 14:40:27]

I've got some pending test fixture changes that start triggering the DCHECK.

[Victor Starodub, 2014-06-06 14:40:27]

I've got some pending test fixture changes that start triggering the DCHECK.

[chromium-reviews, 2014-06-06 14:58:50]

Can you point reviewlog at the triggering fixture change?
On Jun 6, 2014 7:40 AM, <starodub@google.com> wrote:

> Reviewers: scherkus, dmichael,
>
> Message:
> I've got some pending test fixture changes that start triggering the
> DCHECK.
>
> Description:
> Fixed failing DCHECK in GpuVideoDecodeAccelerator destructor.
>
> BUG=
>
> Please review this at https://codereview.chromium.org/314293004/
>
> SVN Base: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+1, -0 lines):
>   M content/common/gpu/media/gpu_video_decode_accelerator.cc
>
>
> Index: content/common/gpu/media/gpu_video_decode_accelerator.cc
> diff --git a/content/common/gpu/media/gpu_video_decode_accelerator.cc
> b/content/common/gpu/media/gpu_video_decode_accelerator.cc
> index 0d94eaa2d80876ff37f0460af66f1350eeb2a575..
> ab97618ca0b111d1a522d12be5272337107e98ad 100644
> --- a/content/common/gpu/media/gpu_video_decode_accelerator.cc
> +++ b/content/common/gpu/media/gpu_video_decode_accelerator.cc
> @@ -459,6 +459,7 @@ void GpuVideoDecodeAccelerator::OnWillDestroyStub() {
>    stub_->channel()->RemoveRoute(host_route_id_);
>    stub_->RemoveDestructionObserver(this);
>
> +  video_decode_accelerator_.reset();
>    delete this;
>  }
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Victor Starodub, 2014-06-06 15:39:54]

https://codereview.chromium.org/311943002/ -- this contains all the changes to
the text fixture and other changes required to implement correct shutdown on
android. I'm currently splitting this change into smaller more reviewable parts.

The fix is actually required because of
https://codereview.chromium.org/292183011 , hence dmichael@ is included in the
review.

[Ami GONE FROM CHROMIUM, 2014-06-06 17:48:56]

OIC; I thought you were implying a subtle bug in dmichael's scoped_ptr deleter
change, but really this is just a simple oversight of the DCHECK in
~GpuVideoDecodeAccelerator() that asserts that the underlying decoder has
already been freed.

FTR the error this fixes is
http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...

LGTM

[dmichael (off chromium), 2014-06-06 18:14:11]

On 2014/06/06 17:48:56, Ami leaving Chromium June 6th wrote:
> OIC; I thought you were implying a subtle bug in dmichael's scoped_ptr deleter
> change, but really this is just a simple oversight of the DCHECK in
> ~GpuVideoDecodeAccelerator() that asserts that the underlying decoder has
> already been freed.
> 
> FTR the error this fixes is
>
http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...
> 
> LGTM

Oops, sorry, didn't realize the destructor did that DCHECK. I saw "delete this"
and figured that was good enough.

[dmichael (off chromium), 2014-06-06 18:14:22]

On 2014/06/06 18:14:11, dmichael wrote:
> On 2014/06/06 17:48:56, Ami leaving Chromium June 6th wrote:
> > OIC; I thought you were implying a subtle bug in dmichael's scoped_ptr
deleter
> > change, but really this is just a simple oversight of the DCHECK in
> > ~GpuVideoDecodeAccelerator() that asserts that the underlying decoder has
> > already been freed.
> > 
> > FTR the error this fixes is
> >
>
http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...
> > 
> > LGTM
> 
> Oops, sorry, didn't realize the destructor did that DCHECK. I saw "delete
this"
> and figured that was good enough.

oh, and lgtm

[Victor Starodub, 2014-06-06 19:18:59]

The CQ bit was checked by starodub@google.com

[commit-bot: I haz the power, 2014-06-06 19:20:53]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/starodub@google.com/314293004/1

[commit-bot: I haz the power, 2014-06-06 20:32:12]

Change committed as 275513