[Media Router] Design MediaRouter interface with stub implementation.

chrome/browser/media/router/media_router.h

Index: chrome/browser/media/router/media_router.h
diff --git a/chrome/browser/media/router/media_router.h b/chrome/browser/media/router/media_router.h
new file mode 100644
index 0000000000000000000000000000000000000000..9198137881f16ab21b3941a81ed27f985afc866f
--- /dev/null
+++ b/chrome/browser/media/router/media_router.h
@@ -0,0 +1,90 @@
+// Copyright 2015 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_ROUTER_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_H_
+
+#include <string>
+#include <vector>
+
+#include "chrome/browser/media/router/media_route_id.h"
+#include "chrome/browser/media/router/media_sink.h"
+
+namespace media_router {
+
+class MediaRoute;
+class MediaRouteResponse;
+class MediaRoutesObserver;
+class MediaSource;
+class MediaSinksObserver;
+
+using MediaRouteResponseCallback =
+    base::Callback<void(const MediaRouteResponse&)>;

[xhwang, 2015/04/02 17:13:02]

Add include for base::Callback.

[imcheng, 2015/04/02 23:05:24]

Done.

+
+// An interface for handling resources related to media routing.
+// Responsible for registering observers for receiving sink availability
+// updates, handling route requests/responses, and operating on routes (e.g.
+// posting messages or closing).
+class MediaRouter {
+ public:
+  virtual ~MediaRouter() {}
+
+  // Requests a media route from |source| to |sink_id|.
+  // |callback| is invoked with a response indicating success or failure.
+  // If it is not possible to request a route, returns kInvalidRouteRequestId.
+  // Otherwise a valid RouteRequestId will be returned, which can be used with

[xhwang, 2015/04/02 17:13:02]

nit: What's a definition of a valid RouteRequestId?

id != kInvalidRouteRequestId

or 

id > kInvalidRouteRequestId

?

[imcheng, 2015/04/02 23:05:24]

id != kInvalidRouteRequestId. Added comment.

[xhwang, 2015/04/06 20:51:30]

hmm, so the id can be negative?

[imcheng, 2015/04/06 21:54:20]

Our implementation actually allocates non-negative route request IDs, so let's
say that it must be id > kInvalidRouteRequestId here.

+  // |UnregisterMediaRouteResponseCallback()| if the caller no longer wants
+  // the callback invoked when a response comes back.
+  virtual RouteRequestId StartRouteRequest(
+      const MediaSource& source,
+      const MediaSinkId& sink_id,
+      const MediaRouteResponseCallback& callback) = 0;
+
+  // Unregisters a callback registered with a pending media route request,
+  // e.g., when the user has canceled presentation by closing the media router
+  // dialog.
+  // It does NOT cancel the route request, only the callback previously
+  // registered via |StartRouteRequest|.

[xhwang, 2015/04/02 17:13:02]

This is a bit odd (from interface's perspective; haven't looked at the impl
yet):

1, if the MediaRouter impl actually CAN cancel the route request, should it do
so?

2, The callback may be posted already and waiting in the message loop. In that
case, you have no way to cancel the callback. Are you saying the caller may just
ignore the callback?

[imcheng, 2015/04/02 23:05:24]

1. It currently cannot cancel the route request, and there's no use case that
requires it.
2. The implementation does not post the callbacks to a message loop. Instead it
keeps the callbacks in a map and matched (by request id) and invoked when a
response comes back. Thus the callback can easily be cancelled by removing the
callback from the map.

+  // |request_id|: A valid request ID returned by |StartRouteRequest()|.
+  virtual void UnregisterMediaRouteResponseCallback(

[xhwang, 2015/04/02 17:13:02]

How about just CancelRouteRequest()? Unregistering the callback is really just
an implementation detail.

[imcheng, 2015/04/02 23:05:24]

The call doesn't cancel the route request.

+      const RouteRequestId& request_id) = 0;
+
+  // Closes the media route specified by |route_id|.
+  virtual void CloseRoute(const MediaRouteId& route_id) = 0;
+
+  // Registers |observer| with this MediaRouter. |observer| specifies a media
+  // source and will receive updates with media sinks that are compatible with
+  // that source. The initial update may happen synchronously.
+  // NOTE: This class does not assume ownership of |observer|. Callers must
+  // manage |observer| and make sure |UnregisterObserver()| is called
+  // before the observer is destroyed.
+  // Returns true if registration succeeded or the |observer| already exists.

[xhwang, 2015/04/02 17:13:02]

Should "|observer| already exists" ever happen? What's the legit use case for
that?

[imcheng, 2015/04/02 23:05:24]

It should not happen because each observer should only be registered at most
once (w/o unregistering of course). Would you prefer it if the function enforced
this and CHECK'ed instead?

[xhwang, 2015/04/06 20:51:29]

Yes. Please make the interface simple and clean. Comment like this make it hard
to understand what's the expected behavior. Please update this comment, and
DCHECK() in code.

[imcheng, 2015/04/06 21:54:20]

Ok, I updated the comment to say it is invalid to register the same observer
twice.

+  // If the MRPM Host is not available, the registration request will fail
+  // immediately.
+  virtual bool RegisterMediaSinksObserver(MediaSinksObserver* observer) = 0;
+
+  // Removes a previously added MediaSinksObserver. |observer| will stop
+  // receiving further updates.
+  virtual void UnregisterMediaSinksObserver(MediaSinksObserver* observer) = 0;
+
+  // Posts |message| to a MediaSink connected via MediaRoute with |route_id|.
+  // TODO(imcheng): Support additional data types: Blob, ArrayBuffer,
+  // ArrayBufferView.
+  virtual void PostMessage(const MediaRouteId& route_id,
+                           const std::string& message) = 0;
+
+  // Adds a MediaRoutesObserver to listen for updates on MediaRoutes.
+  // The initial update may happen synchronously.
+  // MediaRouter does not own |observer|. |RemoveMediaRoutesObserver| should
+  // be called before |observer| is destroyed.
+  virtual bool RegisterMediaRoutesObserver(MediaRoutesObserver* observer) = 0;
+
+  // Removes a previously added MediaRoutesObserver. |observer| will stop
+  // receiving further updates.
+  virtual void UnregisterMediaRoutesObserver(MediaRoutesObserver* observer) = 0;
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTER_H_