diff options
26 files changed, 267 insertions, 112 deletions
diff --git a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc index 7e1a5ad..c4ee5df 100644 --- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc @@ -18,6 +18,7 @@ #include "chrome/browser/history/history_service_factory.h" #include "chrome/common/importer/imported_bookmark_entry.h" #include "chrome/common/importer/imported_favicon_usage.h" +#include "chrome/common/importer/importer_data_types.h" #include "chrome/test/base/testing_profile.h" #include "chrome/utility/importer/bookmark_html_reader.h" #include "components/bookmarks/browser/bookmark_model.h" @@ -242,11 +243,13 @@ TEST_F(BookmarkHTMLWriterTest, Test) { // Read the bookmarks back in. std::vector<ImportedBookmarkEntry> parsed_bookmarks; + std::vector<importer::SearchEngineInfo> parsed_search_engines; std::vector<ImportedFaviconUsage> favicons; bookmark_html_reader::ImportBookmarksFile(base::Callback<bool(void)>(), base::Callback<bool(const GURL&)>(), path_, &parsed_bookmarks, + &parsed_search_engines, &favicons); // Check loaded favicon (url1 is represented by 4 separate bookmarks). @@ -261,6 +264,10 @@ TEST_F(BookmarkHTMLWriterTest, Test) { } } + // Since we did not populate the BookmarkModel with any entry which can be + // imported as search engine, verify that we got back no search engines. + ASSERT_EQ(0U, parsed_search_engines.size()); + // Verify we got back what we wrote. ASSERT_EQ(9U, parsed_bookmarks.size()); // Windows and ChromeOS builds use Sentence case. diff --git a/chrome/browser/importer/external_process_importer_client.cc b/chrome/browser/importer/external_process_importer_client.cc index 85d3e47..116c72f 100644 --- a/chrome/browser/importer/external_process_importer_client.cc +++ b/chrome/browser/importer/external_process_importer_client.cc @@ -240,11 +240,11 @@ void ExternalProcessImporterClient::OnPasswordFormImportReady( } void ExternalProcessImporterClient::OnKeywordsImportReady( - const std::vector<importer::URLKeywordInfo>& url_keywords, + const std::vector<importer::SearchEngineInfo>& search_engines, bool unique_on_host_and_path) { if (cancelled_) return; - bridge_->SetKeywords(url_keywords, unique_on_host_and_path); + bridge_->SetKeywords(search_engines, unique_on_host_and_path); } void ExternalProcessImporterClient::OnFirefoxSearchEngineDataReceived( diff --git a/chrome/browser/importer/external_process_importer_client.h b/chrome/browser/importer/external_process_importer_client.h index 8328a50..feca90f 100644 --- a/chrome/browser/importer/external_process_importer_client.h +++ b/chrome/browser/importer/external_process_importer_client.h @@ -37,7 +37,7 @@ namespace importer { struct ImporterIE7PasswordInfo; #endif struct ImporterAutofillFormDataEntry; -struct URLKeywordInfo; +struct SearchEngineInfo; } // This class is the client for the out of process profile importing. It @@ -80,7 +80,7 @@ class ExternalProcessImporterClient : public content::UtilityProcessHostClient { const std::vector<ImportedFaviconUsage>& favicons_group); void OnPasswordFormImportReady(const autofill::PasswordForm& form); void OnKeywordsImportReady( - const std::vector<importer::URLKeywordInfo>& url_keywords, + const std::vector<importer::SearchEngineInfo>& search_engines, bool unique_on_host_and_path); void OnFirefoxSearchEngineDataReceived( const std::vector<std::string> search_engine_data); diff --git a/chrome/browser/importer/firefox_importer_browsertest.cc b/chrome/browser/importer/firefox_importer_browsertest.cc index 0076597..2044a52 100644 --- a/chrome/browser/importer/firefox_importer_browsertest.cc +++ b/chrome/browser/importer/firefox_importer_browsertest.cc @@ -106,6 +106,11 @@ const KeywordInfo kFirefoxKeywords[] = { "http://www.webster.com/cgi-bin/dictionary?va={searchTerms}"}, // Search keywords. {L"\x4E2D\x6587", L"\x4E2D\x6587", "http://www.google.com/"}, + {L"keyword", L"keyword", "http://example.{searchTerms}.com/"}, + // in_process_importer_bridge.cc:CreateTemplateURL() will return NULL for + // the following bookmark. Consequently, it won't be imported as a search + // engine. + {L"", L"", "http://%x.example.{searchTerms}.com/"}, }; const AutofillFormDataInfo kFirefoxAutofillEntries[] = { @@ -143,7 +148,11 @@ class FirefoxObserver : public ProfileWriter, EXPECT_EQ(arraysize(kFirefoxBookmarks), bookmark_count_); EXPECT_EQ(1U, history_count_); EXPECT_EQ(arraysize(kFirefoxPasswords), password_count_); - EXPECT_EQ(arraysize(kFirefoxKeywords), keyword_count_); + // The following test case from |kFirefoxKeywords| won't be imported: + // "http://%x.example.{searchTerms}.com/"}. + // Hence, value of |keyword_count_| should be lower than size of + // |kFirefoxKeywords| by 1. + EXPECT_EQ(arraysize(kFirefoxKeywords) - 1, keyword_count_); } bool BookmarkModelIsLoaded() const override { diff --git a/chrome/browser/importer/in_process_importer_bridge.cc b/chrome/browser/importer/in_process_importer_bridge.cc index 3bd1fde..c017f6e 100644 --- a/chrome/browser/importer/in_process_importer_bridge.cc +++ b/chrome/browser/importer/in_process_importer_bridge.cc @@ -91,25 +91,20 @@ class FirefoxURLParameterFilter : public TemplateURLParser::ParameterFilter { DISALLOW_COPY_AND_ASSIGN(FirefoxURLParameterFilter); }; -// Creates a TemplateURL with the |keyword| and |url|. |title| may be empty. +// Attempts to create a TemplateURL from the provided data. |title| is optional. +// If TemplateURL creation fails, returns NULL. // This function transfers ownership of the created TemplateURL to the caller. -TemplateURL* CreateTemplateURL(const base::string16& title, +TemplateURL* CreateTemplateURL(const base::string16& url, const base::string16& keyword, - const GURL& url) { - // Skip if the url is invalid. - if (!url.is_valid()) + const base::string16& title) { + if (url.empty() || keyword.empty()) return NULL; - TemplateURLData data; - if (keyword.empty()) - data.SetKeyword(TemplateURL::GenerateKeyword(url)); - else - data.SetKeyword(keyword); + data.SetKeyword(keyword); // We set short name by using the title if it exists. // Otherwise, we use the shortcut. data.short_name = title.empty() ? keyword : title; - data.SetURL( - TemplateURLRef::DisplayURLToURLRef(base::UTF8ToUTF16(url.spec()))); + data.SetURL(TemplateURLRef::DisplayURLToURLRef(url)); return new TemplateURL(data); } @@ -221,14 +216,16 @@ void InProcessImporterBridge::SetHistoryItems( } void InProcessImporterBridge::SetKeywords( - const std::vector<importer::URLKeywordInfo>& url_keywords, + const std::vector<importer::SearchEngineInfo>& search_engines, bool unique_on_host_and_path) { ScopedVector<TemplateURL> owned_template_urls; - for (size_t i = 0; i < url_keywords.size(); ++i) { - owned_template_urls.push_back( - CreateTemplateURL(url_keywords[i].display_name, - url_keywords[i].keyword, - url_keywords[i].url)); + for (const auto& search_engine : search_engines) { + TemplateURL* owned_template_url = + CreateTemplateURL(search_engine.url, + search_engine.keyword, + search_engine.display_name); + if (owned_template_url) + owned_template_urls.push_back(owned_template_url); } BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(&ProfileWriter::AddKeywords, writer_, diff --git a/chrome/browser/importer/in_process_importer_bridge.h b/chrome/browser/importer/in_process_importer_bridge.h index b64845f..9761214 100644 --- a/chrome/browser/importer/in_process_importer_bridge.h +++ b/chrome/browser/importer/in_process_importer_bridge.h @@ -25,7 +25,7 @@ namespace importer { struct ImporterIE7PasswordInfo; #endif struct ImporterURlRow; -struct URLKeywordInfo; +struct SearchEngineInfo; } class InProcessImporterBridge : public ImporterBridge { @@ -49,8 +49,9 @@ class InProcessImporterBridge : public ImporterBridge { void SetHistoryItems(const std::vector<ImporterURLRow>& rows, importer::VisitSource visit_source) override; - void SetKeywords(const std::vector<importer::URLKeywordInfo>& url_keywords, - bool unique_on_host_and_path) override; + void SetKeywords( + const std::vector<importer::SearchEngineInfo>& search_engines, + bool unique_on_host_and_path) override; void SetFirefoxSearchEnginesXMLData( const std::vector<std::string>& search_engine_data) override; diff --git a/chrome/chrome_utility.gypi b/chrome/chrome_utility.gypi index 55c691e..eb9e7c6 100644 --- a/chrome/chrome_utility.gypi +++ b/chrome/chrome_utility.gypi @@ -101,6 +101,7 @@ 'dependencies': [ '../base/base.gyp:base', '../components/components_strings.gyp:components_strings', + '../components/components.gyp:search_engines', '../components/components.gyp:url_fixer', '../content/content.gyp:content_common', '../content/content.gyp:content_utility', diff --git a/chrome/common/importer/importer_bridge.h b/chrome/common/importer/importer_bridge.h index 3e6dbde..880f03d 100644 --- a/chrome/common/importer/importer_bridge.h +++ b/chrome/common/importer/importer_bridge.h @@ -29,7 +29,7 @@ namespace importer { struct ImporterIE7PasswordInfo; #endif struct ImporterURlRow; -struct URLKeywordInfo; +struct SearchEngineInfo; } class ImporterBridge : public base::RefCountedThreadSafe<ImporterBridge> { @@ -54,7 +54,7 @@ class ImporterBridge : public base::RefCountedThreadSafe<ImporterBridge> { importer::VisitSource visit_source) = 0; virtual void SetKeywords( - const std::vector<importer::URLKeywordInfo>& url_keywords, + const std::vector<importer::SearchEngineInfo>& search_engines, bool unique_on_host_and_path) = 0; // The search_engine_data vector contains XML data retrieved from the Firefox diff --git a/chrome/common/importer/importer_data_types.h b/chrome/common/importer/importer_data_types.h index d9594e7..98ecc9b 100644 --- a/chrome/common/importer/importer_data_types.h +++ b/chrome/common/importer/importer_data_types.h @@ -47,9 +47,11 @@ struct SourceProfile { std::string locale; }; -// Contains information needed for importing bookmarks/search engine urls, etc. -struct URLKeywordInfo { - GURL url; +// Contains information needed for importing search engine urls. +struct SearchEngineInfo { + // |url| is a string instead of a GURL since it may not actually be a valid + // GURL directly (e.g. for "http://%s.com"). + base::string16 url; base::string16 keyword; base::string16 display_name; }; diff --git a/chrome/common/importer/profile_import_process_messages.h b/chrome/common/importer/profile_import_process_messages.h index 82daf5b..25e4851 100644 --- a/chrome/common/importer/profile_import_process_messages.h +++ b/chrome/common/importer/profile_import_process_messages.h @@ -87,7 +87,7 @@ IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyPasswordFormReady, autofill::PasswordForm) IPC_MESSAGE_CONTROL2(ProfileImportProcessHostMsg_NotifyKeywordsReady, - std::vector<importer::URLKeywordInfo>, // url_keywords + std::vector<importer::SearchEngineInfo>, // search_engines bool /* unique on host and path */) IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyFirefoxSearchEngData, diff --git a/chrome/common/importer/profile_import_process_param_traits_macros.h b/chrome/common/importer/profile_import_process_param_traits_macros.h index d42522f..45a8ad9 100644 --- a/chrome/common/importer/profile_import_process_param_traits_macros.h +++ b/chrome/common/importer/profile_import_process_param_traits_macros.h @@ -62,7 +62,7 @@ IPC_STRUCT_TRAITS_BEGIN(ImportedFaviconUsage) IPC_STRUCT_TRAITS_MEMBER(urls) IPC_STRUCT_TRAITS_END() -IPC_STRUCT_TRAITS_BEGIN(importer::URLKeywordInfo) +IPC_STRUCT_TRAITS_BEGIN(importer::SearchEngineInfo) IPC_STRUCT_TRAITS_MEMBER(url) IPC_STRUCT_TRAITS_MEMBER(keyword) IPC_STRUCT_TRAITS_MEMBER(display_name) diff --git a/chrome/test/data/bookmark_html_reader/firefox_bookmark_keyword.html b/chrome/test/data/bookmark_html_reader/firefox_bookmark_keyword.html new file mode 100644 index 0000000..b6fdc3a --- /dev/null +++ b/chrome/test/data/bookmark_html_reader/firefox_bookmark_keyword.html @@ -0,0 +1,9 @@ +<!DOCTYPE NETSCAPE-Bookmark-file-1> +<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=UTF-8"> +<TITLE>Bookmarks Keyword</TITLE> +<H1 LAST_MODIFIED="1281990997">Bookmarks</H1> +<DL><p> + <DT><A HREF="http://example.%s.com/" LAST_MODIFIED="1281990997" SHORTCUTURL="keyword" >Bookmark Keyword</A> + <DD> + <DT><A HREF="http://example.com/?q=%s" LAST_MODIFIED="1413038311" SHORTCUTURL="keyword">BookmarkName</A> + </DL><p> diff --git a/chrome/test/data/firefox35_profile/places.sqlite b/chrome/test/data/firefox35_profile/places.sqlite Binary files differindex 9540a3e..0f1284a 100644 --- a/chrome/test/data/firefox35_profile/places.sqlite +++ b/chrome/test/data/firefox35_profile/places.sqlite diff --git a/chrome/test/data/firefox3_profile/places.sqlite b/chrome/test/data/firefox3_profile/places.sqlite Binary files differindex 9540a3e..8b30d8e8 100644 --- a/chrome/test/data/firefox3_profile/places.sqlite +++ b/chrome/test/data/firefox3_profile/places.sqlite diff --git a/chrome/test/data/firefox_profile/places.sqlite b/chrome/test/data/firefox_profile/places.sqlite Binary files differindex 9540a3e..d1e23f0 100644 --- a/chrome/test/data/firefox_profile/places.sqlite +++ b/chrome/test/data/firefox_profile/places.sqlite diff --git a/chrome/utility/BUILD.gn b/chrome/utility/BUILD.gn index 7b8e1d0..fed82f8 100644 --- a/chrome/utility/BUILD.gn +++ b/chrome/utility/BUILD.gn @@ -17,6 +17,7 @@ static_library("utility") { public_deps = [] deps = [ "//base", + "//components/search_engines", "//components/strings", "//components/url_fixer", "//content/public/common", diff --git a/chrome/utility/importer/DEPS b/chrome/utility/importer/DEPS index 29165e3..2559836 100644 --- a/chrome/utility/importer/DEPS +++ b/chrome/utility/importer/DEPS @@ -1,4 +1,5 @@ include_rules = [ "+components/autofill/core/common", # For PasswordForm. + "+components/search_engines", "+components/strings/grit", ] diff --git a/chrome/utility/importer/bookmark_html_reader.cc b/chrome/utility/importer/bookmark_html_reader.cc index e11dc1d..8ee67f1 100644 --- a/chrome/utility/importer/bookmark_html_reader.cc +++ b/chrome/utility/importer/bookmark_html_reader.cc @@ -10,10 +10,13 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" +#include "base/strings/utf_string_conversions.h" #include "base/time/time.h" #include "chrome/common/importer/imported_bookmark_entry.h" #include "chrome/common/importer/imported_favicon_usage.h" #include "chrome/utility/importer/favicon_reencode.h" +#include "components/search_engines/search_terms_data.h" +#include "components/search_engines/template_url.h" #include "net/base/data_url.h" #include "net/base/escape.h" #include "url/gurl.h" @@ -91,6 +94,7 @@ void ImportBookmarksFile( const base::Callback<bool(const GURL&)>& valid_url_callback, const base::FilePath& file_path, std::vector<ImportedBookmarkEntry>* bookmarks, + std::vector<importer::SearchEngineInfo>* search_engines, std::vector<ImportedFaviconUsage>* favicons) { std::string content; base::ReadFileToString(file_path, &content); @@ -150,6 +154,20 @@ void ImportBookmarksFile( &add_date, &post_data) || internal::ParseMinimumBookmarkFromLine(line, charset, &title, &url); + // If bookmark contains a valid replaceable url and a keyword then import + // it as search engine. + std::string search_engine_url; + if (is_bookmark && post_data.empty() && + CanImportURLAsSearchEngine(url, &search_engine_url) && + !shortcut.empty()) { + importer::SearchEngineInfo search_engine_info; + search_engine_info.url.assign(base::UTF8ToUTF16(search_engine_url)); + search_engine_info.keyword = shortcut; + search_engine_info.display_name = title; + search_engines->push_back(search_engine_info); + continue; + } + if (is_bookmark) last_folder_is_empty = false; @@ -238,6 +256,26 @@ void ImportBookmarksFile( } } +bool CanImportURLAsSearchEngine(const GURL& url, + std::string* search_engine_url) { + std::string url_spec = url.possibly_invalid_spec(); + + if (url_spec.empty()) + return false; + + url_spec = net::UnescapeURLComponent(url_spec, + net::UnescapeRule::URL_SPECIAL_CHARS); + + // Replace replacement terms ("%s") in |url_spec| with {searchTerms}. + url_spec = + TemplateURLRef::DisplayURLToURLRef(base::UTF8ToUTF16(url_spec)); + + TemplateURLData data; + data.SetURL(url_spec); + *search_engine_url = url_spec; + return TemplateURL(data).SupportsReplacement(SearchTermsData()); +} + namespace internal { bool ParseCharsetFromLine(const std::string& line, std::string* charset) { diff --git a/chrome/utility/importer/bookmark_html_reader.h b/chrome/utility/importer/bookmark_html_reader.h index 274c9a2..e324634 100644 --- a/chrome/utility/importer/bookmark_html_reader.h +++ b/chrome/utility/importer/bookmark_html_reader.h @@ -10,6 +10,7 @@ #include "base/callback_forward.h" #include "base/strings/string16.h" +#include "chrome/common/importer/importer_data_types.h" class GURL; struct ImportedBookmarkEntry; @@ -37,6 +38,9 @@ namespace bookmark_html_reader { // |bookmarks| is a pointer to a vector, which is filled with the imported // bookmarks. It may not be NULL. // +// |search_engines| is a pointer to a vector, which is filled with the imported +// search engines. +// // |favicons| is a pointer to a vector, which is filled with the favicons of // imported bookmarks. It may be NULL, in which case favicons are not imported. void ImportBookmarksFile( @@ -44,8 +48,15 @@ void ImportBookmarksFile( const base::Callback<bool(const GURL&)>& valid_url_callback, const base::FilePath& file_path, std::vector<ImportedBookmarkEntry>* bookmarks, + std::vector<importer::SearchEngineInfo>* search_engines, std::vector<ImportedFaviconUsage>* favicons); +// Returns true if |url| should be imported as a search engine, i.e. because it +// has replacement terms. Chrome treats such bookmarks as search engines rather +// than true bookmarks. +bool CanImportURLAsSearchEngine(const GURL& url, + std::string* search_engine_url); + namespace internal { // The file format that BookmarkHTMLReader parses starts with a heading diff --git a/chrome/utility/importer/bookmark_html_reader_unittest.cc b/chrome/utility/importer/bookmark_html_reader_unittest.cc index fd68f54..ac5fa86 100644 --- a/chrome/utility/importer/bookmark_html_reader_unittest.cc +++ b/chrome/utility/importer/bookmark_html_reader_unittest.cc @@ -137,6 +137,35 @@ TEST(BookmarkHTMLReaderTest, ParseTests) { EXPECT_EQ("http://www.google.com/", url.spec()); } +TEST(BookmarkHTMLReaderTest, CanImportURLAsSearchEngineTest) { + struct TestCase { + const std::string url; + const bool can_be_imported_as_search_engine; + } test_cases[] = { + { "http://www.example.%s.com", true }, + { "http://www.example.%S.com", true }, + { "http://www.example.%x.com", false }, + { "http://www.example.com", false }, + { "http://%s.example.com", true }, + { "http://www.example.%s.test.%s.com", true }, + { "http://www.test&test.%s.com", true }, + { "http://www.example.com?q=%s&foo=bar", true }, + { "http://www.example.com/%s/?q=%s&foo=bar", true }, + { "http//google.com", false }, + { "path", false }, + { "http:/path/%s/", true }, + { "path", false }, + { "", false }, + }; + + std::string search_engine_url; + for (size_t i = 0; i < arraysize(test_cases); ++i) { + EXPECT_EQ(test_cases[i].can_be_imported_as_search_engine, + CanImportURLAsSearchEngine(GURL(test_cases[i].url), + &search_engine_url)); + } +} + namespace { class BookmarkHTMLReaderTestWithData : public testing::Test { @@ -152,6 +181,10 @@ class BookmarkHTMLReaderTestWithData : public testing::Test { void ExpectFirstFirefox23Bookmark(const ImportedBookmarkEntry& entry); void ExpectSecondFirefox23Bookmark(const ImportedBookmarkEntry& entry); void ExpectThirdFirefox23Bookmark(const ImportedBookmarkEntry& entry); + void ExpectFirstFirefoxBookmarkWithKeyword( + const importer::SearchEngineInfo& info); + void ExpectSecondFirefoxBookmarkWithKeyword( + const importer::SearchEngineInfo& info); base::FilePath test_data_path_; }; @@ -236,6 +269,20 @@ void BookmarkHTMLReaderTestWithData::ExpectThirdFirefox23Bookmark( EXPECT_EQ("http://code.google.com/p/chromium/codesearch", entry.url.spec()); } +void BookmarkHTMLReaderTestWithData::ExpectFirstFirefoxBookmarkWithKeyword( + const importer::SearchEngineInfo& info) { + EXPECT_EQ(ASCIIToUTF16("http://example.{searchTerms}.com/"), info.url); + EXPECT_EQ(ASCIIToUTF16("keyword"), info.keyword); + EXPECT_EQ(ASCIIToUTF16("Bookmark Keyword"), info.display_name); +} + +void BookmarkHTMLReaderTestWithData::ExpectSecondFirefoxBookmarkWithKeyword( + const importer::SearchEngineInfo& info) { + EXPECT_EQ(ASCIIToUTF16("http://example.com/?q={searchTerms}"), info.url); + EXPECT_EQ(ASCIIToUTF16("keyword"), info.keyword); + EXPECT_EQ(ASCIIToUTF16("BookmarkName"), info.display_name); +} + } // namespace TEST_F(BookmarkHTMLReaderTestWithData, Firefox2BookmarkFileImport) { @@ -244,7 +291,7 @@ TEST_F(BookmarkHTMLReaderTestWithData, Firefox2BookmarkFileImport) { std::vector<ImportedBookmarkEntry> bookmarks; ImportBookmarksFile(base::Callback<bool(void)>(), base::Callback<bool(const GURL&)>(), - path, &bookmarks, NULL); + path, &bookmarks, NULL, NULL); ASSERT_EQ(3U, bookmarks.size()); ExpectFirstFirefox2Bookmark(bookmarks[0]); @@ -258,7 +305,7 @@ TEST_F(BookmarkHTMLReaderTestWithData, BookmarkFileWithHrTagImport) { std::vector<ImportedBookmarkEntry> bookmarks; ImportBookmarksFile(base::Callback<bool(void)>(), base::Callback<bool(const GURL&)>(), - path, &bookmarks, NULL); + path, &bookmarks, NULL, NULL); ASSERT_EQ(3U, bookmarks.size()); ExpectFirstFirefox23Bookmark(bookmarks[0]); @@ -272,13 +319,27 @@ TEST_F(BookmarkHTMLReaderTestWithData, EpiphanyBookmarkFileImport) { std::vector<ImportedBookmarkEntry> bookmarks; ImportBookmarksFile(base::Callback<bool(void)>(), base::Callback<bool(const GURL&)>(), - path, &bookmarks, NULL); + path, &bookmarks, NULL, NULL); ASSERT_EQ(2U, bookmarks.size()); ExpectFirstEpiphanyBookmark(bookmarks[0]); ExpectSecondEpiphanyBookmark(bookmarks[1]); } +TEST_F(BookmarkHTMLReaderTestWithData, FirefoxBookmarkFileWithKeywordImport) { + base::FilePath path = test_data_path_.AppendASCII( + "firefox_bookmark_keyword.html"); + + std::vector<importer::SearchEngineInfo> search_engines; + ImportBookmarksFile(base::Callback<bool(void)>(), + base::Callback<bool(const GURL&)>(), + path, NULL, &search_engines, NULL); + + ASSERT_EQ(2U, search_engines.size()); + ExpectFirstFirefoxBookmarkWithKeyword(search_engines[0]); + ExpectSecondFirefoxBookmarkWithKeyword(search_engines[1]); +} + namespace { class CancelAfterFifteenCalls { @@ -301,7 +362,7 @@ TEST_F(BookmarkHTMLReaderTestWithData, CancellationCallback) { ImportBookmarksFile(base::Bind(&CancelAfterFifteenCalls::ShouldCancel, base::Unretained(&cancel_fifteen)), base::Callback<bool(const GURL&)>(), - path, &bookmarks, NULL); + path, &bookmarks, NULL, NULL); // The cancellation callback is checked before each line is read, so fifteen // lines are imported. The first fifteen lines of firefox2.html include only @@ -326,7 +387,7 @@ TEST_F(BookmarkHTMLReaderTestWithData, ValidURLCallback) { std::vector<ImportedBookmarkEntry> bookmarks; ImportBookmarksFile(base::Callback<bool(void)>(), base::Bind(&IsURLValid), - path, &bookmarks, NULL); + path, &bookmarks, NULL, NULL); ASSERT_EQ(2U, bookmarks.size()); ExpectFirstFirefox2Bookmark(bookmarks[0]); diff --git a/chrome/utility/importer/bookmarks_file_importer.cc b/chrome/utility/importer/bookmarks_file_importer.cc index 387257d..502c73d 100644 --- a/chrome/utility/importer/bookmarks_file_importer.cc +++ b/chrome/utility/importer/bookmarks_file_importer.cc @@ -90,6 +90,7 @@ void BookmarksFileImporter::StartImport( bridge->NotifyItemStarted(importer::FAVORITES); std::vector<ImportedBookmarkEntry> bookmarks; + std::vector<importer::SearchEngineInfo> search_engines; std::vector<ImportedFaviconUsage> favicons; bookmark_html_reader::ImportBookmarksFile( @@ -97,6 +98,7 @@ void BookmarksFileImporter::StartImport( base::Bind(internal::CanImportURL), source_profile.source_path, &bookmarks, + &search_engines, &favicons); if (!bookmarks.empty() && !cancelled()) { @@ -104,6 +106,8 @@ void BookmarksFileImporter::StartImport( bridge->GetLocalizedString(IDS_BOOKMARK_GROUP); bridge->AddBookmarks(bookmarks, first_folder_name); } + if (!search_engines.empty()) + bridge->SetKeywords(search_engines, false); if (!favicons.empty()) bridge->SetFavicons(favicons); diff --git a/chrome/utility/importer/external_process_importer_bridge.cc b/chrome/utility/importer/external_process_importer_bridge.cc index 0ea6971..eef5685 100644 --- a/chrome/utility/importer/external_process_importer_bridge.cc +++ b/chrome/utility/importer/external_process_importer_bridge.cc @@ -126,10 +126,10 @@ void ExternalProcessImporterBridge::SetHistoryItems( } void ExternalProcessImporterBridge::SetKeywords( - const std::vector<importer::URLKeywordInfo>& url_keywords, + const std::vector<importer::SearchEngineInfo>& search_engines, bool unique_on_host_and_path) { Send(new ProfileImportProcessHostMsg_NotifyKeywordsReady( - url_keywords, unique_on_host_and_path)); + search_engines, unique_on_host_and_path)); } void ExternalProcessImporterBridge::SetFirefoxSearchEnginesXMLData( diff --git a/chrome/utility/importer/external_process_importer_bridge.h b/chrome/utility/importer/external_process_importer_bridge.h index f20b285..2647302 100644 --- a/chrome/utility/importer/external_process_importer_bridge.h +++ b/chrome/utility/importer/external_process_importer_bridge.h @@ -26,7 +26,7 @@ namespace importer { struct ImporterIE7PasswordInfo; #endif struct ImporterURLRow; -struct URLKeywordInfo; +struct SearchEngineInfo; } namespace IPC { @@ -62,8 +62,9 @@ class ExternalProcessImporterBridge : public ImporterBridge { void SetHistoryItems(const std::vector<ImporterURLRow>& rows, importer::VisitSource visit_source) override; - void SetKeywords(const std::vector<importer::URLKeywordInfo>& url_keywords, - bool unique_on_host_and_path) override; + void SetKeywords( + const std::vector<importer::SearchEngineInfo>& search_engines, + bool unique_on_host_and_path) override; void SetFirefoxSearchEnginesXMLData( const std::vector<std::string>& seach_engine_data) override; diff --git a/chrome/utility/importer/firefox_importer.cc b/chrome/utility/importer/firefox_importer.cc index 728f51d..771aaf9 100644 --- a/chrome/utility/importer/firefox_importer.cc +++ b/chrome/utility/importer/firefox_importer.cc @@ -50,10 +50,12 @@ void LoadDefaultBookmarks(const base::FilePath& app_path, urls->clear(); std::vector<ImportedBookmarkEntry> bookmarks; + std::vector<importer::SearchEngineInfo> search_engines; bookmark_html_reader::ImportBookmarksFile(base::Callback<bool(void)>(), base::Callback<bool(const GURL&)>(), file, &bookmarks, + &search_engines, NULL); for (size_t i = 0; i < bookmarks.size(); ++i) urls->insert(bookmarks[i].url); @@ -225,7 +227,7 @@ void FirefoxImporter::ImportBookmarks() { GetWholeBookmarkFolder(&db, &list, i, NULL); std::vector<ImportedBookmarkEntry> bookmarks; - std::vector<importer::URLKeywordInfo> url_keywords; + std::vector<importer::SearchEngineInfo> search_engines; FaviconMap favicon_map; // TODO(jcampan): http://b/issue?id=1196285 we do not support POST based @@ -247,81 +249,87 @@ void FirefoxImporter::ImportBookmarks() { for (size_t i = 0; i < list.size(); ++i) { BookmarkItem* item = list[i]; - if (item->type == TYPE_FOLDER) { - // Folders are added implicitly on adding children, so we only explicitly - // add empty folders. - if (!item->empty_folder) - continue; - } else if (item->type == TYPE_BOOKMARK) { - // Import only valid bookmarks - if (!CanImportURL(item->url)) - continue; - } else { + // Folders are added implicitly on adding children, so we only explicitly + // add empty folders. + if (item->type != TYPE_BOOKMARK && + ((item->type != TYPE_FOLDER) || !item->empty_folder)) continue; - } - // Skip the default bookmarks and unwanted URLs. - if (default_urls.find(item->url) != default_urls.end() || - post_keyword_ids.find(item->id) != post_keyword_ids.end()) - continue; + if (CanImportURL(item->url)) { + // Skip the default bookmarks and unwanted URLs. + if (default_urls.find(item->url) != default_urls.end() || + post_keyword_ids.find(item->id) != post_keyword_ids.end()) + continue; - // Find the bookmark path by tracing their links to parent folders. - std::vector<base::string16> path; - BookmarkItem* child = item; - bool found_path = false; - bool is_in_toolbar = false; - while (child->parent >= 0) { - BookmarkItem* parent = list[child->parent]; - if (livemark_id.find(parent->id) != livemark_id.end()) { - // Don't import live bookmarks. - break; - } + // Find the bookmark path by tracing their links to parent folders. + std::vector<base::string16> path; + BookmarkItem* child = item; + bool found_path = false; + bool is_in_toolbar = false; + while (child->parent >= 0) { + BookmarkItem* parent = list[child->parent]; + if (livemark_id.find(parent->id) != livemark_id.end()) { + // Don't import live bookmarks. + break; + } - if (parent->id != menu_folder_id) { - // To avoid excessive nesting, omit the name for the bookmarks menu - // folder. - path.insert(path.begin(), parent->title); - } + if (parent->id != menu_folder_id) { + // To avoid excessive nesting, omit the name for the bookmarks menu + // folder. + path.insert(path.begin(), parent->title); + } - if (parent->id == toolbar_folder_id) - is_in_toolbar = true; + if (parent->id == toolbar_folder_id) + is_in_toolbar = true; - if (parent->id == toolbar_folder_id || - parent->id == menu_folder_id || - parent->id == unsorted_folder_id) { - // We've reached a root node, hooray! - found_path = true; - break; - } + if (parent->id == toolbar_folder_id || + parent->id == menu_folder_id || + parent->id == unsorted_folder_id) { + // We've reached a root node, hooray! + found_path = true; + break; + } - child = parent; - } + child = parent; + } - if (!found_path) - continue; + if (!found_path) + continue; - ImportedBookmarkEntry entry; - entry.creation_time = item->date_added; - entry.title = item->title; - entry.url = item->url; - entry.path = path; - entry.in_toolbar = is_in_toolbar; - entry.is_folder = item->type == TYPE_FOLDER; + ImportedBookmarkEntry entry; + entry.creation_time = item->date_added; + entry.title = item->title; + entry.url = item->url; + entry.path = path; + entry.in_toolbar = is_in_toolbar; + entry.is_folder = item->type == TYPE_FOLDER; - bookmarks.push_back(entry); + bookmarks.push_back(entry); + } if (item->type == TYPE_BOOKMARK) { if (item->favicon) favicon_map[item->favicon].insert(item->url); - // This bookmark has a keyword, we should import it. - if (!item->keyword.empty() && item->url.is_valid()) { - importer::URLKeywordInfo url_keyword_info; - url_keyword_info.url = item->url; - url_keyword_info.keyword.assign(base::UTF8ToUTF16(item->keyword)); - url_keyword_info.display_name = item->title; - url_keywords.push_back(url_keyword_info); - } + // Import this bookmark as a search engine if it has a keyword and its URL + // is usable as a search engine URL. (Even if the URL doesn't allow + // substitution, importing as a "search engine" allows users to trigger + // the bookmark by entering its keyword in the omnibox.) + if (item->keyword.empty()) + continue; + importer::SearchEngineInfo search_engine_info; + std::string search_engine_url; + if (item->url.is_valid()) + search_engine_info.url = base::UTF8ToUTF16(item->url.spec()); + else if (bookmark_html_reader::CanImportURLAsSearchEngine( + item->url, + &search_engine_url)) + search_engine_info.url = base::UTF8ToUTF16(search_engine_url); + else + continue; + search_engine_info.keyword = base::UTF8ToUTF16(item->keyword); + search_engine_info.display_name = item->title; + search_engines.push_back(search_engine_info); } } @@ -333,8 +341,8 @@ void FirefoxImporter::ImportBookmarks() { bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP_FROM_FIREFOX); bridge_->AddBookmarks(bookmarks, first_folder_name); } - if (!url_keywords.empty() && !cancelled()) { - bridge_->SetKeywords(url_keywords, false); + if (!search_engines.empty() && !cancelled()) { + bridge_->SetKeywords(search_engines, false); } if (!favicon_map.empty() && !cancelled()) { std::vector<ImportedFaviconUsage> favicons; diff --git a/chrome/utility/importer/ie_importer_win.cc b/chrome/utility/importer/ie_importer_win.cc index 3109576..95ab6e7 100644 --- a/chrome/utility/importer/ie_importer_win.cc +++ b/chrome/utility/importer/ie_importer_win.cc @@ -724,15 +724,15 @@ void IEImporter::ImportSearchEngines() { } } // ProfileWriter::AddKeywords() requires a vector and we have a map. - std::vector<importer::URLKeywordInfo> url_keywords; + std::vector<importer::SearchEngineInfo> search_engines; for (SearchEnginesMap::iterator i = search_engines_map.begin(); i != search_engines_map.end(); ++i) { - importer::URLKeywordInfo url_keyword_info; - url_keyword_info.url = GURL(i->first); - url_keyword_info.display_name = i->second; - url_keywords.push_back(url_keyword_info); + importer::SearchEngineInfo search_engine_info; + search_engine_info.url = base::UTF8ToUTF16(i->first); + search_engine_info.display_name = i->second; + search_engines.push_back(search_engine_info); } - bridge_->SetKeywords(url_keywords, true); + bridge_->SetKeywords(search_engines, true); } void IEImporter::ImportHomepage() { diff --git a/components/search_engines/BUILD.gn b/components/search_engines/BUILD.gn index 089ebb4..a1cb181 100644 --- a/components/search_engines/BUILD.gn +++ b/components/search_engines/BUILD.gn @@ -41,6 +41,10 @@ static_library("search_engines") { "util.h", ] + public_deps = [ + "//components/metrics/proto", + ] + deps = [ ":prepopulated_engines", "//base", |