[Media Router] Use DialMediaSinkService in MediaRouterMojoImpl

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

Index: chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc
diff --git a/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b083e14ca124ba4701233303391862cd03b57212
--- /dev/null
+++ b/chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.cc
@@ -0,0 +1,80 @@
+// 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.
+
+#include "chrome/browser/media/router/discovery/dial/dial_media_sink_service_proxy.h"
+
+#include "chrome/browser/media/router/discovery/dial/dial_media_sink_service.h"
+#include "content/public/browser/browser_context.h"
+
+using content::BrowserThread;
+
+namespace media_router {
+
+DialMediaSinkServiceProxy::DialMediaSinkServiceProxy(
+    const MediaSinkService::OnSinksDiscoveredCallback& callback,
+    content::BrowserContext* context)
+    : MediaSinkService(callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  auto* profile = Profile::FromBrowserContext(context);
+  request_context_ = profile->GetRequestContext();
+  DCHECK(request_context_);
+}
+
+DialMediaSinkServiceProxy::~DialMediaSinkServiceProxy() = default;
+
+void DialMediaSinkServiceProxy::Start() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&DialMediaSinkServiceProxy::StartOnIOThread, this));
+}
+
+void DialMediaSinkServiceProxy::Stop() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+

[mark a. foltz, 2017/05/17 18:16:20]

Do you need to reset |sink_discovery_callback_| here?

[zhaobin, 2017/05/17 22:36:47]

Leave |sink_discovery_callback| so we can support Start()/Stop()/Start(). Make
underlying DialMediaSinkService controls timer in Start()/Stop() so that
callback wont be invoked after Stop().

+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&DialMediaSinkServiceProxy::StopOnIOThread, this));
+}
+
+void DialMediaSinkServiceProxy::SetDialMediaSinkServiceForTest(
+    std::unique_ptr<DialMediaSinkService> dial_media_sink_service) {
+  DCHECK(dial_media_sink_service);
+  DCHECK(!dial_media_sink_service_);
+  dial_media_sink_service_ = std::move(dial_media_sink_service);
+}
+
+void DialMediaSinkServiceProxy::StartOnIOThread() {
+  if (!dial_media_sink_service_) {
+    dial_media_sink_service_ = base::MakeUnique<DialMediaSinkService>(
+        base::Bind(&DialMediaSinkServiceProxy::OnSinksDiscoveredOnIOThread,
+                   base::Unretained(this)),

[mark a. foltz, 2017/05/17 18:16:20]

It would be more consistent IMO to pass |this| to base::Bind as it is
refcounted.  In general passing raw pointers to refcounted objects is
discouraged - it subverts the purpose of refcounting which is to manage the
lifetime of an object based on every user holding a reference.

If the destruction order is as you describe, destroying
|dial_media_sink_service| should also delete the last reference held to |this|. 
However, you would need to explicitly delete DMMS outside the dtor of this class
(i.e. in StopOnIOThread()) to avoid a cycle.

You could also not make this not refcounted at all.  Is there a reason for
refcounting?

[zhaobin, 2017/05/17 22:36:48]

Changed to |this| and reset in StopOnIOThread().

+        request_context_);
+  }
+
+  dial_media_sink_service_->Start();
+}
+
+void DialMediaSinkServiceProxy::StopOnIOThread() {
+  DCHECK(dial_media_sink_service_);
+  dial_media_sink_service_->Stop();

[mark a. foltz, 2017/05/17 18:16:20]

Should you delete dial_media_sink_service_?

[zhaobin, 2017/05/17 22:36:47]

Done.

+}
+
+void DialMediaSinkServiceProxy::OnSinksDiscoveredOnIOThread(
+    const std::vector<MediaSinkInternal>& sinks) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&DialMediaSinkServiceProxy::OnSinksDiscoveredOnUIThread, this,
+                 std::vector<MediaSinkInternal>(sinks.begin(), sinks.end())));
+}
+
+void DialMediaSinkServiceProxy::OnSinksDiscoveredOnUIThread(
+    const std::vector<MediaSinkInternal>& sinks) {
+  if (sink_discovery_callback_)
+    sink_discovery_callback_.Run(sinks);
+}
+
+}  // namespace media_router