[Media Router] (device discovery) Create MediaSinkService interface

chrome/browser/media/router/media_sink_service.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_MEDIA_ROUTER_MEDIA_SINK_SERVICE_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_SINK_SERVICE_H_
+
+#include <memory>
+#include <vector>
+
+#include "base/callback.h"
+#include "chrome/browser/media/router/media_sink.h"
+
+namespace media_router {
+
+class MediaSinksObserver;
+
+// A service which can be used to start device discovery and resolution in the

[mark a. foltz, 2017/02/09 23:47:52]

... to start background discovery and resolution of MediaSinks.  Often these are
remote devices, like Chromecast.

[zhaobin, 2017/02/14 00:42:17]

Done.

+// background to generate MediaSinks. In addition, the service is capable of
+// answering MediaSink queries using the sinks that it generated.
+// This class is not thread safe. All methods must be called from the UI thread.

[mark a. foltz, 2017/02/09 23:47:51]

I expect most of the code that implements actual sink discovery to run on the IO
thread (i.e., using net::).
Do you plan on copying data from the IO thread each time a sink is discovered?

[zhaobin, 2017/02/14 00:42:16]

Done.

+class MediaSinkService {
+ public:
+  // Callback used for |Start()|.
+  // Arg 0: Sinks discovered and resolved by the service.
+  using OnSinksDiscoveredCallback =
+      base::Callback<std::vector<std::unique_ptr<MediaSink>>>;

[mark a. foltz, 2017/02/09 23:47:52]

Why does this need to pass ownership of the sinks it's discovered?

[zhaobin, 2017/02/14 00:42:16]

Done.

+
+  virtual ~MediaSinkService() = default;
+
+  // Starts sink discovery. No-ops if already started.

[mark a. foltz, 2017/02/09 23:47:52]

Maybe the callback should be passed in the ctor if there's no reason to pass a
different one?

[zhaobin, 2017/02/14 00:42:16]

Done.

+  // Sinks discovered and resolved are continuously passed to
+  // |callback|.
+  virtual void Start(const OnSinksDiscoveredCallback& callback) = 0;
+
+  // Adds a sink query to observe for MediaSink updates.
+  // Only one observer can be added for a given MediaSource.

[mark a. foltz, 2017/02/09 23:47:52]

Where is the MediaSource passed?

What happens if a duplicate observer/source is registered?

[zhaobin, 2017/02/14 00:42:17]

We can get MediaSource from observer->source().

We will add a DCHECK or use existing DCHECK to prevent duplicate observer.

[mark a. foltz, 2017/02/15 01:01:02]

That relies on the caller to keep track of a source -> observer map to ensure
that no attempt is made to register a duplicate.

IMO, this API would be easier to use if it:
- Allowed multiple observers per source
- Or, unregistered an existing observer for a source if a new one is registered.

+  // Start() must be called first. This class does not take
+  // ownership of |observer|.
+  virtual void AddSinkQuery(MediaSinksObserver* observer) = 0;
+
+  // Removes a sink query and stop observing for MediaSink updates.

[mark a. foltz, 2017/02/09 23:47:52]

...stops observing MediaSink...

[zhaobin, 2017/02/14 00:42:16]

Done.

+  virtual void RemoveSinkQuery(MediaSinksObserver* observer) = 0;

[mark a. foltz, 2017/02/09 23:47:52]

What happens if called with an observer not registered with this class?

[zhaobin, 2017/02/14 00:42:17]

no-op if observer does not exist.

+};

[mark a. foltz, 2017/02/09 23:47:52]

Allocate a base::ThreadChecker to enforce thread safety?

Should this DISALLOW_COPY_AND_ASSIGN?

[zhaobin, 2017/02/14 00:42:16]

Done.

+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_SINK_SERVICE_H_