[Device Discovery] Make DialRegistry a Singleton

chrome/browser/extensions/api/dial/dial_api.cc

Index: chrome/browser/extensions/api/dial/dial_api.cc
diff --git a/chrome/browser/extensions/api/dial/dial_api.cc b/chrome/browser/extensions/api/dial/dial_api.cc
index 415ae15dce5e13b6b05ebc49be296b64fb4c74cd..bf217be39bf81db5f94ebcdee2148b68f926ced4 100644
--- a/chrome/browser/extensions/api/dial/dial_api.cc
+++ b/chrome/browser/extensions/api/dial/dial_api.cc
@@ -13,6 +13,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/extensions/api/dial/dial_api_factory.h"
 #include "chrome/browser/media/router/discovery/dial/device_description_fetcher.h"
+#include "chrome/browser/media/router/discovery/dial/dial_registry.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/extensions/api/dial.h"
 #include "content/public/browser/browser_thread.h"
@@ -29,41 +30,30 @@ using media_router::DialRegistry;
 
 namespace extensions {
 
-namespace {
-
-// How often to poll for devices.
-const int kDialRefreshIntervalSecs = 120;
-
-// We prune a device if it does not respond after this time.
-const int kDialExpirationSecs = 240;
-
-// The maximum number of devices retained at once in the registry.
-const size_t kDialMaxDevices = 256;
-
-}  // namespace
-
 DialAPI::DialAPI(Profile* profile)
     : RefcountedKeyedService(
           BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)),
-      profile_(profile) {
+      profile_(profile),
+      dial_registry_(nullptr) {
   EventRouter::Get(profile)->RegisterObserver(
       this, api::dial::OnDeviceList::kEventName);
 }
 
-DialAPI::~DialAPI() {}
+DialAPI::~DialAPI() {
+  if (dial_registry_)
+    dial_registry_->StopPeriodicDiscovery();

[imcheng, 2017/03/23 23:06:00]

Wouldn't that affect other observers of DialRegistry? It seems it should
unregister itself from DialRegistry instead?

[zhaobin, 2017/03/24 00:42:35]

Yes. It is fine for now since DialAPI is the only observer; and dtor is invoked
when IO thread tears down (We only tear down IO thread when we close the
browser?)

Yes, we should UnregisterObserver() instead. StopPeriodicDiscovery() is a temp
workaround here. Current impl does not StopDiscovery() when there is no observer
(it only StopDiscovery() when there is no listener). Need to do some refactoring
regarding when to start/stop discovery in DialRegistry when DialMediaSinkService
is ready. Trying to keep this patch simple for now... 

[imcheng, 2017/03/24 00:54:24]

Ok. Seems like the easiest way would be to have this class keep a listener count
and Register/UnregisterObserver as needed. Could you add a TODO here?

[zhaobin, 2017/03/24 20:00:14]

Done.

+}
 
 DialRegistry* DialAPI::dial_registry() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!dial_registry_.get()) {
-    dial_registry_.reset(new DialRegistry(
-        TimeDelta::FromSeconds(kDialRefreshIntervalSecs),
-        TimeDelta::FromSeconds(kDialExpirationSecs), kDialMaxDevices));
+  if (!dial_registry_) {
+    dial_registry_ = media_router::DialRegistry::GetInstance();
     dial_registry_->RegisterObserver(this);
     if (test_device_data_) {
       dial_registry_->AddDeviceForTest(*test_device_data_);
     }
   }
-  return dial_registry_.get();
+  return dial_registry_;
 }
 
 void DialAPI::OnListenerAdded(const EventListenerInfo& details) {