Re-landing https://codereview.chromium.org/18184004/.

This new attempt will not stop the test dead in its tracks if a sleep fails, just log a warning. I'll try this and keep an eye on the bot logs to see if it happens a lot. It's the best I can do for now until I figure out why the sleep fails every now and then.

Original description:
Fixed spinlocking poll method in WebRTC browser tests, fixed duplication.

BUG=225698
R=tommi@chromium.org

Review URL: https://codereview.chromium.org/19675011

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

6 files changed, 149 insertions, 105 deletions

diff --git a/chrome/browser/media/chrome_media_stream_infobar_browsertest.cc b/chrome/browser/media/chrome_media_stream_infobar_browsertest.cc
index 5396b11..a0aada7 100644
--- a/chrome/browser/media/chrome_media_stream_infobar_browsertest.cc
+++ b/chrome/browser/media/chrome_media_stream_infobar_browsertest.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/infobars/infobar.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/media/media_stream_infobar_delegate.h"
+#include "chrome/browser/media/webrtc_browsertest_common.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -115,7 +116,7 @@ class MediaStreamInfobarTest : public InProcessBrowserTest {
     CloseInfobarInTab(tab_contents, media_infobar);
 
     // Wait for WebRTC to call the success callback.
-    EXPECT_TRUE(UglyPollingWaitUntil(
+    EXPECT_TRUE(PollingWaitUntil(
         "obtainGetUserMediaResult()", kOkGotStream, tab_contents));
   }
 
@@ -134,9 +135,9 @@ class MediaStreamInfobarTest : public InProcessBrowserTest {
     CloseInfobarInTab(tab_contents, media_infobar);
 
     // Wait for WebRTC to call the fail callback.
-    EXPECT_TRUE(UglyPollingWaitUntil("obtainGetUserMediaResult()",
-                                     kFailedWithErrorPermissionDenied,
-                                     tab_contents));
+    EXPECT_TRUE(PollingWaitUntil("obtainGetUserMediaResult()",
+                                 kFailedWithErrorPermissionDenied,
+                                 tab_contents));
   }
 
   void TestDismissOnInfobar(content::WebContents* tab_contents) {
@@ -148,35 +149,9 @@ class MediaStreamInfobarTest : public InProcessBrowserTest {
     CloseInfobarInTab(tab_contents, media_infobar);
 
     // A dismiss should be treated like a deny.
-    EXPECT_TRUE(UglyPollingWaitUntil("obtainGetUserMediaResult()",
-                                     kFailedWithErrorPermissionDenied,
-                                     tab_contents));
-  }
-
-  // TODO(phoglund): de-dupe
-  // TODO(phoglund): This ugly poll method is only here while we transition
-  // the test javascript to just post events when things happen. Right now they
-  // don't because the webrtc_call.py and other tests use this polling way of
-  // communicating when we are waiting from an asynchronous event in the
-  // javascript. This method is meant to emulate WaitUntil in the PyAuto
-  // framework.
-  bool UglyPollingWaitUntil(const std::string& javascript,
-                            const std::string& evaluates_to,
-                            content::WebContents* tab_contents) {
-    const base::Time start_time = base::Time::Now();
-    const base::TimeDelta timeout = TestTimeouts::action_max_timeout();
-    std::string result;
-
-    while (base::Time::Now() - start_time < timeout) {
-      result = ExecuteJavascript(javascript, tab_contents);
-      LOG(INFO) << result;
-      if (evaluates_to == result)
-        return true;
-    }
-    LOG(ERROR) << "Timed out while waiting for " << javascript
-               << " to evaluate to " << evaluates_to << "; last result was '"
-               << result << "'";
-    return false;
+    EXPECT_TRUE(PollingWaitUntil("obtainGetUserMediaResult()",
+                                 kFailedWithErrorPermissionDenied,
+                                 tab_contents));
   }
 
   void GetUserMedia(const std::string& constraints,
@@ -219,9 +194,9 @@ IN_PROC_BROWSER_TEST_F(MediaStreamInfobarTest,
 
   // Should fail with permission denied right away with no infobar popping up.
   GetUserMedia(kAudioVideoCallConstraints, tab_contents);
-  EXPECT_TRUE(UglyPollingWaitUntil("obtainGetUserMediaResult()",
-                                   kFailedWithErrorPermissionDenied,
-                                   tab_contents));
+  EXPECT_TRUE(PollingWaitUntil("obtainGetUserMediaResult()",
+                               kFailedWithErrorPermissionDenied,
+                               tab_contents));
   InfoBarService* infobar_service =
       InfoBarService::FromWebContents(tab_contents);
   EXPECT_EQ(0u, infobar_service->infobar_count());
diff --git a/chrome/browser/media/chrome_webrtc_browsertest.cc b/chrome/browser/media/chrome_webrtc_browsertest.cc
index 2b6eb8e..b9c6b51 100644
--- a/chrome/browser/media/chrome_webrtc_browsertest.cc
+++ b/chrome/browser/media/chrome_webrtc_browsertest.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/infobars/infobar.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/media/media_stream_infobar_delegate.h"
+#include "chrome/browser/media/webrtc_browsertest_common.h"
 #include "chrome/browser/media/webrtc_log_uploader.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
@@ -98,30 +99,6 @@ class WebrtcBrowserTest : public InProcessBrowserTest {
         switches::kUseFakeUIForMediaStream));
   }
 
-  // TODO(phoglund): This ugly poll method is only here while we transition
-  // the test javascript to just post events when things happen. Right now they
-  // don't because the webrtc_call.py and other tests use this polling way of
-  // communicating when we are waiting from an asynchronous event in the
-  // javascript. This method is meant to emulate WaitUntil in the PyAuto
-  // framework.
-  bool UglyPollingWaitUntil(const std::string& javascript,
-                            const std::string& evaluates_to,
-                            content::WebContents* tab_contents) {
-    base::Time start_time = base::Time::Now();
-    base::TimeDelta timeout = base::TimeDelta::FromSeconds(20);
-    std::string result;
-
-    while (base::Time::Now() - start_time < timeout) {
-      result = ExecuteJavascript(javascript, tab_contents);
-      if (evaluates_to == result)
-        return true;
-    }
-    LOG(ERROR) << "Timed out while waiting for " << javascript <<
-        " to evaluate to " << evaluates_to << "; last result was '" << result <<
-        "'";
-    return false;
-  }
-
   // Convenience method which executes the provided javascript in the context
   // of the provided web contents and returns what it evaluated to.
   std::string ExecuteJavascript(const std::string& javascript,
@@ -149,9 +126,9 @@ class WebrtcBrowserTest : public InProcessBrowserTest {
     media_infobar->Accept();
 
     // Wait for WebRTC to call the success callback.
-    EXPECT_TRUE(UglyPollingWaitUntil("obtainGetUserMediaResult();",
-                                     "ok-got-stream",
-                                     tab_contents));
+    EXPECT_TRUE(PollingWaitUntil("obtainGetUserMediaResult();",
+                                 "ok-got-stream",
+                                 tab_contents));
   }
 
   // Ensures we didn't get any errors asynchronously (e.g. while no javascript
@@ -187,10 +164,10 @@ class WebrtcBrowserTest : public InProcessBrowserTest {
               ExecuteJavascript("negotiateCall()", from_tab));
 
     // Ensure the call gets up on both sides.
-    EXPECT_TRUE(UglyPollingWaitUntil("getPeerConnectionReadyState()",
-                                     "active", from_tab));
-    EXPECT_TRUE(UglyPollingWaitUntil("getPeerConnectionReadyState()",
-                                     "active", to_tab));
+    EXPECT_TRUE(PollingWaitUntil("getPeerConnectionReadyState()",
+                                 "active", from_tab));
+    EXPECT_TRUE(PollingWaitUntil("getPeerConnectionReadyState()",
+                                 "active", to_tab));
   }
 
   void StartDetectingVideo(content::WebContents* tab_contents,
@@ -202,13 +179,13 @@ class WebrtcBrowserTest : public InProcessBrowserTest {
   }
 
   void WaitForVideoToPlay(content::WebContents* tab_contents) {
-    EXPECT_TRUE(UglyPollingWaitUntil("isVideoPlaying()", "video-playing",
-                                     tab_contents));
+    EXPECT_TRUE(PollingWaitUntil("isVideoPlaying()", "video-playing",
+                                 tab_contents));
   }
 
   void WaitForVideoToStopPlaying(content::WebContents* tab_contents) {
-    EXPECT_TRUE(UglyPollingWaitUntil("isVideoPlaying()", "video-not-playing",
-                                     tab_contents));
+    EXPECT_TRUE(PollingWaitUntil("isVideoPlaying()", "video-not-playing",
+                                 tab_contents));
   }
 
   void HangUp(content::WebContents* from_tab) {
@@ -216,8 +193,8 @@ class WebrtcBrowserTest : public InProcessBrowserTest {
   }
 
   void WaitUntilHangupVerified(content::WebContents* tab_contents) {
-    EXPECT_TRUE(UglyPollingWaitUntil("getPeerConnectionReadyState()",
-                                     "no-peer-connection", tab_contents));
+    EXPECT_TRUE(PollingWaitUntil("getPeerConnectionReadyState()",
+                                 "no-peer-connection", tab_contents));
   }
 
   std::string ToggleLocalVideoTrack(content::WebContents* tab_contents) {
diff --git a/chrome/browser/media/chrome_webrtc_video_quality_browsertest.cc b/chrome/browser/media/chrome_webrtc_video_quality_browsertest.cc
index 8e5b3e4..f6c5a97 100644
--- a/chrome/browser/media/chrome_webrtc_video_quality_browsertest.cc
+++ b/chrome/browser/media/chrome_webrtc_video_quality_browsertest.cc
@@ -14,6 +14,7 @@
 #include "chrome/browser/infobars/infobar.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/media/media_stream_infobar_delegate.h"
+#include "chrome/browser/media/webrtc_browsertest_common.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -169,30 +170,6 @@ class WebrtcVideoQualityBrowserTest : public InProcessBrowserTest {
         << "Failed to shut down pywebsocket server!";
   }
 
-  // TODO(phoglund): This ugly poll method is only here while we transition
-  // the test javascript to just post events when things happen. Right now they
-  // don't because the webrtc_call.py and other tests use this polling way of
-  // communicating when we are waiting from an asynchronous event in the
-  // javascript. This method is meant to emulate WaitUntil in the PyAuto
-  // framework.
-  bool UglyPollingWaitUntil(const std::string& javascript,
-                            const std::string& evaluates_to,
-                            content::WebContents* tab_contents) {
-    base::Time start_time = base::Time::Now();
-    base::TimeDelta timeout = TestTimeouts::action_max_timeout();
-    std::string result;
-
-    while (base::Time::Now() - start_time < timeout) {
-      result = ExecuteJavascript(javascript, tab_contents);
-      if (evaluates_to == result)
-        return true;
-    }
-    LOG(ERROR) << "Timed out while waiting for " << javascript
-               << " to evaluate to " << evaluates_to << "; last result was '"
-               << result << "'";
-    return false;
-  }
-
   // Convenience method which executes the provided javascript in the context
   // of the provided web contents and returns what it evaluated to.
   std::string ExecuteJavascript(const std::string& javascript,
@@ -221,7 +198,7 @@ class WebrtcVideoQualityBrowserTest : public InProcessBrowserTest {
     media_infobar->Accept();
 
     // Wait for WebRTC to call the success callback.
-    EXPECT_TRUE(UglyPollingWaitUntil(
+    EXPECT_TRUE(PollingWaitUntil(
         "obtainGetUserMediaResult();", "ok-got-stream", tab_contents));
   }
 
@@ -251,9 +228,9 @@ class WebrtcVideoQualityBrowserTest : public InProcessBrowserTest {
     EXPECT_EQ("ok-negotiating", ExecuteJavascript("negotiateCall()", from_tab));
 
     // Ensure the call gets up on both sides.
-    EXPECT_TRUE(UglyPollingWaitUntil(
+    EXPECT_TRUE(PollingWaitUntil(
         "getPeerConnectionReadyState()", "active", from_tab));
-    EXPECT_TRUE(UglyPollingWaitUntil(
+    EXPECT_TRUE(PollingWaitUntil(
         "getPeerConnectionReadyState()", "active", to_tab));
   }
 
@@ -262,7 +239,7 @@ class WebrtcVideoQualityBrowserTest : public InProcessBrowserTest {
   }
 
   void WaitUntilHangupVerified(content::WebContents* tab_contents) {
-    EXPECT_TRUE(UglyPollingWaitUntil(
+    EXPECT_TRUE(PollingWaitUntil(
         "getPeerConnectionReadyState()", "no-peer-connection", tab_contents));
   }
 
@@ -498,9 +475,14 @@ IN_PROC_BROWSER_TEST_F(WebrtcVideoQualityBrowserTest,
   AssertNoAsynchronousErrors(left_tab);
   AssertNoAsynchronousErrors(right_tab);
 
+  // Poll slower here to avoid flooding the log with messages: capturing and
+  // sending frames take quite a bit of time.
+  int polling_interval_msec = 1000;
+
   // TODO(phoglund): (de-dupe later) different from original flow.
-  EXPECT_TRUE(UglyPollingWaitUntil(
-      "doneFrameCapturing()", "done-capturing", right_tab));
+  EXPECT_TRUE(PollingWaitUntil(
+      "doneFrameCapturing()", "done-capturing", right_tab,
+      polling_interval_msec));
 
   HangUp(left_tab);
   WaitUntilHangupVerified(left_tab);
@@ -510,8 +492,9 @@ IN_PROC_BROWSER_TEST_F(WebrtcVideoQualityBrowserTest,
   AssertNoAsynchronousErrors(right_tab);
 
   // TODO(phoglund): (de-dupe later) different from original flow.
-  EXPECT_TRUE(UglyPollingWaitUntil(
-      "haveMoreFramesToSend()", "no-more-frames", right_tab));
+  EXPECT_TRUE(PollingWaitUntil(
+      "haveMoreFramesToSend()", "no-more-frames", right_tab,
+      polling_interval_msec));
 
   RunARGBtoI420Converter(
       kVgaWidth, kVgaHeight, GetWorkingDir().Append(kCapturedYuvFileName));
diff --git a/chrome/browser/media/webrtc_browsertest_common.cc b/chrome/browser/media/webrtc_browsertest_common.cc
new file mode 100644
index 0000000..83c24fc
--- /dev/null
+++ b/chrome/browser/media/webrtc_browsertest_common.cc
@@ -0,0 +1,80 @@
+// 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/browser/media/webrtc_browsertest_common.h"
+
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/process_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/test_timeouts.h"
+#include "base/time/time.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/infobars/infobar.h"
+#include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/media/media_stream_infobar_delegate.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "chrome/test/ui/ui_test.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/test/browser_test_utils.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+
+const int kDefaultPollIntervalMsec = 250;
+
+bool SleepInJavascript(content::WebContents* tab_contents, int timeout_msec) {
+  const std::string javascript = base::StringPrintf(
+      "setTimeout(function() {"
+      "  window.domAutomationController.send('sleep-ok');"
+      "}, %d)", timeout_msec);
+
+  std::string result;
+  bool ok = content::ExecuteScriptAndExtractString(
+      tab_contents, javascript, &result);
+  return ok && result == "sleep-ok";
+}
+
+bool PollingWaitUntil(const std::string& javascript,
+                      const std::string& evaluates_to,
+                      content::WebContents* tab_contents) {
+  return PollingWaitUntil(javascript, evaluates_to, tab_contents,
+                          kDefaultPollIntervalMsec);
+}
+
+bool PollingWaitUntil(const std::string& javascript,
+                      const std::string& evaluates_to,
+                      content::WebContents* tab_contents,
+                      int poll_interval_msec) {
+  base::Time start_time = base::Time::Now();
+  base::TimeDelta timeout = TestTimeouts::action_max_timeout();
+  std::string result;
+
+  while (base::Time::Now() - start_time < timeout) {
+    std::string result;
+    if (!content::ExecuteScriptAndExtractString(tab_contents, javascript,
+                                                &result)) {
+      LOG(ERROR) << "Failed to execute javascript " << javascript;
+      return false;
+    }
+
+    if (evaluates_to == result)
+      return true;
+
+    // Sleep a bit here to keep this loop from spinlocking too badly.
+    if (!SleepInJavascript(tab_contents, poll_interval_msec)) {
+      // TODO(phoglund): Figure out why this fails every now and then.
+      // It's not a huge deal if it does though.
+      LOG(ERROR) << "Failed to sleep.";
+    }
+  }
+  LOG(ERROR) << "Timed out while waiting for " << javascript <<
+      " to evaluate to " << evaluates_to << "; last result was '" << result <<
+      "'";
+  return false;
+}
diff --git a/chrome/browser/media/webrtc_browsertest_common.h b/chrome/browser/media/webrtc_browsertest_common.h
new file mode 100644
index 0000000..19d7ae9
--- /dev/null
+++ b/chrome/browser/media/webrtc_browsertest_common.h
@@ -0,0 +1,28 @@
+// 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.
+
+#ifndef CHROME_BROWSER_MEDIA_WEBRTC_BROWSERTEST_COMMON_H_
+#define CHROME_BROWSER_MEDIA_WEBRTC_BROWSERTEST_COMMON_H_
+
+#include <string>
+
+namespace content {
+class WebContents;
+}
+
+// This function will execute the provided |javascript| until it causes a call
+// to window.domAutomationController.send() with |evaluates_to| as the message.
+// That is, we are NOT checking what the javascript evaluates to. Returns false
+// if we exceed the TestTimeouts::action_max_timeout().
+// TODO(phoglund): Consider a better interaction method with the javascript
+// than polling javascript methods.
+bool PollingWaitUntil(const std::string& javascript,
+                      const std::string& evaluates_to,
+                      content::WebContents* tab_contents);
+bool PollingWaitUntil(const std::string& javascript,
+                      const std::string& evaluates_to,
+                      content::WebContents* tab_contents,
+                      int poll_interval_msec);
+
+#endif  // CHROME_BROWSER_MEDIA_WEBRTC_BROWSERTEST_COMMON_H_
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 1412782..e16aecd 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1453,6 +1453,7 @@
         'browser/media/chrome_media_stream_infobar_browsertest.cc',
         'browser/media/chrome_webrtc_browsertest.cc',
         'browser/media/chrome_webrtc_video_quality_browsertest.cc',
+        'browser/media/webrtc_browsertest_common.cc',
         'browser/media_galleries/fileapi/itunes_finder_win_browsertest.cc',
         'browser/media_galleries/fileapi/media_file_validator_unittest.cc',
         'browser/media_galleries/media_galleries_dialog_controller_mock.cc',