media: Refactor VideoFrameStreamTest

media/filters/fake_video_decoder.h

Index: media/filters/fake_video_decoder.h
diff --git a/media/filters/fake_video_decoder.h b/media/filters/fake_video_decoder.h
index 3ac932f3f4bb0295721295d357d90ea193243892..4c5568f781735129833e80d31d8748dd751bc0c9 100644
--- a/media/filters/fake_video_decoder.h
+++ b/media/filters/fake_video_decoder.h
@@ -34,7 +34,8 @@ class FakeVideoDecoder : public VideoDecoder {
   // Constructs an object with a decoding delay of |decoding_delay| frames.
   // |bytes_decoded_cb| is called after each decode. The sum of the byte
   // count over all calls will be equal to total_bytes_decoded().
-  FakeVideoDecoder(int decoding_delay,
+  FakeVideoDecoder(const std::string& decoder_name,

[sandersd (OOO until July 31), 2017/04/26 20:11:05]

I am still not comfortable with this concept. I'm also seeing that the
MediaCapabilities work is going to need some way to reference specific decoders
without necessarily having an instance of them.

At the same time, we have other "static" decoder data that is not currently
guaranteed to actually be static. I think we need a new concept that identifies
a decoder implementation and guarantees to answer static queries statically. If
that concept also provides a factory, it could replace our decoder factory
callbacks too, while simultaneously providing "names" for them in the sense of
operator==.

Of course what we really want is metaclasses, but I don't know a good way to
fake that directly in C++.

[sandersd (OOO until July 31), 2017/04/26 21:46:47]

After taking this through in SEA, here is what we came up with:
  - There doesn't seem to be any reason we can't create decoders on the media
thread, and the fact that we are using a list of instances already implies that
Decoder constructors are not allowed to do much.
  - If you use a list of create callbacks rather than instances, there is no
need for a blacklist--you can remove callbacks from your local list, but not
remove callbacks for Initialize()d classes.
  - Callbacks already have operator== if that is useful to you.
  - DecoderSelector should be maintaining the blacklist, this doesn't seem to be
the caller's concern. (Possibly config changes should be plumbed explicitly?)

I should move this conversation back to the other CL, but basically the proposal
is 'use factory callbacks instead of instances'. If there are no fundamental
blockers to that, it seems to be a lot simpler.

+                   int decoding_delay,
                    int max_parallel_decoding_requests,
                    const BytesDecodedCB& bytes_decoded_cb);
   ~FakeVideoDecoder() override;
@@ -98,6 +99,7 @@ class FakeVideoDecoder : public VideoDecoder {
 
   base::ThreadChecker thread_checker_;
 
+  const std::string decoder_name_;
   const size_t decoding_delay_;
   const int max_parallel_decoding_requests_;
   BytesDecodedCB bytes_decoded_cb_;