Import keywords for search engines imported from Firefox

chrome/browser/importer/firefox_importer_browsertest.cc

 // Copyright (c) 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 "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
@@ skipping 28 matching lines @@
   const char* action;
   const char* realm;
   const char* username_element;
   const char* username;
   const char* password_element;
   const char* password;
   bool blacklisted;
 };
 
 struct KeywordInfo {
-  const wchar_t* keyword;
-  const char* url;
+  KeywordInfo(const char* keyword, const char* url)
+      : keyword(keyword), url(url) {}
+  KeywordInfo() {}
+  std::string keyword;
+  std::string url;
 };
 
+std::vector<KeywordInfo> search_list;
+
 const BookmarkInfo kFirefoxBookmarks[] = {
   {true, 1, {"Bookmarks Toolbar"},
     L"Toolbar",
     "http://site/"},
   {false, 0, {},
     L"Title",
     "http://www.google.com/"},
 };
 
 const PasswordInfo kFirefoxPasswords[] = {
   {"http://localhost:8080/", "http://localhost:8080/", "http://localhost:8080/",
     "loginuser", "abc", "loginpass", "123", false},
   {"http://localhost:8080/", "", "http://localhost:8080/localhost",
     "", "http", "", "Http1+1abcdefg", false},
 };
 
-const KeywordInfo kFirefoxKeywords[] = {
-  { L"amazon.com",
-    "http://www.amazon.com/exec/obidos/external-search/?field-keywords="
-    "{searchTerms}&mode=blended" },
-  { L"answers.com",
-    "http://www.answers.com/main/ntquery?s={searchTerms}&gwp=13" },
-  { L"search.creativecommons.org",
-    "http://search.creativecommons.org/?q={searchTerms}" },
-  { L"search.ebay.com",
-    "http://search.ebay.com/search/search.dll?query={searchTerms}&"
-    "MfcISAPICommand=GetResult&ht=1&ebaytag1=ebayreg&srchdesc=n&"
-    "maxRecordsReturned=300&maxRecordsPerPage=50&SortProperty=MetaEndSort" },
-  { L"google.com",
-    "http://www.google.com/search?q={searchTerms}&ie=utf-8&oe=utf-8&aq=t" },
-  { L"en.wikipedia.org",
-    "http://en.wikipedia.org/wiki/Special:Search?search={searchTerms}" },
-  { L"search.yahoo.com",
-    "http://search.yahoo.com/search?p={searchTerms}&ei=UTF-8" },
-  { L"flickr.com",
-    "http://www.flickr.com/photos/tags/?q={searchTerms}" },
-  { L"imdb.com",
-    "http://www.imdb.com/find?q={searchTerms}" },
-  { L"webster.com",
-    "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}" },
-  // Search keywords.
-  { L"\x4E2D\x6587", "http://www.google.com/" },
-};
-
 class FirefoxObserver : public ProfileWriter,
                         public importer::ImporterProgressObserver {
  public:
   FirefoxObserver()
       : ProfileWriter(NULL),
         bookmark_count_(0),
         history_count_(0),
-        password_count_(0),
-        keyword_count_(0) {}
+        password_count_(0) {}
 
   // importer::ImporterProgressObserver:
   virtual void ImportStarted() OVERRIDE {}
   virtual void ImportItemStarted(importer::ImportItem item) OVERRIDE {}
   virtual void ImportItemEnded(importer::ImportItem item) OVERRIDE {}
   virtual void ImportEnded() OVERRIDE {
     base::MessageLoop::current()->Quit();
     EXPECT_EQ(arraysize(kFirefoxBookmarks), bookmark_count_);
     EXPECT_EQ(1U, history_count_);
     EXPECT_EQ(arraysize(kFirefoxPasswords), password_count_);
-    EXPECT_EQ(arraysize(kFirefoxKeywords), keyword_count_);
   }
 
   virtual bool BookmarkModelIsLoaded() const OVERRIDE {
     // Profile is ready for writing.
     return true;
   }
 
   virtual bool TemplateURLServiceIsLoaded() const OVERRIDE {
     return true;
   }
@@ skipping 34 matching lines @@
     for (size_t i = 0; i < bookmarks.size(); ++i) {
       EXPECT_NO_FATAL_FAILURE(
           TestEqualBookmarkEntry(bookmarks[i],
                                  kFirefoxBookmarks[bookmark_count_])) << i;
       ++bookmark_count_;
     }
   }
 
   virtual void AddKeywords(ScopedVector<TemplateURL> template_urls,
                            bool unique_on_host_and_path) OVERRIDE {
     for (size_t i = 0; i < template_urls.size(); ++i) {

[Peter Kasting, 2014/08/11 22:25:05]

Nit: Prefer iterators to indexes when the actual index value isn't used elswhere
(3 places)

[Nikhil, 2014/08/12 09:48:55]

Done.

-      // The order might not be deterministic, look in the expected list for
-      // that template URL.
-      bool found = false;
-      const base::string16& keyword = template_urls[i]->keyword();
-      for (size_t j = 0; j < arraysize(kFirefoxKeywords); ++j) {
-        if (keyword == base::WideToUTF16(kFirefoxKeywords[j].keyword)) {
-          EXPECT_EQ(kFirefoxKeywords[j].url, template_urls[i]->url());
-          found = true;
-          break;
-        }
-      }
-      EXPECT_TRUE(found);
-      ++keyword_count_;
+      KeywordInfo item;
+      item.keyword = base::UTF16ToUTF8(template_urls[i]->keyword());
+      item.url = template_urls[i]->url();

[Peter Kasting, 2014/08/11 22:25:05]

You added a 2-arg constructor for KeywordInfo, so why not use it here?

[Nikhil, 2014/08/12 09:48:55]

Done.

+      search_list.push_back(item);
     }
   }
 
   virtual void AddFavicons(
       const std::vector<ImportedFaviconUsage>& favicons) OVERRIDE {
   }
 
  private:
   virtual ~FirefoxObserver() {}
 
   size_t bookmark_count_;
   size_t history_count_;
   size_t password_count_;
