[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..ec44258d2702405a5254423d3282de9eb5e029d1
--- /dev/null
+++ b/chrome/browser/media/router/media_router.h
@@ -0,0 +1,101 @@
+// 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 "base/callback.h"
+#include "chrome/browser/media/router/media_route.h"
+#include "chrome/browser/media/router/media_route_id.h"
+#include "chrome/browser/media/router/media_sink.h"
+
+namespace media_router {
+
+class MediaRouteResponse;
+class MediaRoutesObserver;
+class MediaSource;
+class MediaSinksObserver;
+
+using MediaRouteResponseCallback =
+    base::Callback<void(const MediaRouteResponse&)>;
+
+// 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 (id > kInvalidRouteRequestId) will be
+  // returned, which can be used with |UnregisterMediaRouteResponseCallback()|
+  // if the caller no longer wants the callback invoked when a response
+  // comes back.
+  virtual RouteRequestId StartRouteRequest(

[xhwang, 2015/04/08 17:46:25]

(See comment above.) 

Does this correspond to RequestRoute() on the MediaRouteProviderManagerHost
interface? If so, can we make the naming more consistent?

In MediaRouteProviderManagerHost you use a delegate, but here you use a
callback. Can we be consistent?

[imcheng, 2015/04/08 19:21:31]

Renamed to RequestRoute(). For delegate vs. cb, please see comment above.

+      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|.
+  // |request_id|: A valid request ID returned by |StartRouteRequest()|.
+  virtual void UnregisterMediaRouteResponseCallback(
+      const RouteRequestId& request_id) = 0;

[xhwang, 2015/04/08 17:46:25]

Since we don't actually cancel the request, what's the value of unregistering
the callback? Are we trying to avoid access violation because maybe the caller
of StartRouteRequest() has been destructed?

Is this what you'd expect in implementation? After this call, the request will
still finish, but then we found the callback has already been unregistered, so
we'll drop the result on the floor and do nothing.

If this is the case, usually we don't need to unregister the callback
explicitly. You can use a weak pointer to bind the callback. When the caller of
StartRouteRequest() is destroyed, all callbacks will do nothing. This solves the
lifetime issue and unregistering callbacks automatically.

let me know if I am missing something though.

[imcheng, 2015/04/08 19:21:31]

What you described is correct. However removing this API also means it will
force all users of RequestRoutes to support WeakPtrs. Is that an ok thing to do?

[xhwang, 2015/04/08 20:32:51]

We use WeakPtrs in many many places in Chromium and media/:

https://code.google.com/p/chromium/codesearch#search/&q=weak_ptr&sq=package:c...

I don't see any problem having all callers of RequestRoute() to manage the
lifetime themselves.

[imcheng, 2015/04/08 23:28:09]

Removed API.

+
+  // Closes the media route specified by |route_id|.
+  virtual void CloseRoute(const MediaRouteId& route_id) = 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;
+
+ private:
+  friend class MediaSinksObserver;
+  friend class MediaRoutesObserver;

[xhwang, 2015/04/08 17:46:25]

Does MediaSinkObserver and MediaRoutesObserver correspond to OnSinksReceived()
and OnRoutesUpdated() respectively?

ditto for Delegate vs Observer pattern.

[imcheng, 2015/04/08 19:21:31]

Yes.

+
+  // The following APIs are called by MediaSinksObserver/MediaRoutesObserver
+  // only.
+
+  // 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.
+  // It is invalid to request the same observer more than once and will result
+  // in undefined behavior.
+  // 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;
+
+  // 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_