websocket-to-TCP proxy: observe value of listening port in right place.

Also minor change for new API: increase timeout for waiting WEB_SOCKET_PROXY_STARTED notification.  New API (GetURLForTCP as opposed to GetPassportForTCP) runs on ephemeral port so if port is not known then we cannot construct URL at all.  3 seconds may be too little to launch proxy on slow overloaded machine.

BUG=chromium-os:21942
TEST=Manual+Apitest

Review URL: http://codereview.chromium.org/8362027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107125 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 46 insertions, 12 deletions

diff --git a/chrome/browser/chromeos/web_socket_proxy_controller.cc b/chrome/browser/chromeos/web_socket_proxy_controller.cc
index 5436afa..ad1fc42 100644
--- a/chrome/browser/chromeos/web_socket_proxy_controller.cc
+++ b/chrome/browser/chromeos/web_socket_proxy_controller.cc
@@ -24,6 +24,7 @@
 #include "content/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/common/url_constants.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/network_change_notifier.h"
@@ -99,18 +100,35 @@ class ProxyTask : public Task {
   virtual void Run() OVERRIDE;
 };
 
-class ProxyLifetime : public net::NetworkChangeNotifier::OnlineStateObserver {
+class ProxyLifetime
+    : public net::NetworkChangeNotifier::OnlineStateObserver,
+      public content::NotificationObserver {
  public:
-  ProxyLifetime() : delay_ms_(1000), shutdown_requested_(false) {
+  ProxyLifetime() : delay_ms_(1000), port_(-1), shutdown_requested_(false) {
+    DLOG(INFO) << "WebSocketProxyController initiation";
     BrowserThread::PostTask(
         BrowserThread::WEB_SOCKET_PROXY, FROM_HERE, new ProxyTask());
     net::NetworkChangeNotifier::AddOnlineStateObserver(this);
+    registrar_.Add(
+        this, chrome::NOTIFICATION_WEB_SOCKET_PROXY_STARTED,
+        content::NotificationService::AllSources());
   }
 
   virtual ~ProxyLifetime() {
     net::NetworkChangeNotifier::RemoveOnlineStateObserver(this);
   }
 
+  virtual void Observe(int type, const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE {
+    base::AutoLock alk(lock_);
+    port_ = *content::Details<int>(details).ptr();
+  }
+
+  int GetPort() {
+    base::AutoLock alk(lock_);
+    return port_;
+  }
+
  private:
   // net::NetworkChangeNotifier::OnlineStateObserver overrides.
   virtual void OnOnlineStateChanged(bool online) OVERRIDE {
@@ -121,7 +139,10 @@ class ProxyLifetime : public net::NetworkChangeNotifier::OnlineStateObserver {
   }
 
   // Delay between next attempt to run proxy.
-  int delay_ms_;
+  int volatile delay_ms_;
+
+  // Proxy listens for incoming websocket connections on this port.
+  int volatile port_;
 
   chromeos::WebSocketProxy* volatile server_;
   volatile bool shutdown_requested_;
@@ -173,7 +194,6 @@ void FillWithExtensionsIdsWithPrivateAccess(std::vector<std::string>* ids) {
 
 // static
 void WebSocketProxyController::Initiate() {
-  LOG(INFO) << "WebSocketProxyController initiation";
   g_proxy_lifetime.Get();
 }
 
@@ -183,9 +203,16 @@ bool WebSocketProxyController::IsInitiated() {
 }
 
 // static
+int WebSocketProxyController::GetPort() {
+  int port = g_proxy_lifetime.Get().GetPort();
+  DCHECK(IsInitiated());
+  return port;
+}
+
+// static
 void WebSocketProxyController::Shutdown() {
   if (IsInitiated()) {
-    LOG(INFO) << "WebSocketProxyController shutdown";
+    DLOG(INFO) << "WebSocketProxyController shutdown";
     base::AutoLock alk(g_proxy_lifetime.Get().lock_);
     g_proxy_lifetime.Get().shutdown_requested_ = true;
     if (g_proxy_lifetime.Get().server_)
diff --git a/chrome/browser/chromeos/web_socket_proxy_controller.h b/chrome/browser/chromeos/web_socket_proxy_controller.h
index bcee38e..4e8ad0d 100644
--- a/chrome/browser/chromeos/web_socket_proxy_controller.h
+++ b/chrome/browser/chromeos/web_socket_proxy_controller.h
@@ -29,6 +29,7 @@ class WebSocketProxyController {
   // All methods can be called on any thread.
   static void Shutdown();
   static bool IsInitiated();
+  static int GetPort();  // Returns port listening websocket connections.
 
   static bool CheckCredentials(
       const std::string& extension_id,
diff --git a/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc b/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
index bda21c73..31fd122 100644
--- a/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
+++ b/chrome/browser/extensions/extension_web_socket_proxy_private_api.cc
@@ -41,7 +41,7 @@ void WebSocketProxyPrivate::Finalize() {
   if (is_finalized_)
     return;
   is_finalized_ = true;
-  SendResponse(true);
+  SendResponse(listening_port_ > 0);
   Release();
 }
 
@@ -59,10 +59,14 @@ void WebSocketProxyPrivateGetURLForTCPFunction::Observe(
 }
 
 void WebSocketProxyPrivateGetURLForTCPFunction::Finalize() {
+#if defined(OS_CHROMEOS)
   StringValue* url = Value::CreateStringValue(std::string(
       "ws://127.0.0.1:" + base::IntToString(listening_port_) +
       "/tcpproxy?" + query_));
   result_.reset(url);
+  if (listening_port_ < 1)
+    listening_port_ = chromeos::WebSocketProxyController::GetPort();
+#endif
   WebSocketProxyPrivate::Finalize();
 }
 
@@ -80,12 +84,12 @@ bool WebSocketProxyPrivateGetPassportForTCPFunction::RunImpl() {
   if (chromeos::WebSocketProxyController::CheckCredentials(
           extension_id(), hostname, port,
           chromeos::WebSocketProxyController::PLAIN_TCP)) {
-    if (!chromeos::WebSocketProxyController::IsInitiated()) {
+    listening_port_ = chromeos::WebSocketProxyController::GetPort();
+    if (listening_port_ < 1) {
       delay_response = true;
       registrar_.Add(
           this, chrome::NOTIFICATION_WEB_SOCKET_PROXY_STARTED,
           content::NotificationService::AllSources());
-      chromeos::WebSocketProxyController::Initiate();
     }
 
     std::map<std::string, std::string> map;
@@ -132,12 +136,12 @@ bool WebSocketProxyPrivateGetURLForTCPFunction::RunImpl() {
           extension_id(), hostname, port,
           do_tls ? chromeos::WebSocketProxyController::TLS_OVER_TCP :
               chromeos::WebSocketProxyController::PLAIN_TCP)) {
-    if (!chromeos::WebSocketProxyController::IsInitiated()) {
+    listening_port_ = chromeos::WebSocketProxyController::GetPort();
+    if (listening_port_ < 1) {
       delay_response = true;
       registrar_.Add(
           this, chrome::NOTIFICATION_WEB_SOCKET_PROXY_STARTED,
           content::NotificationService::AllSources());
-      chromeos::WebSocketProxyController::Initiate();
     }
 
     std::map<std::string, std::string> map;
@@ -155,7 +159,7 @@ bool WebSocketProxyPrivateGetURLForTCPFunction::RunImpl() {
 #endif  // defined(OS_CHROMEOS)
 
   if (delay_response) {
-    const int kTimeout = 3;
+    const int kTimeout = 12;
     timer_.Start(FROM_HERE, base::TimeDelta::FromSeconds(kTimeout),
         this, &WebSocketProxyPrivate::Finalize);
   } else {
diff --git a/chrome/browser/extensions/extension_web_socket_proxy_private_api.h b/chrome/browser/extensions/extension_web_socket_proxy_private_api.h
index 8f888cf..eb2af4a 100644
--- a/chrome/browser/extensions/extension_web_socket_proxy_private_api.h
+++ b/chrome/browser/extensions/extension_web_socket_proxy_private_api.h
@@ -35,7 +35,7 @@ class WebSocketProxyPrivate
 
   content::NotificationRegistrar registrar_;
 
-  // Proxy listens incoming websocket connection on this port.
+  // Proxy accepts websocket connections on this port.
   int listening_port_;
 };
 
diff --git a/chrome/browser/extensions/extension_web_socket_proxy_private_apitest.cc b/chrome/browser/extensions/extension_web_socket_proxy_private_apitest.cc
index 8d6d12d..060583e 100644
--- a/chrome/browser/extensions/extension_web_socket_proxy_private_apitest.cc
+++ b/chrome/browser/extensions/extension_web_socket_proxy_private_apitest.cc
@@ -18,6 +18,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebSocketProxyPrivateApiTest, Pass) {
 #if defined(OS_CHROMEOS)
   ASSERT_TRUE(StartTestServer());
   ASSERT_TRUE(RunExtensionTest("web_socket_proxy_private")) << message_;
+  // Check if API still works on subsequent calls.
+  ASSERT_TRUE(RunExtensionTest("web_socket_proxy_private")) << message_;
 #endif
 }