-  size_t keyword_count_;
 };
 
 }  // namespace
 
 // These tests need to be browser tests in order to be able to run the OOP
 // import (via ExternalProcessImporterHost) which launches a utility process on
 // supported platforms.
 class FirefoxProfileImporterBrowserTest : public InProcessBrowserTest {
  protected:
   virtual void SetUp() OVERRIDE {
     // Creates a new profile in a new subdirectory in the temp directory.
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     base::FilePath test_path = temp_dir_.path().AppendASCII("ImporterTest");
     base::DeleteFile(test_path, true);
     base::CreateDirectory(test_path);
     profile_path_ = test_path.AppendASCII("profile");
     app_path_ = test_path.AppendASCII("app");
     base::CreateDirectory(app_path_);
 
     // This will launch the browser test and thus needs to happen last.
     InProcessBrowserTest::SetUp();
   }
 
+  void VerifyImportedSearchEngines(std::vector<KeywordInfo*>& expected_data) {
+    EXPECT_EQ(expected_data.size(), search_list.size());
+
+    for (size_t i = 0; i < expected_data.size(); ++i) {
+      // The order might not be deterministic, look in the expected list for
+      // that template URL.
+      bool found = false;
+      const std::string& keyword = expected_data[i]->keyword;
+      for (size_t j = 0; j < search_list.size(); ++j) {
+        if (keyword == search_list[j].keyword) {
+          EXPECT_EQ(expected_data[i]->url, search_list[j].url);
+          found = true;
+          break;
+        }
+      }
+      EXPECT_TRUE(found);
+    }
+  }
+
   void FirefoxImporterBrowserTest(std::string profile_dir,
                                   importer::ImporterProgressObserver* observer,
                                   ProfileWriter* writer) {
     base::FilePath data_path;
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
     data_path = data_path.AppendASCII(profile_dir);
     ASSERT_TRUE(base::CopyDirectory(data_path, profile_path_, true));
 
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
     data_path = data_path.AppendASCII("firefox3_nss");
@@ skipping 53 matching lines @@
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(Firefox35Importer)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
   FirefoxImporterBrowserTest(
       "firefox35_profile", observer.get(), observer.get());
 }
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(FirefoxImporter)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+
+  // Expected data for search engine list and keywords.
+  std::vector<KeywordInfo*> expected_data;

[Peter Kasting, 2014/08/11 22:25:05]

Why does this take objects by pointer instead of value?  This results in the
test leaking all added objects.

[Nikhil, 2014/08/12 09:48:54]

Done.

+  expected_data.push_back(
+      new KeywordInfo("amazon.com",
+                      "http://www.amazon.com/exec/obidos/external-search/"
+                      "?field-keywords={searchTerms}&mode=blended"));
+  expected_data.push_back(new KeywordInfo(
+      "google.com",
+      "http://www.google.com/search?q={searchTerms}&ie=utf-8&oe=utf-8&aq=t"));
+  expected_data.push_back(new KeywordInfo(
+      "wiki",
+      "http://en.wikipedia.org/wiki/Special:Search?search={searchTerms}"));
+  expected_data.push_back(new KeywordInfo(
+      "webster.com",
+      "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}"));
+  expected_data.push_back(new KeywordInfo(
+      base::WideToUTF8(L"\x4E2D\x6587").c_str(), "http://www.google.com/"));
+
+  // Import search engines.
   FirefoxImporterBrowserTest("firefox_profile", observer.get(), observer.get());
+
+  // Verify imported search engine list and keywords.
+  VerifyImportedSearchEngines(expected_data);
+
+  expected_data.clear();

[Peter Kasting, 2014/08/11 22:25:05]

This statement isn't necessary.

[Nikhil, 2014/08/12 09:48:55]

Done.

 }