Populating browser list in WebRTCDeviceProvider.

chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc

Index: chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
diff --git a/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc b/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
index b8491944a9984839576946552e871e09333814bf..86ad647deb89280108daf2cbcfa89646fd83aada 100644
--- a/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
+++ b/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
@@ -5,6 +5,9 @@
 #include "chrome/browser/devtools/device/webrtc/webrtc_device_provider.h"
 
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/local_discovery/cloud_device_list.h"
+#include "chrome/browser/local_discovery/cloud_device_list_delegate.h"
+#include "chrome/browser/local_discovery/gcd_api_flow.h"
 #include "chrome/browser/signin/profile_identity_provider.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/url_constants.h"
@@ -20,6 +23,10 @@
 #include "net/socket/stream_socket.h"
 #include "ui/base/page_transition_types.h"
 
+using BrowserInfo = AndroidDeviceManager::BrowserInfo;
+using BrowserInfoList = std::vector<BrowserInfo>;
+using DeviceInfo = AndroidDeviceManager::DeviceInfo;
+using Serials = std::vector<std::string>;

[dgozman, 2014/12/08 14:27:21]

Let's be consistent: SerialsList.

[SeRya, 2014/12/08 15:01:22]

Done.

 using content::BrowserThread;
 using content::NotificationDetails;
 using content::NotificationObserver;
@@ -29,24 +36,35 @@ using content::Source;
 using content::WebContents;
 using content::WebUIDataSource;
 using content::WebUIMessageHandler;
+using local_discovery::CloudDeviceList;
+using local_discovery::CloudDeviceListDelegate;
+using local_discovery::GCDApiFlow;
 
 namespace {
 
 const char kBackgroundWorkerURL[] =
     "chrome://webrtc-device-provider/background_worker.html";
-
+const char kSerial[] = "clouddevices";

[dgozman, 2014/12/08 14:27:21]

nit: add empty line after this please.

[SeRya, 2014/12/08 15:01:22]

Done.

 }  // namespace
 
 // Lives on the UI thread.
 class WebRTCDeviceProvider::DevToolsBridgeClient :
     private NotificationObserver,
-    private IdentityProvider::Observer {
+    private IdentityProvider::Observer,
+    private CloudDeviceListDelegate {
  public:
   static base::WeakPtr<DevToolsBridgeClient> Create(
       Profile* profile, ProfileIdentityProvider* identity_provider);
 
   void DeleteSelf();
 
+  void UpdateBrowserList();

[dgozman, 2014/12/08 14:27:22]

this one is private

[SeRya, 2014/12/08 15:01:22]

Done.

+  BrowserInfoList const& GetRemoteBrowsers() const { return browsers_; }

[dgozman, 2014/12/08 14:27:22]

nit: this is not used.

[SeRya, 2014/12/08 15:01:22]

Done.

+
+  static Serials GetDevices(base::WeakPtr<DevToolsBridgeClient> weak_ptr);

[dgozman, 2014/12/06 13:16:33]

Why not use an instance method with callback instead of static?

[SeRya, 2014/12/07 13:40:46]

Non-void method can't be bound to a weak reference. And it makes sense: the call
may not be swallowed if the object died. It must return something.

+  static DeviceInfo GetDeviceInfo(base::WeakPtr<DevToolsBridgeClient> weak_ptr,
+                                  const std::string& serial);
+
  private:
   DevToolsBridgeClient(Profile* profile,
                        ProfileIdentityProvider* identity_provider);
@@ -64,10 +82,18 @@ class WebRTCDeviceProvider::DevToolsBridgeClient :
                const NotificationSource& source,
                const NotificationDetails& details) override;
 
+  // CloudDeviceListDelegate implementation.
+  void OnDeviceListReady(
+      const CloudDeviceListDelegate::DeviceList& devices) override;
+  void OnDeviceListUnavailable() override;
+
   Profile* const profile_;
   ProfileIdentityProvider* const identity_provider_;
   NotificationRegistrar registrar_;
   scoped_ptr<WebContents> background_worker_;
+  scoped_ptr<GCDApiFlow> browser_list_request_;
+  BrowserInfoList browsers_;
+  bool browser_list_updating_;
   base::WeakPtrFactory<DevToolsBridgeClient> weak_factory_;
 };
 
@@ -122,6 +148,7 @@ WebRTCDeviceProvider::DevToolsBridgeClient::DevToolsBridgeClient(
     Profile* profile, ProfileIdentityProvider* identity_provider)
   : profile_(profile),
     identity_provider_(identity_provider),
+    browser_list_updating_(false),
     weak_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -144,6 +171,40 @@ void WebRTCDeviceProvider::DevToolsBridgeClient::DeleteSelf() {
   delete this;
 }
 
