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(std::string keyword, std::string url)

[Peter Kasting, 2014/08/12 18:16:45]

Nit: Const refs

[Nikhil, 2014/08/13 05:52:41]

Done.

+      : keyword(keyword), url(url) {}
+  KeywordInfo() {}

[Peter Kasting, 2014/08/12 18:16:45]

Nit: Is this constructor necessary?  Maybe for the compiler to instantiate
vector<KeywordInfo>?  If not, remove.

[Nikhil, 2014/08/13 05:52:41]

Done.

+  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 32 matching lines @@
     // Importer should import the FF favorites the same as the list, in the same
     // order.
     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,

[Peter Kasting, 2014/08/12 18:16:45]

Nit: While here: Make this a const ref (requires changing the parent class)

[Peter Kasting, 2014/08/12 18:23:06]

Wait, don't do that.  This is trying to pass ownership, changing to a const ref
wouldn't be correct.

[Nikhil, 2014/08/13 05:52:41]

Acknowledged.

                            bool unique_on_host_and_path) OVERRIDE {
-    for (size_t i = 0; i < template_urls.size(); ++i) {
-      // 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_;
+    for (ScopedVector<TemplateURL>::const_iterator i = template_urls.begin();
+         i != template_urls.end();
+         ++i) {
+      search_list.push_back(
+          KeywordInfo(base::UTF16ToUTF8((*i)->keyword()), (*i)->url()));
     }
   }
 
   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) {

[Peter Kasting, 2014/08/12 18:16:45]

Nit: const arg

[Nikhil, 2014/08/13 05:52:41]

Done.

+    EXPECT_EQ(expected_data.size(), search_list.size());

[Peter Kasting, 2014/08/12 18:16:45]

Nit: (expected, actual)

[Nikhil, 2014/08/13 05:52:41]

Are you suggesting to change variable name for vectors? expected_data is
initializer list against whom search_list is compared. search_list is list of
imported search engines. So, (expected, actual) is maintained for comparison.

[Peter Kasting, 2014/08/13 06:21:07]

OK.

+
+    for (std::vector<KeywordInfo>::const_iterator i = expected_data.begin();
+         i != expected_data.end();
+         ++i) {
+      // The order might not be deterministic, look in the expected list for
+      // that template URL.
+      bool found = false;
+      const std::string& keyword = i->keyword;
+      for (std::vector<KeywordInfo>::const_iterator j = search_list.begin();
+           j != search_list.end();
+           ++j) {
+        if (keyword == j->keyword) {
+          EXPECT_EQ(i->url, 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 39 matching lines @@
   }
 
   base::ScopedTempDir temp_dir_;
   base::FilePath profile_path_;
   base::FilePath app_path_;
 };
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(Firefox30Importer)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+
+  std::vector<KeywordInfo> expected_data;
+  expected_data.push_back(KeywordInfo(
+      "amazon.com",
+      "http://www.amazon.com/exec/obidos/external-search/?field-keywords="
+      "{searchTerms}&mode=blended"));
+  expected_data.push_back(KeywordInfo(
+      "answers.com",
+      "http://www.answers.com/main/ntquery?s={searchTerms}&gwp=13"));
+  expected_data.push_back(
+      KeywordInfo("search.creativecommons.org",
+                  "http://search.creativecommons.org/?q={searchTerms}"));
+  expected_data.push_back(KeywordInfo(
+      "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"));
+  expected_data.push_back(KeywordInfo(
+      "google.com",
+      "http://www.google.com/search?q={searchTerms}&ie=utf-8&oe=utf-8&aq=t"));
+  expected_data.push_back(KeywordInfo(
+      "en.wikipedia.org",
+      "http://en.wikipedia.org/wiki/Special:Search?search={searchTerms}"));
+  expected_data.push_back(
+      KeywordInfo("search.yahoo.com",
+                  "http://search.yahoo.com/search?p={searchTerms}&ei=UTF-8"));
+  expected_data.push_back(KeywordInfo(
+      "flickr.com", "http://www.flickr.com/photos/tags/?q={searchTerms}"));
+  expected_data.push_back(
+      KeywordInfo("imdb.com", "http://www.imdb.com/find?q={searchTerms}"));
+  expected_data.push_back(KeywordInfo(
+      "webster.com",
+      "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}"));
+  expected_data.push_back(KeywordInfo(base::WideToUTF8(L"\x4E2D\x6587").c_str(),
+                                      "http://www.google.com/"));
+
   FirefoxImporterBrowserTest(
       "firefox3_profile", observer.get(), observer.get());
+
+  VerifyImportedSearchEngines(expected_data);
 }
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(Firefox35Importer)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+
+  std::vector<KeywordInfo> expected_data;
+  expected_data.push_back(KeywordInfo(
+      "amazon.com",
+      "http://www.amazon.com/exec/obidos/external-search/?field-keywords="
+      "{searchTerms}&mode=blended"));
+  expected_data.push_back(KeywordInfo(
+      "answers.com",
+      "http://www.answers.com/main/ntquery?s={searchTerms}&gwp=13"));
+  expected_data.push_back(
+      KeywordInfo("search.creativecommons.org",
+                  "http://search.creativecommons.org/?q={searchTerms}"));
+  expected_data.push_back(KeywordInfo(
+      "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"));
+  expected_data.push_back(KeywordInfo(
+      "google.com",
+      "http://www.google.com/search?q={searchTerms}&ie=utf-8&oe=utf-8&aq=t"));
+  expected_data.push_back(KeywordInfo(
+      "en.wikipedia.org",
+      "http://en.wikipedia.org/wiki/Special:Search?search={searchTerms}"));
+  expected_data.push_back(
+      KeywordInfo("search.yahoo.com",
+                  "http://search.yahoo.com/search?p={searchTerms}&ei=UTF-8"));
+  expected_data.push_back(KeywordInfo(
+      "flickr.com", "http://www.flickr.com/photos/tags/?q={searchTerms}"));
+  expected_data.push_back(
+      KeywordInfo("imdb.com", "http://www.imdb.com/find?q={searchTerms}"));
+  expected_data.push_back(KeywordInfo(
+      "webster.com",
+      "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}"));
+  expected_data.push_back(KeywordInfo(base::WideToUTF8(L"\x4E2D\x6587").c_str(),
+                                      "http://www.google.com/"));
+
   FirefoxImporterBrowserTest(
       "firefox35_profile", observer.get(), observer.get());
+
+  VerifyImportedSearchEngines(expected_data);
 }
 
 IN_PROC_BROWSER_TEST_F(FirefoxProfileImporterBrowserTest,
                        MAYBE_IMPORTER(FirefoxImporter)) {
   scoped_refptr<FirefoxObserver> observer(new FirefoxObserver());
+
+  // Expected data for search engine list and keywords.

[Peter Kasting, 2014/08/12 18:16:45]

Nit: These comments don't seem very necessary (and aren't present above), I'd
remove them.

[Nikhil, 2014/08/13 05:52:41]

Done.

+  std::vector<KeywordInfo> expected_data;
+  expected_data.push_back(
+      KeywordInfo("amazon.com",
+                  "http://www.amazon.com/exec/obidos/external-search/"
+                  "?field-keywords={searchTerms}&mode=blended"));
+  expected_data.push_back(KeywordInfo(
+      "google.com",
+      "http://www.google.com/search?q={searchTerms}&ie=utf-8&oe=utf-8&aq=t"));
+  expected_data.push_back(KeywordInfo(
+      "wiki",
+      "http://en.wikipedia.org/wiki/Special:Search?search={searchTerms}"));
+  expected_data.push_back(KeywordInfo(
+      "webster.com",
+      "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}"));
+  expected_data.push_back(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);
 }