Adding more bidichecker tests and doing some minor cleanups.

chrome/browser/ui/webui/bidi_checker_web_ui_test.cc

Index: chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
diff --git a/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc b/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
index 5eef27045283b9ea191e6f00a458fa88d65652f7..a1264ecbe67eaa87ad81ab8f4c1a45edf8d51e3f 100644
--- a/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
+++ b/chrome/browser/ui/webui/bidi_checker_web_ui_test.cc
@@ -14,6 +14,8 @@
 #include "chrome/browser/autofill/personal_data_manager.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
 #include "chrome/browser/history/history.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/url_constants.h"
@@ -51,18 +53,28 @@ void WebUIBidiCheckerBrowserTest::SetUpInProcessBrowserTestFixture() {
 
 void WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(const char pageURL[],
                                                        bool isRTL) {
-  ui_test_utils::NavigateToURL(browser(), GURL(pageURL));
+  if (pageURL)

[jeremy, 2011/10/16 12:54:59]

What is this line supposed to do?

[ofri1, 2011/10/23 15:22:36]

Hmm, no idea. Removed.

+    ui_test_utils::NavigateToURL(browser(), GURL(pageURL));
   ASSERT_TRUE(RunJavascriptTest("runBidiChecker",
                                 Value::CreateStringValue(pageURL),
                                 Value::CreateBooleanValue(isRTL)));
 }
 
+void WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(const char pageURL[]) {
+  RunBidiCheckerOnPage(pageURL, false);
+}
+
 // WebUIBidiCheckerBrowserTestFakeBidi
 
 WebUIBidiCheckerBrowserTestFakeBidi::~WebUIBidiCheckerBrowserTestFakeBidi() {}
 
 WebUIBidiCheckerBrowserTestFakeBidi::WebUIBidiCheckerBrowserTestFakeBidi() {}
 
+void WebUIBidiCheckerBrowserTestFakeBidi::RunBidiCheckerOnPage(
+    const char pageURL[]) {
+  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(pageURL, true);
+}
+
 void WebUIBidiCheckerBrowserTestFakeBidi::SetUpOnMainThread() {
   WebUIBidiCheckerBrowserTest::SetUpOnMainThread();
   FilePath pak_path;
@@ -92,41 +104,50 @@ void WebUIBidiCheckerBrowserTestFakeBidi::CleanUpOnMainThread() {
 
 // Tests
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestMainHistoryPageLTR) {
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestMainHistoryPage) {

[jeremy, 2011/10/16 12:54:59]

This only tests the page as LTR, right so why rename the test?

   HistoryService* history_service =
       browser()->profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
-  GURL history_url = GURL("http://www.ynet.co.il");
+  const GURL history_url = GURL("http://www.ynet.co.il");
   history_service->AddPage(history_url, history::SOURCE_BROWSED);
   string16 title;
   ASSERT_TRUE(UTF8ToUTF16("\xD7\x91\xD7\x93\xD7\x99\xD7\xA7\xD7\x94\x21",
                           12,
                           &title));
   history_service->SetPageTitle(history_url, title);
-  RunBidiCheckerOnPage(chrome::kChromeUIHistoryURL, false);
+  RunBidiCheckerOnPage(chrome::kChromeUIHistoryURL);
 }
 
 IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       TestMainHistoryPageRTL) {
+                       TestMainHistoryPage) {
   HistoryService* history_service =
       browser()->profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
-  GURL history_url = GURL("http://www.google.com");
+  const GURL history_url = GURL("http://www.google.com");
   history_service->AddPage(history_url, history::SOURCE_BROWSED);
   string16 title = UTF8ToUTF16("Google");
   history_service->SetPageTitle(history_url, title);
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(chrome::kChromeUIHistoryURL,
-                                                    true);
+  RunBidiCheckerOnPage(chrome::kChromeUIHistoryURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestAboutPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUIAboutURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestAboutPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUIAboutURL);
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestBugReportPage) {
+  RunBidiCheckerOnPage(
+      "chrome://bugreport#0?description=%D7%91%D7%93%D7%99%D7%A7%D7%94&issueType=1");

[jeremy, 2011/10/16 12:54:59]

Could you add a comment explaining what the weird escaped characters are?

[ofri1, 2011/10/23 15:22:36]

Done.

+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestBugReportPage) {
+  RunBidiCheckerOnPage("chrome://bugreport#0?description=test&issueType=1");
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestBugReportPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUIBugReportURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestCrashesPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUICrashesURL);

[jeremy, 2011/10/16 12:54:59]

I'm pretty sure about:crash isn't HTML and that you can remove this test?

[ofri1, 2011/10/23 15:22:36]

This is not about:crash but about:crashes, which I just verified as HTML.

 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestCrashesPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUICrashesURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi, TestCrashesPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUICrashesURL);
 }
 
 IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
