Enabling WebRTCDeviceProvider under enable-devtools-experiments flag

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 fa69ad8abf922d8649ffa3758d5e58d3c8b3aa76..b8491944a9984839576946552e871e09333814bf 100644
--- a/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
+++ b/chrome/browser/devtools/device/webrtc/webrtc_device_provider.cc
@@ -4,59 +4,92 @@
 
 #include "chrome/browser/devtools/device/webrtc/webrtc_device_provider.h"
 
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/signin/profile_identity_provider.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
+#include "google_apis/gaia/identity_provider.h"
 #include "grit/webrtc_device_provider_resources_map.h"
+#include "net/base/net_errors.h"
+#include "net/socket/stream_socket.h"
 #include "ui/base/page_transition_types.h"
 
+using content::BrowserThread;
+using content::NotificationDetails;
+using content::NotificationObserver;
+using content::NotificationRegistrar;
+using content::NotificationSource;
+using content::Source;
+using content::WebContents;
+using content::WebUIDataSource;
+using content::WebUIMessageHandler;

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

Using so much content classes may be confusing.

[SeRya, 2014/12/08 07:44:32]

So what you suggest? "using namespace content;"?

+
+namespace {
+
 const char kBackgroundWorkerURL[] =
     "chrome://webrtc-device-provider/background_worker.html";
 
-namespace {
+}  // namespace
 
-class MessageHandler : public content::WebUIMessageHandler {
+// Lives on the UI thread.
+class WebRTCDeviceProvider::DevToolsBridgeClient :
+    private NotificationObserver,

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

Why use private inheritance?

[SeRya, 2014/12/08 07:44:32]

Like with private members: if there is no need to be public it should be
private.

+    private IdentityProvider::Observer {
  public:
-  explicit MessageHandler(WebRTCDeviceProvider* owner);
+  static base::WeakPtr<DevToolsBridgeClient> Create(
+      Profile* profile, ProfileIdentityProvider* identity_provider);
 
-  void RegisterMessages() override;
+  void DeleteSelf();
 
  private:
-  void HandleLoaded(const base::ListValue* args);
+  DevToolsBridgeClient(Profile* profile,
+                       ProfileIdentityProvider* identity_provider);
 
-  WebRTCDeviceProvider* const owner_;
-};
+  ~DevToolsBridgeClient();
 
-// MessageHandler -------------------------------------------------------------
+  void CreateBackgroundWorker();
 
-MessageHandler::MessageHandler(
-    WebRTCDeviceProvider* owner) : owner_(owner) {
-}
+  // implementation of IdentityProvider::Observer.

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

nit (here and below): Implementation

[SeRya, 2014/12/08 07:44:32]

Done.

+  void OnActiveAccountLogin() override;
+  void OnActiveAccountLogout() override;
 
-void MessageHandler::RegisterMessages() {
-  web_ui()->RegisterMessageCallback(
-      "loaded",
-      base::Bind(&MessageHandler::HandleLoaded, base::Unretained(this)));
-}
+  // implementation of NotificationObserver.
+  void Observe(int type,
+               const NotificationSource& source,
+               const NotificationDetails& details) override;
 
-void MessageHandler::HandleLoaded(
-    const base::ListValue* args) {
-  if (!owner_)
-    return;
-  // TODO(serya): implement
-}
+  Profile* const profile_;
+  ProfileIdentityProvider* const identity_provider_;
+  NotificationRegistrar registrar_;
+  scoped_ptr<WebContents> background_worker_;
+  base::WeakPtrFactory<DevToolsBridgeClient> weak_factory_;
+};
 
-}  // namespace
+class WebRTCDeviceProvider::MessageHandler : public WebUIMessageHandler {
+ public:
+  explicit MessageHandler(DevToolsBridgeClient* owner);
+
+  void RegisterMessages() override;
+
+ private:
+  void HandleLoaded(const base::ListValue* args);
+
+  DevToolsBridgeClient* const owner_;
+};
 
