Motown: Move responsibility for binding to MediaFactoryService::Product
MediaFactoryService::Product is a base class for the products of the
factory. Previously, it was just about the factory's ownership of the
product. This CL moves responsibility for the binding to Product as
well, eliminating some duplicated code. This change also introduces
RCHECK, a macro that products can use instead of DCHECK when a failure
should result in the shutdown of the product.
R=kulakowski@chromium.org
Review URL: https://codereview.chromium.org/1945903006 .
diff --git a/services/media/factory_service/factory_service.cc b/services/media/factory_service/factory_service.cc
index d329a43..4a17d82 100644
--- a/services/media/factory_service/factory_service.cc
+++ b/services/media/factory_service/factory_service.cc
@@ -13,12 +13,12 @@
namespace mojo {
namespace media {
-MediaFactoryService::Product::Product(MediaFactoryService* owner)
+MediaFactoryService::ProductBase::ProductBase(MediaFactoryService* owner)
: owner_(owner) {
DCHECK(owner_);
}
-MediaFactoryService::Product::~Product() {}
+MediaFactoryService::ProductBase::~ProductBase() {}
MediaFactoryService::MediaFactoryService() {}
@@ -41,41 +41,42 @@
void MediaFactoryService::CreatePlayer(InterfaceHandle<SeekingReader> reader,
InterfaceRequest<MediaPlayer> player) {
- products_.insert(std::static_pointer_cast<Product>(
+ products_.insert(std::static_pointer_cast<ProductBase>(
MediaPlayerImpl::Create(reader.Pass(), player.Pass(), this)));
}
void MediaFactoryService::CreateSource(InterfaceHandle<SeekingReader> reader,
Array<MediaTypeSetPtr> media_types,
InterfaceRequest<MediaSource> source) {
- products_.insert(std::static_pointer_cast<Product>(MediaSourceImpl::Create(
- reader.Pass(), media_types, source.Pass(), this)));
+ products_.insert(
+ std::static_pointer_cast<ProductBase>(MediaSourceImpl::Create(
+ reader.Pass(), media_types, source.Pass(), this)));
}
void MediaFactoryService::CreateSink(const String& destination_url,
MediaTypePtr media_type,
InterfaceRequest<MediaSink> sink) {
- products_.insert(std::static_pointer_cast<Product>(MediaSinkImpl::Create(
+ products_.insert(std::static_pointer_cast<ProductBase>(MediaSinkImpl::Create(
destination_url, media_type.Pass(), sink.Pass(), this)));
}
void MediaFactoryService::CreateDemux(InterfaceHandle<SeekingReader> reader,
InterfaceRequest<MediaDemux> demux) {
- products_.insert(std::static_pointer_cast<Product>(
+ products_.insert(std::static_pointer_cast<ProductBase>(
MediaDemuxImpl::Create(reader.Pass(), demux.Pass(), this)));
}
void MediaFactoryService::CreateDecoder(
MediaTypePtr input_media_type,
InterfaceRequest<MediaTypeConverter> decoder) {
- products_.insert(std::static_pointer_cast<Product>(
+ products_.insert(std::static_pointer_cast<ProductBase>(
MediaDecoderImpl::Create(input_media_type.Pass(), decoder.Pass(), this)));
}
void MediaFactoryService::CreateNetworkReader(
const String& url,
InterfaceRequest<SeekingReader> reader) {
- products_.insert(std::static_pointer_cast<Product>(
+ products_.insert(std::static_pointer_cast<ProductBase>(
NetworkReaderImpl::Create(url, reader.Pass(), this)));
}
diff --git a/services/media/factory_service/factory_service.h b/services/media/factory_service/factory_service.h
index 62ebc72..309a4ed 100644
--- a/services/media/factory_service/factory_service.h
+++ b/services/media/factory_service/factory_service.h
@@ -20,12 +20,12 @@
public MediaFactory {
public:
// Provides common behavior for all objects created by the factory service.
- class Product : public std::enable_shared_from_this<Product> {
+ class ProductBase : public std::enable_shared_from_this<ProductBase> {
public:
- virtual ~Product();
+ virtual ~ProductBase();
protected:
- Product(MediaFactoryService* owner);
+ ProductBase(MediaFactoryService* owner);
// Returns the ApplicationImpl.
ApplicationImpl* app() {
@@ -43,6 +43,33 @@
MediaFactoryService* owner_;
};
+ template <typename Interface>
+ class Product : public ProductBase {
+ public:
+ virtual ~Product() {}
+
+ protected:
+ Product(Interface* impl,
+ InterfaceRequest<Interface> request,
+ MediaFactoryService* owner)
+ : ProductBase(owner), binding_(impl, request.Pass()) {
+ DCHECK(impl);
+ binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
+ }
+
+ // Closes the binding and calls ReleaseFromOwner.
+ void UnbindAndReleaseFromOwner() {
+ if (binding_.is_bound()) {
+ binding_.Close();
+ }
+
+ ReleaseFromOwner();
+ }
+
+ private:
+ Binding<Interface> binding_;
+ };
+
MediaFactoryService();
~MediaFactoryService() override;
@@ -80,9 +107,20 @@
private:
BindingSet<MediaFactory> bindings_;
ApplicationImpl* app_;
- std::unordered_set<std::shared_ptr<Product>> products_;
+ std::unordered_set<std::shared_ptr<ProductBase>> products_;
};
+// For use by products when handling mojo requests.
+// Checks the condition, and, if it's false, unbinds, releases from the owner
+// and calls return. Doesn't support stream arguments.
+// TODO(dalesat): Support stream arguments.
+#define RCHECK(condition) \
+ if (!(condition)) { \
+ LOG(ERROR) << "request precondition failed: " #condition "."; \
+ UnbindAndReleaseFromOwner(); \
+ return; \
+ }
+
} // namespace media
} // namespace mojo
diff --git a/services/media/factory_service/media_decoder_impl.cc b/services/media/factory_service/media_decoder_impl.cc
index 137d226..2e00494 100644
--- a/services/media/factory_service/media_decoder_impl.cc
+++ b/services/media/factory_service/media_decoder_impl.cc
@@ -21,23 +21,19 @@
MediaDecoderImpl::MediaDecoderImpl(MediaTypePtr input_media_type,
InterfaceRequest<MediaTypeConverter> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner),
- binding_(this, request.Pass()),
+ : MediaFactoryService::Product<MediaTypeConverter>(this,
+ request.Pass(),
+ owner),
consumer_(MojoConsumer::Create()),
producer_(MojoProducer::Create()) {
DCHECK(input_media_type);
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
std::unique_ptr<StreamType> input_stream_type =
input_media_type.To<std::unique_ptr<StreamType>>();
if (Decoder::Create(*input_stream_type, &decoder_) != Result::kOk) {
LOG(WARNING) << "Couldn't find decoder for stream type";
- if (binding_.is_bound()) {
- binding_.Close();
- }
+ UnbindAndReleaseFromOwner();
return;
}
diff --git a/services/media/factory_service/media_decoder_impl.h b/services/media/factory_service/media_decoder_impl.h
index 637d0f7..913169e 100644
--- a/services/media/factory_service/media_decoder_impl.h
+++ b/services/media/factory_service/media_decoder_impl.h
@@ -19,8 +19,9 @@
namespace media {
// Mojo agent that decodes a stream.
-class MediaDecoderImpl : public MediaFactoryService::Product,
- public MediaTypeConverter {
+class MediaDecoderImpl
+ : public MediaFactoryService::Product<MediaTypeConverter>,
+ public MediaTypeConverter {
public:
static std::shared_ptr<MediaDecoderImpl> Create(
MediaTypePtr input_media_type,
@@ -41,7 +42,6 @@
InterfaceRequest<MediaTypeConverter> request,
MediaFactoryService* owner);
- Binding<MediaTypeConverter> binding_;
Graph graph_;
std::shared_ptr<MojoConsumer> consumer_;
std::shared_ptr<Decoder> decoder_;
diff --git a/services/media/factory_service/media_demux_impl.cc b/services/media/factory_service/media_demux_impl.cc
index e53ab44..37478e4 100644
--- a/services/media/factory_service/media_demux_impl.cc
+++ b/services/media/factory_service/media_demux_impl.cc
@@ -26,7 +26,7 @@
MediaDemuxImpl::MediaDemuxImpl(InterfaceHandle<SeekingReader> reader,
InterfaceRequest<MediaDemux> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner), binding_(this, request.Pass()) {
+ : MediaFactoryService::Product<MediaDemux>(this, request.Pass(), owner) {
DCHECK(reader);
task_runner_ = base::MessageLoop::current()->task_runner();
@@ -38,9 +38,6 @@
: nullptr);
});
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
std::shared_ptr<Reader> reader_ptr = MojoReader::Create(reader.Pass());
if (!reader_ptr) {
NOTREACHED() << "couldn't create reader";
@@ -101,7 +98,7 @@
void MediaDemuxImpl::GetProducer(uint32_t stream_index,
InterfaceRequest<MediaProducer> producer) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
if (stream_index >= streams_.size()) {
return;
@@ -116,7 +113,7 @@
}
void MediaDemuxImpl::Prime(const PrimeCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
std::shared_ptr<CallbackJoiner> callback_joiner = CallbackJoiner::Create();
@@ -128,7 +125,7 @@
}
void MediaDemuxImpl::Flush(const FlushCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
graph_.FlushAllOutputs(demux_part_);
@@ -142,7 +139,7 @@
}
void MediaDemuxImpl::Seek(int64_t position, const SeekCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
demux_->Seek(position, [this, callback]() {
task_runner_->PostTask(FROM_HERE, base::Bind(&RunSeekCallback, callback));
diff --git a/services/media/factory_service/media_demux_impl.h b/services/media/factory_service/media_demux_impl.h
index 91e6fbf..0be2435 100644
--- a/services/media/factory_service/media_demux_impl.h
+++ b/services/media/factory_service/media_demux_impl.h
@@ -24,7 +24,8 @@
namespace media {
// Mojo agent that decodes a stream.
-class MediaDemuxImpl : public MediaFactoryService::Product, public MediaDemux {
+class MediaDemuxImpl : public MediaFactoryService::Product<MediaDemux>,
+ public MediaDemux {
public:
static std::shared_ptr<MediaDemuxImpl> Create(
InterfaceHandle<SeekingReader> reader,
@@ -86,7 +87,6 @@
static void RunSeekCallback(const SeekCallback& callback);
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
- Binding<MediaDemux> binding_;
Graph graph_;
PartRef demux_part_;
std::shared_ptr<Demux> demux_;
diff --git a/services/media/factory_service/media_player_impl.cc b/services/media/factory_service/media_player_impl.cc
index ac25409..8fa10af 100644
--- a/services/media/factory_service/media_player_impl.cc
+++ b/services/media/factory_service/media_player_impl.cc
@@ -23,7 +23,7 @@
MediaPlayerImpl::MediaPlayerImpl(InterfaceHandle<SeekingReader> reader,
InterfaceRequest<MediaPlayer> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner), binding_(this, request.Pass()) {
+ : MediaFactoryService::Product<MediaPlayer>(this, request.Pass(), owner) {
DCHECK(reader);
status_publisher_.SetCallbackRunner(
@@ -37,9 +37,6 @@
state_ = State::kWaiting;
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
ConnectToService(app()->shell(), "mojo:media_factory", GetProxy(&factory_));
factory_->CreateDemux(reader.Pass(), GetProxy(&demux_));
diff --git a/services/media/factory_service/media_player_impl.h b/services/media/factory_service/media_player_impl.h
index f9f53df..33a7595 100644
--- a/services/media/factory_service/media_player_impl.h
+++ b/services/media/factory_service/media_player_impl.h
@@ -20,7 +20,7 @@
namespace media {
// Mojo agent that renders streams from an origin specified by URL.
-class MediaPlayerImpl : public MediaFactoryService::Product,
+class MediaPlayerImpl : public MediaFactoryService::Product<MediaPlayer>,
public MediaPlayer {
public:
static std::shared_ptr<MediaPlayerImpl> Create(
@@ -119,7 +119,6 @@
uint64_t version = MediaSink::kInitialStatus,
MediaSinkStatusPtr status = nullptr);
- Binding<MediaPlayer> binding_;
MediaFactoryPtr factory_;
MediaDemuxPtr demux_;
std::vector<std::unique_ptr<Stream>> streams_;
diff --git a/services/media/factory_service/media_sink_impl.cc b/services/media/factory_service/media_sink_impl.cc
index 36aa156..da4b7ef 100644
--- a/services/media/factory_service/media_sink_impl.cc
+++ b/services/media/factory_service/media_sink_impl.cc
@@ -26,16 +26,12 @@
MediaTypePtr media_type,
InterfaceRequest<MediaSink> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner),
- binding_(this, request.Pass()),
+ : MediaFactoryService::Product<MediaSink>(this, request.Pass(), owner),
consumer_(MojoConsumer::Create()),
producer_(MojoProducer::Create()) {
DCHECK(destination_url);
DCHECK(media_type);
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
status_publisher_.SetCallbackRunner(
[this](const GetStatusCallback& callback, uint64_t version) {
MediaSinkStatusPtr status = MediaSinkStatus::New();
@@ -83,13 +79,7 @@
return;
}
- if (destination_url != "mojo:audio_server") {
- LOG(ERROR) << "mojo:audio_server is the only supported destination";
- if (binding_.is_bound()) {
- binding_.Close();
- }
- return;
- }
+ RCHECK(destination_url == "mojo:audio_server");
// TODO(dalesat): Once we have c++14, get rid of this shared pointer hack.
std::shared_ptr<StreamType> captured_stream_type(
diff --git a/services/media/factory_service/media_sink_impl.h b/services/media/factory_service/media_sink_impl.h
index 8d098e9..17db63a 100644
--- a/services/media/factory_service/media_sink_impl.h
+++ b/services/media/factory_service/media_sink_impl.h
@@ -25,7 +25,8 @@
// Mojo agent that consumes a stream and delivers it to a destination specified
// by URL.
-class MediaSinkImpl : public MediaFactoryService::Product, public MediaSink {
+class MediaSinkImpl : public MediaFactoryService::Product<MediaSink>,
+ public MediaSink {
public:
static std::shared_ptr<MediaSinkImpl> Create(
const String& destination_url,
@@ -56,7 +57,6 @@
void MaybeSetRate();
Incident ready_;
- Binding<MediaSink> binding_;
Graph graph_;
std::shared_ptr<MojoConsumer> consumer_;
std::shared_ptr<MojoProducer> producer_;
diff --git a/services/media/factory_service/media_source_impl.cc b/services/media/factory_service/media_source_impl.cc
index 4c6d439..10311bc 100644
--- a/services/media/factory_service/media_source_impl.cc
+++ b/services/media/factory_service/media_source_impl.cc
@@ -31,8 +31,7 @@
const Array<MediaTypeSetPtr>& allowed_media_types,
InterfaceRequest<MediaSource> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner),
- binding_(this, request.Pass()),
+ : MediaFactoryService::Product<MediaSource>(this, request.Pass(), owner),
allowed_media_types_(allowed_media_types.Clone()) {
DCHECK(reader);
@@ -48,9 +47,6 @@
callback.Run(version, status.Pass());
});
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
std::shared_ptr<Reader> reader_ptr = MojoReader::Create(reader.Pass());
if (!reader_ptr) {
NOTREACHED() << "couldn't create reader";
@@ -109,7 +105,7 @@
void MediaSourceImpl::GetProducer(uint32_t stream_index,
InterfaceRequest<MediaProducer> producer) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
if (stream_index >= streams_.size()) {
return;
@@ -121,7 +117,7 @@
void MediaSourceImpl::GetPullModeProducer(
uint32_t stream_index,
InterfaceRequest<MediaPullModeProducer> producer) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
if (stream_index >= streams_.size()) {
return;
@@ -136,7 +132,7 @@
}
void MediaSourceImpl::Prepare(const PrepareCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
for (std::unique_ptr<Stream>& stream : streams_) {
stream->EnsureSink();
@@ -148,7 +144,7 @@
}
void MediaSourceImpl::Prime(const PrimeCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
std::shared_ptr<CallbackJoiner> callback_joiner = CallbackJoiner::Create();
@@ -160,7 +156,7 @@
}
void MediaSourceImpl::Flush(const FlushCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
graph_.FlushAllOutputs(demux_part_);
@@ -174,7 +170,7 @@
}
void MediaSourceImpl::Seek(int64_t position, const SeekCallback& callback) {
- DCHECK(init_complete_.occurred());
+ RCHECK(init_complete_.occurred());
demux_->Seek(position, [this, callback]() {
task_runner_->PostTask(FROM_HERE, base::Bind(&RunSeekCallback, callback));
diff --git a/services/media/factory_service/media_source_impl.h b/services/media/factory_service/media_source_impl.h
index 610daf1..5fdd3ee 100644
--- a/services/media/factory_service/media_source_impl.h
+++ b/services/media/factory_service/media_source_impl.h
@@ -26,7 +26,7 @@
namespace media {
// Mojo agent that produces streams from an origin specified by URL.
-class MediaSourceImpl : public MediaFactoryService::Product,
+class MediaSourceImpl : public MediaFactoryService::Product<MediaSource>,
public MediaSource {
public:
static std::shared_ptr<MediaSourceImpl> Create(
@@ -112,7 +112,6 @@
static void RunSeekCallback(const SeekCallback& callback);
scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
- Binding<MediaSource> binding_;
Array<MediaTypeSetPtr> allowed_media_types_;
Graph graph_;
PartRef demux_part_;
diff --git a/services/media/factory_service/network_reader_impl.cc b/services/media/factory_service/network_reader_impl.cc
index 613dbe7..b0d89a2 100644
--- a/services/media/factory_service/network_reader_impl.cc
+++ b/services/media/factory_service/network_reader_impl.cc
@@ -27,12 +27,8 @@
NetworkReaderImpl::NetworkReaderImpl(const String& url,
InterfaceRequest<SeekingReader> request,
MediaFactoryService* owner)
- : MediaFactoryService::Product(owner),
- binding_(this, request.Pass()),
+ : MediaFactoryService::Product<SeekingReader>(this, request.Pass(), owner),
url_(url) {
- // Go away when the client is no longer connected.
- binding_.set_connection_error_handler([this]() { ReleaseFromOwner(); });
-
NetworkServicePtr network_service;
ConnectToService(app()->shell(), "mojo:network_service",
diff --git a/services/media/factory_service/network_reader_impl.h b/services/media/factory_service/network_reader_impl.h
index ba0e557..d2ccbf3 100644
--- a/services/media/factory_service/network_reader_impl.h
+++ b/services/media/factory_service/network_reader_impl.h
@@ -17,7 +17,7 @@
namespace media {
// Mojo agent that decodes a stream.
-class NetworkReaderImpl : public MediaFactoryService::Product,
+class NetworkReaderImpl : public MediaFactoryService::Product<SeekingReader>,
public SeekingReader {
public:
static std::shared_ptr<NetworkReaderImpl> Create(
@@ -48,7 +48,6 @@
InterfaceRequest<SeekingReader> request,
MediaFactoryService* owner);
- Binding<SeekingReader> binding_;
std::string url_;
URLLoaderPtr url_loader_;
MediaResult result_ = MediaResult::OK;