@@ -135,58 +156,51 @@ IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
                                                     true);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestDownloadsPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUIDownloadsURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestDownloadsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUIDownloadsURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       TestDownloadsPageRTL) {
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(
-      chrome::kChromeUIDownloadsURL, true);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi, TestDownloadsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUIDownloadsURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestNewTabPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUINewTabURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestNewTabPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUINewTabURL);
 }
 
 // http://crbug.com/97453
 IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       DISABLED_TestNewTabPageRTL) {
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(chrome::kChromeUINewTabURL,
-                                                    true);
+                       DISABLED_TestNewTabPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUINewTabURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestPluginsPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUIPluginsURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestPluginsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUIPluginsURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       TestPluginsPageRTL) {
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(chrome::kChromeUIPluginsURL,
-                                                    true);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi, TestPluginsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUIPluginsURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestSettingsPageLTR) {
-  RunBidiCheckerOnPage(chrome::kChromeUISettingsURL, false);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest, TestSettingsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUISettingsURL);
 }
 
-IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       TestSettingsPageRTL) {
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(
-      chrome::kChromeUISettingsURL, true);
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi, TestSettingsPage) {
+  RunBidiCheckerOnPage(chrome::kChromeUISettingsURL);
 }
 
 #if defined(OS_MACOSX)
 // http://crbug.com/94642
-#define MAYBE_TestSettingsAutofillPageLTR FLAKY_TestSettingsAutofillPageLTR
+#define MAYBE_TestSettingsAutofillPage FLAKY_TestSettingsAutofillPage

[jeremy, 2011/10/16 12:54:59]

Are these tests still flaky? Do you know what the status of the fix for them is?

 #elif defined(OS_WIN)
 // http://crbug.com/95425
-#define MAYBE_TestSettingsAutofillPageLTR FAILS_TestSettingsAutofillPageLTR
+#define MAYBE_TestSettingsAutofillPage FAILS_TestSettingsAutofillPage
 #else
-#define MAYBE_TestSettingsAutofillPageLTR TestSettingsAutofillPageLTR
+#define MAYBE_TestSettingsAutofillPage TestSettingsAutofillPage
 #endif  // defined(OS_MACOSX)
 IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
-                       MAYBE_TestSettingsAutofillPageLTR) {
+                       MAYBE_TestSettingsAutofillPage) {
   std::string url(chrome::kChromeUISettingsURL);
   url += std::string(chrome::kAutofillSubPage);
 
@@ -217,7 +231,7 @@ IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
-                       TestSettingsAutofillPageRTL) {
+                       TestSettingsAutofillPage) {
   std::string url(chrome::kChromeUISettingsURL);
   url += std::string(chrome::kAutofillSubPage);
 
@@ -244,5 +258,136 @@ IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
 
   personal_data_manager->AddProfile(profile);
 
-  WebUIBidiCheckerBrowserTest::RunBidiCheckerOnPage(url.c_str(), true);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsBorwserOptionsPage) {
+  // A common Israeli news site with mixed Hebrew/English title
+  GURL testGURL = GURL("http://ynet.co.il");

[jeremy, 2011/10/16 12:54:59]

Just to be sure - there's no chance we actually try to load the site when this
test is run, right?

[ofri1, 2011/10/23 15:22:36]

Right. The only way to get to the site is by manually clicking the link in the
history page.

+  // First, add a history entry for the site. This is needed so the site's
+  // name will appear in the startup sites lists.
+  HistoryService* history_service =
+  browser()->profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);

