Skip a first-run import on Mac when there is nothing left to import.

chrome/browser/first_run/first_run_browsertest.cc

Index: chrome/browser/first_run/first_run_browsertest.cc
diff --git a/chrome/browser/first_run/first_run_browsertest.cc b/chrome/browser/first_run/first_run_browsertest.cc
index fa789ac67ce5a6c30e8dfd083f136edb76c90288..b9c07e7dcf8e7bc790baecf53262d8332737b652 100644
--- a/chrome/browser/first_run/first_run_browsertest.cc
+++ b/chrome/browser/first_run/first_run_browsertest.cc
@@ -3,14 +3,20 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/component_loader.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/test/test_launcher.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -80,9 +86,57 @@ class FirstRunIntegrationBrowserTest : public InProcessBrowserTest {
  private:
   DISALLOW_COPY_AND_ASSIGN(FirstRunIntegrationBrowserTest);
 };
+
+class FirstRunMasterPrefsBrowserTest : public FirstRunIntegrationBrowserTest {
+ public:
+  FirstRunMasterPrefsBrowserTest() {}
+ protected:
+  virtual void SetUp() OVERRIDE {
+    FilePath prefs_file;
+    ASSERT_TRUE(file_util::CreateTemporaryFile(&prefs_file));

[cpu_(ooo_6.6-7.5), 2013/01/30 21:00:53]

who deletes this temporary file?

[tapted, 2013/01/31 00:47:16]

Fixed (how did I miss that..).

+    // TODO(tapted): Make this reusable.
+    const char text[] =
+        "{\n"
+        "  \"distribution\": {\n"
+        "    \"import_bookmarks\": false,\n"
+        "    \"import_history\": false,\n"
+        "    \"import_home_page\": false,\n"
+        "    \"import_search_engine\": false\n"
+        "  }\n"
+        "}\n";
+    EXPECT_TRUE(file_util::WriteFile(prefs_file, text, strlen(text)));
+    first_run::SetMasterPrefsPathForTesting(prefs_file);
+
+    // This invokes BrowserMain, and does the import, so must be done last.
+    FirstRunIntegrationBrowserTest::SetUp();
+  }
+ private:
+  DISALLOW_COPY_AND_ASSIGN(FirstRunMasterPrefsBrowserTest);
+};
 }
 
 IN_PROC_BROWSER_TEST_F(FirstRunIntegrationBrowserTest, WaitForImport) {
   ASSERT_TRUE(ProfileManager::DidPerformProfileImport());
 }
+
+// Test an import with all import options disabled. This is a regression test
+// for http://crbug.com/169984 where this would cause the import process to
+// stay running, and the NTP to be loaded with no apps.
+IN_PROC_BROWSER_TEST_F(FirstRunMasterPrefsBrowserTest,
+                       ImportNothingAndShowNewTabPage) {
+  ASSERT_TRUE(ProfileManager::DidPerformProfileImport());
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(chrome::kChromeUINewTabURL), CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  content::WebContents* tab = browser()->tab_strip_model()->GetWebContentsAt(0);
+  EXPECT_EQ(1, tab->GetMaxPageID());
+
+  // "Web Store" appears 3 times on the NTP. One is a component extension that
+  // is only added after the import process is complete.

[cpu_(ooo_6.6-7.5), 2013/01/30 21:00:53]

this test seems very fagile. There must be a better way to the detect the NTP?

[tapted, 2013/01/31 00:47:16]

This last step is more to address the specific observation from the bug report,
where the NTP loaded OK, but was missing the webstore app icon. The tests in
new_tab_ui_browsertest.cc stop after the check on GetMaxPageID (e.g. [1]).

ui_test_utils::FindInPage is used, e.g., in tests for about://version [2]. But
you are right that if the NTP changes the occurrences of 'Web Store', then this
test would need to be updated.

One alternative might be to convert the test to a WebUIBrowserTest, along the
lines of most_visited_page_test.js [3]. This might be able to check specifically
for the number of app icons displayed on the NTP (or just that one of them is
WebStore, in case more are added). Shall I pursue this?

Or, perhaps the last step is not that important and can be removed. Since, the
root-cause of the bug ended up being the import process hanging, and the code in
the SetUp is enough to cause this test to timeout without the fix.

Let me know what you think. 


[1]
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/ui/we...
[2]
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/ui/br...
[3]
https://code.google.com/p/chromium/codesearch#chrome/src/chrome/browser/ui/we...

+  EXPECT_EQ(
+      ui_test_utils::FindInPage(
+          tab, ASCIIToUTF16("Web Store"), true, false, NULL, NULL),
+      3);
+}
+
 #endif  // !defined(OS_CHROMEOS)