Regression tests for two recently-fixed audio-related crashers.

chrome/browser/media/media_browsertest.cc

Index: chrome/browser/media/media_browsertest.cc
diff --git a/chrome/browser/media/media_browsertest.cc b/chrome/browser/media/media_browsertest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..82ff1a4062457fdc9191ff3e53543768efa475b4
--- /dev/null
+++ b/chrome/browser/media/media_browsertest.cc
@@ -0,0 +1,94 @@
+// Copyright (c) 2011 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 "base/bind.h"
+#include "base/callback.h"
+#include "base/test/test_timeouts.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/browser/tab_contents/tab_contents.h"
+
+class MediaBrowserTest : public InProcessBrowserTest {
+ public:
+  MediaBrowserTest()
+      : seek_jumper_url_(ui_test_utils::GetTestUrl(
+          FilePath("media"), FilePath("seek-jumper.html"))) {}

[scherkus (not reviewing), 2011/10/28 21:03:13]

indent +2

pedantic nit 'o mine: if entire ctor/dtor/fn + {} line doesn't fit on one line,
drop the }

[scherkus (not reviewing), 2011/10/28 21:03:13]

fix file path literals

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

I don't think so - as a continuation line it indents +4 from the previous line's
first char, which in this case is a colon.

+
+  virtual void SetUp() OVERRIDE {
+    EnableDOMAutomation();

[scherkus (not reviewing), 2011/10/28 21:03:13]

sanity check: can we not move this to ctor then avoid calling IPBTest::SetUp() ?

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

Done.

+    InProcessBrowserTest::SetUp();
+  }
+
+ protected:
+  // Loop until |closure| returns true, CHECK-failing if
+  // this fails to happen within a reasonable amount of time.
+  void WaitForSuccess(const base::Callback<bool(void)>& closure) {
+    const int kLoopIntervalMs = 250;
+    const int kNumIntervals =
+        TestTimeouts::action_timeout_ms() / kLoopIntervalMs;
+    for (int i = 0; i < kNumIntervals; ++i) {
+      if (closure.Run())
+        return;
+      MessageLoop::current()->PostDelayedTask(
+          FROM_HERE, new MessageLoop::QuitTask(), kLoopIntervalMs);
+      ui_test_utils::RunMessageLoop();
+    }
+    LOG(FATAL) << "Timed out.";
+  }
+
+  const GURL seek_jumper_url_;
+};
+
+// Helper that executes |js_expr| in |tab| and returns its evaluated value as a
+// bool.
+static bool ExecuteJavaScriptAsBool(TabContents* tab,
+                                    const std::wstring& js_expr) {
+  bool result = false;
+  CHECK(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      tab->render_view_host(), L"", js_expr, &result));
+  return result;
+}
+
+// Helper that returns true iff |browser| has |expected_tab_count| tabs.
+static bool IsTabCount(Browser* browser, int expected_tab_count) {
+  return browser->tab_count() == expected_tab_count;
+}
+
+// Regression test: pending seeks shouldn't crash the browser when the tab is
+// closed.
+IN_PROC_BROWSER_TEST_F(MediaBrowserTest, SeekJumperAlone) {

[scherkus (not reviewing), 2011/10/28 21:03:13]

nit: for prefixed test names we've typically added a _ to separate the cases

SeekJumper_Alone
SeekJumper_SharedRenderer

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

Done.

+  ui_test_utils::NavigateToURL(browser(), seek_jumper_url_);
+  WaitForSuccess(base::Bind(
+      &ExecuteJavaScriptAsBool,
+      browser()->GetTabContentsAt(0),
+      L"window.domAutomationController.send(seekCount > 100)"));
+  CHECK(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_W, true, false, false, false));
+  // Lack of crash is our SUCCESS.
+}
+

[scherkus (not reviewing), 2011/10/28 21:03:13]

remove blank line

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

Done.

+
+// Regression test: pending seeks shouldn't crash a shared renderer when the tab
+// containing the audio element is closed.
+IN_PROC_BROWSER_TEST_F(MediaBrowserTest, SeekJumperSharedRenderer) {
+

[scherkus (not reviewing), 2011/10/28 21:03:13]

remove blank line

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

Done.

+  ui_test_utils::NavigateToURL(browser(), seek_jumper_url_);
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), seek_jumper_url_, NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  WaitForSuccess(base::Bind(&IsTabCount, browser(), 2));
+  WaitForSuccess(base::Bind(
+      &ExecuteJavaScriptAsBool,
+      browser()->GetTabContentsAt(0),
+      L"window.domAutomationController.send(seekCount > 100)"));
+  CHECK(ui_test_utils::SendKeyPressSync(
+      browser(), ui::VKEY_W, true, false, false, false));
+  WaitForSuccess(base::Bind(&IsTabCount, browser(), 1));
+  // Give the renderer a bit of time to crash.  Sad but necessary.
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, new MessageLoop::QuitTask(), 2000);

[scherkus (not reviewing), 2011/10/28 21:03:13]

test timeouts?

[Ami GONE FROM CHROMIUM, 2011/10/28 21:54:28]

I don't think so; none of them are a good fit.  Note that this *always* waits
this long; it's not a timeout value.  Hence the "Sad" in the preceding comment.

+  ui_test_utils::RunMessageLoop();
+  ASSERT_FALSE(browser()->GetTabContentsAt(0)->is_crashed());
+}