[jeremy, 2011/10/16 12:54:59]

Indent

[ofri1, 2011/10/23 15:22:36]

Done.

+  history_service->AddPage(testGURL, history::SOURCE_BROWSED);
+  string16 title;
+  // This is the real title of the site

[jeremy, 2011/10/16 12:54:59]

nit:End comments with a '.'

+  ASSERT_TRUE(UTF8ToUTF16("\x79\x6E\x65\x74\x20\xD7\x97\xD7\x93\xD7\xA9\xD7\x95"
+                          "\xD7\xAA\x20\xD7\xAA\xD7\x95\xD7\x9B\xD7\x9F\x20\xD7"
+                          "\x95\xD7\xA2\xD7\x93\xD7\x9B\xD7\x95\xD7\xA0\xD7\x99"
+                          "\xD7\x9D\x20\x2D\x20\xD7\x99\xD7\x93\xD7\x99\xD7\xA2"
+                          "\xD7\x95\xD7\xAA\x20\xD7\x90\xD7\x97\xD7\xA8\xD7\x95"
+                          "\xD7\xA0\xD7\x95\xD7\xAA",
+                          71,

[jeremy, 2011/10/16 12:54:59]

put the above in a char[] and use array_size() here.

[ofri1, 2011/10/23 15:22:36]

No need, found a cleaner alternative.

+                          &title));
+  history_service->SetPageTitle(testGURL, title);
+
+  // Next, add the site to the startup sites
+  PrefService* prefs = browser()->profile()->GetPrefs();
+  SessionStartupPref pref = SessionStartupPref::GetStartupPref(prefs);
+  pref.urls.push_back(testGURL);
+  SessionStartupPref::SetStartupPref(prefs, pref);
+
+  // Finally, unleash the BidiChecker
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kBrowserOptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsBorwserOptionsPage) {
+  // A search engine some people use
+  GURL testGURL = GURL("http://google.com");

[jeremy, 2011/10/16 12:54:59]

Since both these tests do the same thing, can you put them into a common
function?

[ofri1, 2011/10/23 15:22:36]

Done.

+  // First, add a history entry for the site. This is needed so the site's
+  // name will appear in the startup sites lists.
+  HistoryService* history_service =
+  browser()->profile()->GetHistoryService(Profile::IMPLICIT_ACCESS);
+  history_service->AddPage(testGURL, history::SOURCE_BROWSED);
+  string16 title;
+  // This is the real title of the site
+  ASSERT_TRUE(UTF8ToUTF16("Google",
+                          6,
+                          &title));
+  history_service->SetPageTitle(testGURL, title);
+
+  // Next, add the site to the startup sites
+  PrefService* prefs = browser()->profile()->GetPrefs();
+  SessionStartupPref pref = SessionStartupPref::GetStartupPref(prefs);
+  pref.urls.push_back(testGURL);
+  SessionStartupPref::SetStartupPref(prefs, pref);
+
+  // Finally, unleash the BidiChecker
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kBrowserOptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsClearBrowserDataPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kClearBrowserDataSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsClearBrowserDataPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kClearBrowserDataSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsContentSettingsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kContentSettingsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsContentSettingsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kContentSettingsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsContentSettingsExceptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kContentSettingsExceptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsContentSettingsExceptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kContentSettingsExceptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsLanguageOptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kLanguageOptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsLanguageOptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kLanguageOptionsSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTest,
+                       TestSettingsSearchEnginesOptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kSearchEnginesSubPage);
+  RunBidiCheckerOnPage(url.c_str());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUIBidiCheckerBrowserTestFakeBidi,
+                       TestSettingsSearchEnginesOptionsPage) {
+  std::string url(chrome::kChromeUISettingsURL);
+  url += std::string(chrome::kSearchEnginesSubPage);
+  RunBidiCheckerOnPage(url.c_str());
 }