+void WebRTCDeviceProvider::DevToolsBridgeClient::UpdateBrowserList() {
+  if (browser_list_updating_ || !browser_list_request_.get())
+      return;

[dgozman, 2014/12/06 13:16:33]

nit: indentation

[SeRya, 2014/12/08 15:01:22]

Done.

+  browser_list_updating_ = true;
+
+  browser_list_request_->Start(make_scoped_ptr(new CloudDeviceList(this)));
+}
+
+// static
+Serials WebRTCDeviceProvider::DevToolsBridgeClient::GetDevices(
+    base::WeakPtr<DevToolsBridgeClient> weak_ptr) {
+  Serials result;
+  if (auto* ptr = weak_ptr.get()) {
+    if (ptr->background_worker_.get())
+      result.push_back(kSerial);
+
+    ptr->UpdateBrowserList();
+  }
+  return result;
+}
+
+// static
+DeviceInfo WebRTCDeviceProvider::DevToolsBridgeClient::GetDeviceInfo(
+    base::WeakPtr<DevToolsBridgeClient> weak_ptr,
+    const std::string& serial) {
+  DeviceInfo result;
+  if (auto* ptr = weak_ptr.get()) {
+    result.connected = ptr->background_worker_.get();
+    result.model = "Remote browsers";

[dgozman, 2014/12/08 14:27:21]

nit: make a named constant

[SeRya, 2014/12/08 15:01:22]

Done.

+    result.browser_info = ptr->browsers_;
+  }
+  return result;
+}
+
 void WebRTCDeviceProvider::DevToolsBridgeClient::CreateBackgroundWorker() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -161,6 +222,11 @@ void WebRTCDeviceProvider::DevToolsBridgeClient::CreateBackgroundWorker() {
 
   background_worker_->GetWebUI()->AddMessageHandler(
       new MessageHandler(this));
+
+  browser_list_request_ =
+      GCDApiFlow::Create(profile_->GetRequestContext(),
+                         identity_provider_->GetTokenService(),
+                         identity_provider_->GetActiveAccountId());
 }
 
 void WebRTCDeviceProvider::DevToolsBridgeClient::Observe(
@@ -181,6 +247,31 @@ void WebRTCDeviceProvider::DevToolsBridgeClient::OnActiveAccountLogin() {
 void WebRTCDeviceProvider::DevToolsBridgeClient::OnActiveAccountLogout() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   background_worker_.reset();
+  browser_list_request_.reset();
+  BrowserInfoList().swap(browsers_);

[dgozman, 2014/12/06 13:16:33]

This looks awkward to me...

[SeRya, 2014/12/07 13:40:46]

This is well-known pattern. Standard clear method remains memory reserverd by
the vector.

+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::OnDeviceListReady(
+    const CloudDeviceListDelegate::DeviceList& devices) {
+  browser_list_updating_ = false;
+
+  // TODO(serya): Not all devices are browsers. Filter them out.

[dgozman, 2014/12/06 13:16:33]

Let's implement this right away. We should also group browsers from the same
device.

[SeRya, 2014/12/07 13:40:46]

There is not enough information provided by CloudDeviceList right away.

[dgozman, 2014/12/08 14:27:22]

We should check for the command we use to be present on device. Let's follow up
with a patch immediately.

+  BrowserInfoList browsers;
+  browsers.reserve(devices.size());
+  for (const auto& device : devices) {
+    BrowserInfo browser;
+    browser.type = BrowserInfo::kTypeChrome;
+    browser.display_name = device.display_name;
+    browsers.push_back(browser);
+  }
+
+  browsers_.swap(browsers);
+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::OnDeviceListUnavailable() {
+  browser_list_updating_ = false;
+
+  BrowserInfoList().swap(browsers_);
 }
 
 // WebRTCDeviceProvider::MessageHandler ----------------------------------------
@@ -217,16 +308,20 @@ WebRTCDeviceProvider::~WebRTCDeviceProvider() {
 }
 
 void WebRTCDeviceProvider::QueryDevices(const SerialsCallback& callback) {
-  // TODO(serya): Implement
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE, base::Bind(callback, std::vector<std::string>()));
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&DevToolsBridgeClient::GetDevices, client_),
+      callback);
 }
 
 void WebRTCDeviceProvider::QueryDeviceInfo(const std::string& serial,
                                           const DeviceInfoCallback& callback) {

[dgozman, 2014/12/06 13:16:33]

nit: indentation

[SeRya, 2014/12/08 15:01:22]

Done.

-  // TODO(serya): Implement
-  base::MessageLoop::current()->PostTask(
-      FROM_HERE, base::Bind(callback, AndroidDeviceManager::DeviceInfo()));
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&DevToolsBridgeClient::GetDeviceInfo, client_, serial),
+      callback);
 }
 
 void WebRTCDeviceProvider::OpenSocket(const std::string& serial,