Straighten up Translate browser tests.

There was a potential for a subtle race built into these tests:
if the timing of determining the language shifted, the observer
may end up waiting forever. This is what's happening in
https://codereview.chromium.org/1398823004, and the race
becomes visible.

Instead, let's reogranize observation of
chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED to be managed
at the test harness level and ensure that observation begins
outside of the message loop pump.

Also, let's move a very similar test from browser_browsertest
and convert it to use the same machinery.

Finally, refactor that test to remove another subtle race:
it actually opens two tabs and receives two notifications,
but that is only vaguely implied by the test logic.

R=jam,jochen,toyoshim@chromium.org
BUG=521166

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

Cr-Commit-Position: refs/heads/master@{#357176}

chrome/browser/translate/translate_manager_browsertest.cc

@@ -15,9 +15,91 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/translate/core/common/language_detection_details.h"
+#include "content/public/browser/notification_service.h"
 #include "url/gurl.h"
 
-class TranslateManagerBrowserTest : public InProcessBrowserTest {};
+class TranslateManagerBrowserTest : public InProcessBrowserTest {
+ public:
+  TranslateManagerBrowserTest() {}
+  ~TranslateManagerBrowserTest() override {}
+
+  std::string GetLanguageFor(content::WebContents* web_contents) {
+    translate::LanguageDetectionDetails details;
+    content::Source<content::WebContents> source(web_contents);
+    language_detected_signal_->GetDetailsFor(source.map_key(), &details);
+    return details.adopted_language;
+  }
+
+  void WaitUntilLanguageDetected() { language_detected_signal_->Wait(); }
+
+  void ResetObserver() {
+    language_detected_signal_.reset(new LangageDetectionObserver(
+        chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
+        content::NotificationService::AllSources()));
+  }
+
+ protected:
+  // InProcessBrowserTest members.
+  void SetUp() override {
+    scoped_ptr<test::CldDataHarness> cld_data_harness =
+        test::CldDataHarnessFactory::Get()->CreateCldDataHarness();
+    ASSERT_NO_FATAL_FAILURE(cld_data_harness->Init());
+    InProcessBrowserTest::SetUp();
+  }
+  void SetUpOnMainThread() override {
+    ResetObserver();
+    InProcessBrowserTest::SetUpOnMainThread();
+  }
+
+ private:
+  using LangageDetectionObserver =
+      ui_test_utils::WindowedNotificationObserverWithDetails<
+          translate::LanguageDetectionDetails>;
+
+  scoped_ptr<LangageDetectionObserver> language_detected_signal_;
+};
+
+// Tests that the CLD (Compact Language Detection) works properly.
+IN_PROC_BROWSER_TEST_F(TranslateManagerBrowserTest, PageLanguageDetection) {
+  ASSERT_TRUE(test_server()->Start());
+
+  // The InProcessBrowserTest opens a new tab, let's wait for that first.
+  WaitUntilLanguageDetected();
+
+  content::WebContents* current_web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  std::string adopted_language = GetLanguageFor(current_web_contents);
+  EXPECT_EQ("und", adopted_language);
+
+  // Open a new tab with a page in English.
+  AddTabAtIndex(0, GURL(test_server()->GetURL("files/english_page.html")),
+                ui::PAGE_TRANSITION_TYPED);
+
+  ResetObserver();
+
+  current_web_contents = browser()->tab_strip_model()->GetActiveWebContents();
+  ChromeTranslateClient* chrome_translate_client =
+      ChromeTranslateClient::FromWebContents(current_web_contents);
+
+  WaitUntilLanguageDetected();
+  adopted_language = GetLanguageFor(current_web_contents);
+  EXPECT_EQ("en", adopted_language);
+  EXPECT_EQ("en",
+            chrome_translate_client->GetLanguageState().original_language());
+
+  ResetObserver();
+
+  // Now navigate to a page in French.
+  ui_test_utils::NavigateToURL(
+      browser(), GURL(test_server()->GetURL("files/french_page.html")));
+
+  WaitUntilLanguageDetected();
+  adopted_language = GetLanguageFor(current_web_contents);
+  EXPECT_EQ("fr", adopted_language);
+  EXPECT_EQ("fr",
+            chrome_translate_client->GetLanguageState().original_language());
+}
 
 // Test is flaky on Win http://crbug.com/166334
 #if defined(OS_WIN)
@@ -29,31 +111,28 @@ class TranslateManagerBrowserTest : public InProcessBrowserTest {};
 // settings.
 IN_PROC_BROWSER_TEST_F(TranslateManagerBrowserTest,
                        MAYBE_PRE_TranslateSessionRestore) {
-  scoped_ptr<test::CldDataHarness> cld_data_harness =
-      test::CldDataHarnessFactory::Get()->CreateCldDataHarness();
-  ASSERT_NO_FATAL_FAILURE(cld_data_harness->Init());
   SessionStartupPref pref(SessionStartupPref::LAST);
   SessionStartupPref::SetStartupPref(browser()->profile(), pref);
 
+  WaitUntilLanguageDetected();
+
   content::WebContents* current_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ChromeTranslateClient* chrome_translate_client =
       ChromeTranslateClient::FromWebContents(current_web_contents);
-  content::Source<content::WebContents> source(current_web_contents);
 
-  ui_test_utils::WindowedNotificationObserverWithDetails<
-      translate::LanguageDetectionDetails>
-      fr_language_detected_signal(chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
-                                  source);
+  std::string adopted_language = GetLanguageFor(current_web_contents);
+  EXPECT_EQ("und", adopted_language);
+
+  ResetObserver();
 
   GURL french_url = ui_test_utils::GetTestUrl(
       base::FilePath(), base::FilePath(FILE_PATH_LITERAL("french_page.html")));
   ui_test_utils::NavigateToURL(browser(), french_url);
-  fr_language_detected_signal.Wait();
-  translate::LanguageDetectionDetails details;
-  EXPECT_TRUE(fr_language_detected_signal.GetDetailsFor(
-        source.map_key(), &details));
-  EXPECT_EQ("fr", details.adopted_language);
+
+  WaitUntilLanguageDetected();
+  adopted_language = GetLanguageFor(current_web_contents);
+  EXPECT_EQ("fr", adopted_language);
   EXPECT_EQ("fr",
             chrome_translate_client->GetLanguageState().original_language());
 }
@@ -65,16 +144,28 @@ IN_PROC_BROWSER_TEST_F(TranslateManagerBrowserTest,
 #endif
 IN_PROC_BROWSER_TEST_F(TranslateManagerBrowserTest,
                        MAYBE_TranslateSessionRestore) {
-  scoped_ptr<test::CldDataHarness> cld_data_harness =
-      test::CldDataHarnessFactory::Get()->CreateCldDataHarness();
-  ASSERT_NO_FATAL_FAILURE(cld_data_harness->Init());
-  content::WebContents* current_web_contents =
+  WaitUntilLanguageDetected();
+
+  content::WebContents* active_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
-  content::Source<content::WebContents> source(current_web_contents);
+  std::string active_adopted_language = GetLanguageFor(active_web_contents);
+
+  content::WebContents* restored_web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(0);
+  std::string restored_adopted_language = GetLanguageFor(restored_web_contents);
+
+  // One of the tabs could be still loading, let's check on that and wait,
+  // if necessary.
+  if (active_adopted_language.empty()) {
+    ResetObserver();
+    WaitUntilLanguageDetected();
+    active_adopted_language = GetLanguageFor(active_web_contents);
+  } else if (restored_adopted_language.empty()) {
+    ResetObserver();
+    WaitUntilLanguageDetected();
+    restored_adopted_language = GetLanguageFor(restored_web_contents);
+  }
 
-  ui_test_utils::WindowedNotificationObserverWithDetails<
-      translate::LanguageDetectionDetails>
-      fr_language_detected_signal(chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
-                                  source);
-  fr_language_detected_signal.Wait();
+  EXPECT_EQ("fr", restored_adopted_language);
+  EXPECT_EQ("und", active_adopted_language);
 }

chrome/browser/ui/browser_browsertest.cc

@@ -1604,54 +1604,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, ShouldShowLocationBar) {
   DevToolsWindowTesting::CloseDevToolsWindowSync(devtools_window);
 }
 
-// Tests that the CLD (Compact Language Detection) works properly.
-IN_PROC_BROWSER_TEST_F(BrowserTest, PageLanguageDetection) {
-  scoped_ptr<test::CldDataHarness> cld_data_harness =
-      test::CldDataHarnessFactory::Get()->CreateCldDataHarness();
-  ASSERT_NO_FATAL_FAILURE(cld_data_harness->Init());
-  ASSERT_TRUE(test_server()->Start());
-
-  translate::LanguageDetectionDetails details;
-
-  // Open a new tab with a page in English.
-  AddTabAtIndex(0, GURL(test_server()->GetURL("files/english_page.html")),
-                ui::PAGE_TRANSITION_TYPED);
-
-  WebContents* current_web_contents =
-      browser()->tab_strip_model()->GetActiveWebContents();
-  ChromeTranslateClient* chrome_translate_client =
-      ChromeTranslateClient::FromWebContents(current_web_contents);
-  content::Source<WebContents> source(current_web_contents);
-
-  ui_test_utils::WindowedNotificationObserverWithDetails<
-      translate::LanguageDetectionDetails>
-      en_language_detected_signal(chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
-                                  source);
-  EXPECT_EQ("",
-            chrome_translate_client->GetLanguageState().original_language());
-  en_language_detected_signal.Wait();
-  EXPECT_TRUE(en_language_detected_signal.GetDetailsFor(
-        source.map_key(), &details));
-  EXPECT_EQ("en", details.adopted_language);
-  EXPECT_EQ("en",
-            chrome_translate_client->GetLanguageState().original_language());
-
-  // Now navigate to a page in French.
-  ui_test_utils::WindowedNotificationObserverWithDetails<
-      translate::LanguageDetectionDetails>
-      fr_language_detected_signal(chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
-                                  source);
-  ui_test_utils::NavigateToURL(
-      browser(), GURL(test_server()->GetURL("files/french_page.html")));
-  fr_language_detected_signal.Wait();
-  details.adopted_language.clear();
-  EXPECT_TRUE(fr_language_detected_signal.GetDetailsFor(
-        source.map_key(), &details));
-  EXPECT_EQ("fr", details.adopted_language);
-  EXPECT_EQ("fr",
-            chrome_translate_client->GetLanguageState().original_language());
-}
-
 // Chromeos defaults to restoring the last session, so this test isn't
 // applicable.
 #if !defined(OS_CHROMEOS)