Merge VideoDecodeAcceleratorImpl with VideoDecodeAccelerator

Merge VideoDecodeAcceleratorImpl with VideoDecodeAccelerator

This moves the single method CanDecodeOnIOThread on
VideoDecodeAcceleratorImpl to VideoDecodeAccelerator. The
implementations that were subclasses of VideoDecodeAcceleratorImpl now
get an override for CanDecodeOnIOThread that returns false.

TEST=linux desktop build
BUG=380884
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275621

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

I'm OK with this change % the comment below, but it is being made purely for
forward-looking purposes, so deferring to scherkus@ for real review and
blessing.

https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
File media/video/video_decode_accelerator.h (right):

https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
media/video/video_decode_accelerator.h:135: // Returns true if VDA::Decode and
VDA::Client callbacks can run on the IO
Please preface this comment with:

// GPU PROCESS ONLY.  Implementations of this interface in the
content/common/gpu/media should implement this, and implementations in other
processes should not override the default implementation.

and remove the =0 and provide a default implementation that simply does:
LOG(FATAL) << "This should only get called in the GPU process";

[spang, 2014-06-05 15:45:25]

On 2014/06/05 00:17:56, Ami leaving Chromium June 9th wrote:
> I'm OK with this change % the comment below, but it is being made purely for
> forward-looking purposes, so deferring to scherkus@ for real review and
> blessing.
> 
>
https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
> File media/video/video_decode_accelerator.h (right):
> 
>
https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
> media/video/video_decode_accelerator.h:135: // Returns true if VDA::Decode and
> VDA::Client callbacks can run on the IO
> Please preface this comment with:
> 
> // GPU PROCESS ONLY.  Implementations of this interface in the
> content/common/gpu/media should implement this, and implementations in other
> processes should not override the default implementation.
> 
> and remove the =0 and provide a default implementation that simply does:
> LOG(FATAL) << "This should only get called in the GPU process";

Ok, i can update it. However, should we try to write the contract in a way that
removes references to GPU process instead? Maybe call it something like
IsDecodeThreadSafe().

[Ami GONE FROM CHROMIUM, 2014-06-05 18:52:27]

On 2014/06/05 15:45:25, spang wrote:
> On 2014/06/05 00:17:56, Ami leaving Chromium June 9th wrote:
> > I'm OK with this change % the comment below, but it is being made purely for
> > forward-looking purposes, so deferring to scherkus@ for real review and
> > blessing.
> > 
> >
>
https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
> > File media/video/video_decode_accelerator.h (right):
> > 
> >
>
https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
> > media/video/video_decode_accelerator.h:135: // Returns true if VDA::Decode
and
> > VDA::Client callbacks can run on the IO
> > Please preface this comment with:
> > 
> > // GPU PROCESS ONLY.  Implementations of this interface in the
> > content/common/gpu/media should implement this, and implementations in other
> > processes should not override the default implementation.
> > 
> > and remove the =0 and provide a default implementation that simply does:
> > LOG(FATAL) << "This should only get called in the GPU process";
> 
> Ok, i can update it. However, should we try to write the contract in a way
that
> removes references to GPU process instead? Maybe call it something like
> IsDecodeThreadSafe().

Merely renaming the existing method that way would be wrong; the existing method
is tied strongly to the GPU process' threading model re: message processing.
The more general change triggers my YAGNI alarms.

[spang, 2014-06-06 18:38:42]

https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
File media/video/video_decode_accelerator.h (right):

https://codereview.chromium.org/317083003/diff/1/media/video/video_decode_acc...
media/video/video_decode_accelerator.h:135: // Returns true if VDA::Decode and
VDA::Client callbacks can run on the IO
On 2014/06/05 00:17:57, Ami leaving Chromium June 6th wrote:
> Please preface this comment with:
> 
> // GPU PROCESS ONLY.  Implementations of this interface in the
> content/common/gpu/media should implement this, and implementations in other
> processes should not override the default implementation.
> 
> and remove the =0 and provide a default implementation that simply does:
> LOG(FATAL) << "This should only get called in the GPU process";

Done.

[Ami GONE FROM CHROMIUM, 2014-06-06 18:40:26]

L-G.

[spang, 2014-06-06 18:40:47]

scherkus: PTAL

[scherkus (not reviewing), 2014-06-06 21:37:39]

lgtm

[spang, 2014-06-06 21:39:46]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2014-06-06 21:40:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/spang@chromium.org/317083003/40001

[commit-bot: I haz the power, 2014-06-07 05:12:26]

Change committed as 275621

[dcheng, 2014-06-07 07:16:07]

A revert of this CL has been created in
https://codereview.chromium.org/319323002/ by dcheng@chromium.org.

The reason for reverting is: Speculative revert to fix content_unittest failures
in
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%....