[Media Router] Use DialMediaSinkService in MediaRouterMojoImpl

chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h

Index: chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h
diff --git a/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h
new file mode 100644
index 0000000000000000000000000000000000000000..10895b0fa2ad4701f1a3997076e700fb1086e501
--- /dev/null
+++ b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h
@@ -0,0 +1,72 @@
+// 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_DISCOVERY_DIAL_DIAL_MEDIA_SINK_SERVICE_PROXY_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DIAL_MEDIA_SINK_SERVICE_PROXY_H_
+
+#include <memory>
+#include "chrome/common/media_router/discovery/media_sink_service.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace net {
+class URLRequestContextGetter;
+}
+
+namespace media_router {
+
+class DialMediaSinkService;
+
+// A wrapper class of DialMediaSinkService handling thread hopping between UI
+// and IO threads. This class is thread safe. Public APIs should be invoked on
+// UI thread. It then post tasks to IO thread and invoke them on underlying
+// DialMediaSinkService instance.
+class DialMediaSinkServiceProxy
+    : public MediaSinkService,

[Kevin M, 2017/05/15 19:56:58]

Can MediaSinkService be made into a 100% pure virtual interface? It is jarring
to see members of MediaSinkService implementation appear in the proxy code, like
the base class' callback field.

Instead of having the proxy responsible for instantiating the concrete service
impl object, it can just take the underlying impl object as a ctor argument.
Then you don't need a SetForTest() since the product and test code will use the
same codepath!

[mark a. foltz, 2017/05/15 21:36:28]

I think refactoring the inheritance is in progress in a different patch.

[zhaobin, 2017/05/16 21:26:49]

Both DialMediaSinkServiceProxy and DialMediaSinkService inherit from
MediaServiceSink. They take different sink_discovery_callback_.
DialMediaSinkServiceProxy gets MediaRouterMojoImpl::ProvideSinks(), should be
invoked on UI thread; DialMediaSinkService gets
DialMediaSinkServiceProxy::OnSinksDiscoveredOnIOThread(), should be invoked on
IO thread. This proxy class is RefCountedThreadSafe and handles thread switching
between IO and UI so MediaRouterMojoImpl lives on UI thread while
DialMediaSinkService lives on IO thread.

+      public base::RefCountedThreadSafe<DialMediaSinkServiceProxy> {
+ public:
+  // |callback| is invoked on the UI thread when sinks are discovered.
+  // |request_context| is used for HTTP requests made for discovery.
+  DialMediaSinkServiceProxy(
+      const MediaSinkService::OnSinksDiscoveredCallback& callback,
+      net::URLRequestContextGetter* request_context);

[Kevin M, 2017/05/15 19:56:58]

~DialMediaSinkServiceProxy = default;

[mark a. foltz, 2017/05/15 21:36:28]

That should go in the .cc, as it will invoke the DialMediaSinkService dtor.

[zhaobin, 2017/05/16 21:26:49]

RefCountedThreadSafe class needs private dtor.

+
+  // Starts discovery of DIAL devices on IO thread.
+  void Start() override;
+
+  // Stops discovery of DIAL devices on IO thread.  The callback passed to
+  // Start() is cleared.
+  void Stop();
+
+  void SetDialMediaSinkServiceForTest(
+      std::unique_ptr<DialMediaSinkService,
+                      content::BrowserThread::DeleteOnIOThread>
+          dial_media_sink_service);
+
+ private:
+  friend class DialMediaSinkServiceProxyTest;
+  friend class base::RefCountedThreadSafe<DialMediaSinkServiceProxy>;
+  ~DialMediaSinkServiceProxy() override;
+
+  // Starts DIAL discovery.
+  void StartOnIOThread();
+
+  // Stops DIAL discovery.
+  void StopOnIOThread();
+
+  // Callback passed to |dial_media_sink_service_| ctor. When invoked, post task
+  // to UI thread and invoke |sink_discovery_callback_|;
+  void OnSinksDiscoveredOnIOThread(const std::vector<MediaSinkInternal>& sinks);
+
+  // Invokes |sink_discovery_callback_| on UI thread.
+  void OnSinksDiscoveredOnUIThread(const std::vector<MediaSinkInternal>& sinks);
+
+ private:
+  std::unique_ptr<DialMediaSinkService,
+                  content::BrowserThread::DeleteOnIOThread>
+      dial_media_sink_service_;
+};

[Kevin M, 2017/05/15 19:56:58]

nit: DISALLOW_COPY_AND_ASSIGN(DialMediaSinkServiceProxy);

[zhaobin, 2017/05/16 21:26:49]

Done.

+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DIAL_MEDIA_SINK_SERVICE_PROXY_H_