[Media Router] Use DialMediaSinkService in MediaRouterMojoImpl

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

Index: chrome/browser/media/router/discovery/dial/dial_media_sink_service_delegate.h
diff --git a/chrome/browser/media/router/discovery/dial/dial_media_sink_service_delegate.h b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_delegate.h
new file mode 100644
index 0000000000000000000000000000000000000000..d1d1699d83e2d4d25f7b485dcf8c24e82f5654e1
--- /dev/null
+++ b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_delegate.h
@@ -0,0 +1,70 @@
+// 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_DELEGATE_H_
+#define CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DIAL_MEDIA_SINK_SERVICE_DELEGATE_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 DialMediaSinkServiceDelegate

[mark a. foltz, 2017/05/12 21:37:37]

Bikeshed:

Usually I would see a "delegate" and think of an interface that allows you to
implement part of the DialMediaSinkService with another object.

In this case, this class is acting as a wrapper for the DialMediaSinkService to
allow it to be used on the UI thread.

One object design would then be:

DialMediaSinkService => virtual interface with Start()/Stop(), calling code is
thread-agnostic
DialMediaSinkServiceImpl => Implements DialMediaSinkService, runs on IO thread
DialMediaSinkServiceProxy => Same as the current Delegate, implements
DialMediaSinkService, allows Impl to be constructed and used on any other thread

So I would propose naming this DialMediaSinkServiceProxy and (optionally)
extracting a DialMediaSinkService interface to capture the common API between
this class and the implementation.

What do you think?



 

[zhaobin, 2017/05/12 23:03:38]

Sure. Will do refactor in another patch and update this patch later.

+    : public base::RefCountedThreadSafe<DialMediaSinkServiceDelegate> {
+ public:
+  DialMediaSinkServiceDelegate();
+
+  // Store |callback| and post task to IO thread.

[mark a. foltz, 2017/05/12 21:37:37]

Suggested comment:

Starts discovery of DIAL devices on IO thread.  |callback| is invoked on the UI
thread when sinks are discovered.  |request_context| is used for HTTP requests
made for discovery.

[zhaobin, 2017/05/12 23:03:38]

Done.

+  void Start(const MediaSinkService::OnSinksDiscoveredCallback& callback,
+             net::URLRequestContextGetter* request_context);
+
+  // Clear |sink_discovery_callback_| and post task to IO thread.

[mark a. foltz, 2017/05/12 21:37:37]

Suggest:

Stops discovery of DIAL devices on IO thread.  The callback passed to Start() is
cleared.

[zhaobin, 2017/05/12 23:03:38]

Done.

+  void Stop();
+
+  void SetDialMediaSinkServiceForTest(
+      std::unique_ptr<DialMediaSinkService,
+                      content::BrowserThread::DeleteOnIOThread>
+          dial_media_sink_service);
+
+ private:
+  friend class DialMediaSinkServiceDelegateTest;
+  friend class base::RefCountedThreadSafe<DialMediaSinkServiceDelegate>;
+  ~DialMediaSinkServiceDelegate();
+
+  // Create |dial_media_sink_service_| if none exists before and start DIAL

[mark a. foltz, 2017/05/12 21:37:37]

Creates

[zhaobin, 2017/05/12 23:03:38]

Done.

+  // discvoery.

[mark a. foltz, 2017/05/12 21:37:37]

typo in discovery

[zhaobin, 2017/05/12 23:03:38]

Done.

+  void StartOnIOThread(net::URLRequestContextGetter* request_context);
+
+  // Stop DIAL discovery.

[mark a. foltz, 2017/05/12 21:37:37]

Stops

[zhaobin, 2017/05/12 23:03:38]

Done.

+  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);
+
+  // Invoke |sink_discovery_callback_| on UI thread.

[mark a. foltz, 2017/05/12 21:37:37]

Invokes

[zhaobin, 2017/05/12 23:03:38]

Done.

+  void OnSinksDiscoveredOnUIThread(const std::vector<MediaSinkInternal>& sinks);
+
+ private:
+  std::unique_ptr<DialMediaSinkService,
+                  content::BrowserThread::DeleteOnIOThread>
+      dial_media_sink_service_;
+
+  MediaSinkService::OnSinksDiscoveredCallback sink_discovery_ui_callback_;
+};
+
+}  // namespace media_router
+
+#endif  // CHROME_BROWSER_MEDIA_ROUTER_DISCOVERY_DIAL_DIAL_MEDIA_SINK_SERVICE_DELEGATE_H_