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

chrome/test/remoting/remote_desktop_browsertest.cc

Index: chrome/test/remoting/remote_desktop_browsertest.cc
diff --git a/chrome/test/remoting/remote_desktop_browsertest.cc b/chrome/test/remoting/remote_desktop_browsertest.cc
index c8b006acd011651de27ca2b598ee365815d29d58..ae48cd241e3dbc13f9801753715ba88cf1a9969f 100644
--- a/chrome/test/remoting/remote_desktop_browsertest.cc
+++ b/chrome/test/remoting/remote_desktop_browsertest.cc
@@ -585,10 +585,10 @@ void RemoteDesktopBrowserTest::ConnectToRemoteHost(
   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);
 
@@ -816,6 +816,21 @@ void RemoteDesktopBrowserTest::WaitForConnection() {
   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_?