Adding changes to Remote_Desktop_BrowserTest ConnecToRemoteHost to verify if the remote host is onl…

chrome/test/remoting/remote_desktop_browsertest.cc

 // Copyright 2013 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/test/remoting/remote_desktop_browsertest.h"
 
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/macros.h"
@@ skipping 567 matching lines @@
         base::TimeDelta::FromMilliseconds(500),
         base::Bind(&RemoteDesktopBrowserTest::IsHostListReady, this));
   EXPECT_TRUE(waiter.Wait());
 
   std::string host_id = ExecuteScriptAndExtractString(
       "remoting.hostList.getHostIdForName('" + host_name + "')");
 
   EXPECT_FALSE(host_id.empty());
   std::string element_id = "host_" + host_id;
 
-  // Verify the host is online.
-  std::string host_div_class = ExecuteScriptAndExtractString(
-      "document.getElementById('" + element_id + "').parentNode.className");
-  EXPECT_NE(std::string::npos, host_div_class.find("host-online"));
+  ConditionalTimeoutWaiter hostOnlineWaiter(
+      base::TimeDelta::FromSeconds(30), base::TimeDelta::FromMilliseconds(5000),

[joedow, 2016/01/06 16:33:52]

Why base::TimeDelta::FromMilliseconds instead of using FromSeconds?  If you want
a larger timeout, I think it would be simpler to use FromSeconds(5) here.

[alog1, 2016/01/06 21:49:09]

Done.

+      base::Bind(&RemoteDesktopBrowserTest::IsHostOnline, this, host_id));

[joedow, 2016/01/06 16:33:52]

You should declare the ownership of the this pointer you are passing in, I'm
assuming you want base::Unretained(this).

[alog1, 2016/01/06 21:49:09]

Done.

+  EXPECT_TRUE(hostOnlineWaiter.Wait());
 
   ClickOnControl(element_id);
 
   // Enter the pin # passed in from the command line.
   EnterPin(me2me_pin(), remember_pin);
 
   WaitForConnection();
 }
 
 void RemoteDesktopBrowserTest::EnableDNSLookupForThisTest(
@@ skipping 207 matching lines @@
       base::TimeDelta::FromSeconds(1),
       base::Bind(&RemoteDesktopBrowserTest::IsSessionConnected, this));
   EXPECT_TRUE(waiter.Wait());
 
   // The client is not yet ready to take input when the session state becomes
   // CONNECTED. Wait for 2 seconds for the client to become ready.
   // TODO(weitaosu): Find a way to detect when the client is truly ready.
   TimeoutWaiter(base::TimeDelta::FromSeconds(2)).Wait();
 }
 
+bool RemoteDesktopBrowserTest::IsHostOnline(std::string host_id) {

[joedow, 2016/01/06 16:33:52]

host_id should be passed as a const ref.

[alog1, 2016/01/06 21:49:09]

Done.

+  bool refresh_host_list =
+      ExecuteScriptAndExtractBool("remoting.hostList.refreshAndDisplay()");
+
+  if (!refresh_host_list) {
+    return false;
+  }

[joedow, 2016/01/06 16:33:52]

Add a newline here to make this more readable.

Also, should this be moved into its own helper function?

[alog1, 2016/01/06 21:49:09]

Done.

+  std::string element_id = "host_" + host_id;
+  // Verify the host is online.

[joedow, 2016/01/06 16:33:52]

I think comment should be moved up one line so the related code lines are
bunched together.

[alog1, 2016/01/06 21:49:09]

Done.

+  std::string host_div_class = ExecuteScriptAndExtractString(
+      "document.getElementById('" + element_id + "').parentNode.className");
+
+  return (std::string::npos != host_div_class.find("host-online"));
+}
+
 bool RemoteDesktopBrowserTest::IsLocalHostReady() {
   // TODO(weitaosu): Instead of polling, can we register a callback to
   // remoting.hostList.setLocalHost_?
   return ExecuteScriptAndExtractBool(
       "remoting.hostList.localHostSection_.host_ != null");
 }
 
 bool RemoteDesktopBrowserTest::IsHostListReady() {
   // Wait until hostList is not null.
   // The connect-to-host tests are run on the waterfall using a new profile-dir.
@@ skipping 93 matching lines @@
 
   ConditionalTimeoutWaiter homeWaiter(
           base::TimeDelta::FromSeconds(5),
           base::TimeDelta::FromMilliseconds(500),
           base::Bind(&RemoteDesktopBrowserTest::IsAppModeEqualTo,
                      this, "remoting.AppMode.HOME"));
   EXPECT_TRUE(homeWaiter.Wait());
 }
 
 }  // namespace remoting