-// WebRTCDeviceProvider::WebUI ------------------------------------------------
+// WebRTCDeviceProvider::WebUI -------------------------------------------------
 
 WebRTCDeviceProvider::WebUI::WebUI(content::WebUI* web_ui)
     : content::WebUIController(web_ui) {
   Profile* profile = Profile::FromWebUI(web_ui);
 
-  content::WebUIDataSource* source = content::WebUIDataSource::Create(
+  WebUIDataSource* source = WebUIDataSource::Create(
       chrome::kChromeUIWebRTCDeviceProviderHost);
 
   for (size_t i = 0; i < kWebrtcDeviceProviderResourcesSize; i++) {
@@ -69,21 +102,55 @@ WebRTCDeviceProvider::WebUI::WebUI(content::WebUI* web_ui)
   // a real implementation.
   web_ui->AddMessageHandler(new MessageHandler(nullptr));
 
-  content::WebUIDataSource::Add(profile, source);
+  WebUIDataSource::Add(profile, source);
 }
 
 WebRTCDeviceProvider::WebUI::~WebUI() {
 }
 
-// WebRTCDeviceProvider -------------------------------------------------------
+// WebRTCDeviceProvider::DevToolsBridgeClient ----------------------------------
+
+// static
+base::WeakPtr<WebRTCDeviceProvider::DevToolsBridgeClient>
+WebRTCDeviceProvider::DevToolsBridgeClient::Create(

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

This method seems to be called from handler thread. How does it create client
which lives on UI?

[SeRya, 2014/12/08 07:44:32]

All providers are created on the UI thread from DevToolsAndroidBridge
constructor.

+    Profile* profile, ProfileIdentityProvider* identity_provider) {
+  auto instance = new DevToolsBridgeClient(profile, identity_provider);
+  return instance->weak_factory_.GetWeakPtr();
+}
+
+WebRTCDeviceProvider::DevToolsBridgeClient::DevToolsBridgeClient(
+    Profile* profile, ProfileIdentityProvider* identity_provider)
+  : profile_(profile),

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

nit: indentation

[SeRya, 2014/12/08 07:44:32]

Done.

+    identity_provider_(identity_provider),
+    weak_factory_(this) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-WebRTCDeviceProvider::WebRTCDeviceProvider(content::BrowserContext* context) {
-  background_worker_.reset(content::WebContents::Create(
-      content::WebContents::CreateParams(context)));
+  identity_provider_->AddObserver(this);
+  registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
+                 Source<Profile>(profile_));
 
-  // TODO(serya): Make sure background_worker_ destructed before profile.
-  GURL url(kBackgroundWorkerURL);
+  if (!identity_provider_->GetActiveAccountId().empty())
+    CreateBackgroundWorker();
+}
 
+WebRTCDeviceProvider::DevToolsBridgeClient::~DevToolsBridgeClient() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  identity_provider_->RemoveObserver(this);

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

Is this provider still alive when NOTIFICATION_PROFILE_DESTROYED is fired?

[SeRya, 2014/12/08 07:44:32]

It is. And it's why NOTIFICATION_PROFILE_DESTROYED needed even if would
destroyed synchronously on the UI thread.

+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::DeleteSelf() {

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

Can we have 2 tests, which examine "DeleteSelf, then ProfileDestoryed" and
"ProfileDestroyed, then DeleteSelf"?

[SeRya, 2014/12/08 07:44:32]

Done.

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  delete this;
+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::CreateBackgroundWorker() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  background_worker_.reset(
+      WebContents::Create(WebContents::CreateParams(profile_)));
+
+  GURL url(kBackgroundWorkerURL);
   DCHECK_EQ(chrome::kChromeUIWebRTCDeviceProviderHost, url.host());
 
   background_worker_->GetController().LoadURL(
@@ -96,20 +163,77 @@ WebRTCDeviceProvider::WebRTCDeviceProvider(content::BrowserContext* context) {
       new MessageHandler(this));
 }
 
+void WebRTCDeviceProvider::DevToolsBridgeClient::Observe(
+    int type,
+    const NotificationSource& source,
+    const NotificationDetails& details) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(chrome::NOTIFICATION_PROFILE_DESTROYED, type);
+
+  delete this;
+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::OnActiveAccountLogin() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CreateBackgroundWorker();
+}
+
+void WebRTCDeviceProvider::DevToolsBridgeClient::OnActiveAccountLogout() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  background_worker_.reset();
+}
+
+// WebRTCDeviceProvider::MessageHandler ----------------------------------------
+
+WebRTCDeviceProvider::MessageHandler::MessageHandler(

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

Why has this changed to an inner class?

[SeRya, 2014/12/08 07:44:32]

To have access to the private member DevToolsBridgeClient

+    DevToolsBridgeClient* owner) : owner_(owner) {
+}
+
+void WebRTCDeviceProvider::MessageHandler::RegisterMessages() {
+  web_ui()->RegisterMessageCallback(
+      "loaded",
+      base::Bind(&MessageHandler::HandleLoaded, base::Unretained(this)));
+}
+
+void WebRTCDeviceProvider::MessageHandler::HandleLoaded(
+    const base::ListValue* args) {
+  if (!owner_)
+    return;
+  // TODO(serya): implement
+}
+
+// WebRTCDeviceProvider --------------------------------------------------------
+
+WebRTCDeviceProvider::WebRTCDeviceProvider(
+    Profile* profile, ProfileIdentityProvider* identity_provider)
+    : client_(DevToolsBridgeClient::Create(profile, identity_provider)) {
+}
+
 WebRTCDeviceProvider::~WebRTCDeviceProvider() {
+  BrowserThread::PostTask(
+      BrowserThread::UI,
+      FROM_HERE,
+      base::Bind(&DevToolsBridgeClient::DeleteSelf, client_));
 }
 
 void WebRTCDeviceProvider::QueryDevices(const SerialsCallback& callback) {
   // TODO(serya): Implement
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(callback, std::vector<std::string>()));
 }
 
 void WebRTCDeviceProvider::QueryDeviceInfo(const std::string& serial,
                                           const DeviceInfoCallback& callback) {
   // TODO(serya): Implement
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(callback, AndroidDeviceManager::DeviceInfo()));
 }
 
 void WebRTCDeviceProvider::OpenSocket(const std::string& serial,
                                      const std::string& socket_name,
                                      const SocketCallback& callback) {
   // TODO(serya): Implement
+  scoped_ptr<net::StreamSocket> socket;
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE, base::Bind(callback, net::ERR_FAILED, base::Passed(&socket)));
 }