Importing certain bookmarks from firefox and HTML file as search engines.

chrome/utility/importer/bookmark_html_reader.h

 // Copyright 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.
 
 #ifndef CHROME_UTILITY_IMPORTER_BOOKMARK_HTML_READER_H_
 #define CHROME_UTILITY_IMPORTER_BOOKMARK_HTML_READER_H_
 
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/strings/string16.h"
+#include "chrome/common/importer/importer_data_types.h"
 
 class GURL;
 struct ImportedBookmarkEntry;
 struct ImportedFaviconUsage;
 
 namespace base {
 class FilePath;
 class Time;
 }
 
 namespace bookmark_html_reader {
 
 // Imports the bookmarks from the specified file.
 //
 // |cancellation_callback| is polled to query if the import should be cancelled;
 // if it returns |true| at any time the import will be cancelled. If
 // |cancellation_callback| is a null callback the import will run to completion.
 //
 // |valid_url_callback| is called to determine if a specified URL is valid for
 // import; it returns |true| if it is. If |valid_url_callback| is a null
 // callback, all URLs are considered to be valid.
 //
 // |file_path| is the path of the file on disk to import.
 //
 // |bookmarks| is a pointer to a vector, which is filled with the imported
 // bookmarks. It may not be NULL.
 //
+// |search_engine_keywords| is a pointer to a vector, which is filled with the
+// imported keywords that will be imported as search engines.

[Peter Kasting, 2014/10/07 23:17:38]

Nit: "filled with the imported search engines".  (I don't know what a "keyword
that will be imported as a search engine" is supposed to mean.)

[Tapu Ghose, 2014/10/12 00:58:21]

Rephrased as you suggested.

However, I meant to say "filled with the imported bookmark keywords that will be
imported as search engine". Here, we are trying to import bookmarks as search
engines if the corresponding bookmark contains any keyword.

+//
 // |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(
     const base::Callback<bool(void)>& cancellation_callback,
     const base::Callback<bool(const GURL&)>& valid_url_callback,
     const base::FilePath& file_path,
     std::vector<ImportedBookmarkEntry>* bookmarks,
+    std::vector<importer::URLKeywordInfo>* search_engine_keywords,
     std::vector<ImportedFaviconUsage>* favicons);
 
+// Checks if a bookmark that contains invalid |url| can be imported as search

[Peter Kasting, 2014/10/07 23:17:38]

Wait, so |url| is known to be invalid when this function is called?

[Tapu Ghose, 2014/10/12 00:58:20]

After looking at the flow in chrome/utility/importer/firefox_importer.cc, I
thought that this particular function will only be called if a bookmark contains
invalid |url|. According to the flow, if |url| contains replacement term in host
(e.g., http://example.%s.com) then GURL treats it as invalid. However, it can be
imported as search engine. And from firefox_importer.cc this function is only
called when url.is_valid() returns false.

But when I was addressing one of Ilya's comments for
chrome/utility/importer/bookmark_html_reader.cc, I realized that this function
should also be called for valid url (e.g., http://example.com?id=%s).

In short, |url| can be either valid or invalid when this function is called.

+// engine. Scans |url| to check if there is any replacement term, %s. If yes
+// then substitutes the replacement term(s) with an arbitrary value. Returns
+// true if the resulting url is a valid one.

[Peter Kasting, 2014/10/07 23:17:38]

This is describing more what the implementation does than what the caller cares
about from an API perspective.

[Tapu Ghose, 2014/10/12 00:58:21]

Acknowledged.

+// |search_engine_info| holds search engine data which is composed of |title|,
+// |keyword| and decoded url derived from |url|.

[Peter Kasting, 2014/10/07 23:17:38]

This is mostly a description of the structure definition of URLKeywordInfo,
which doesn't belong here.

[Tapu Ghose, 2014/10/12 00:58:20]

Acknowledged.

+bool CanImportURLAsSearchEngine(const GURL& url,
+                                const base::string16& keyword,
+                                const base::string16& title,
+                                importer::URLKeywordInfo* search_engine_info);

[Ilya Sherman, 2014/10/07 22:08:22]

nit: Please leave a blank line after this one.

[Tapu Ghose, 2014/10/12 00:58:20]

Done.

 namespace internal {
 
 // The file format that BookmarkHTMLReader parses starts with a heading
 // tag, which contains its title. All bookmarks and sub-folders follow,
 // bracketed by a <DL> tag:
 //   <DT><H3 PERSONAL_TOOLBAR_FOLDER="true" ...>title</H3>
 //   <DL><p>
 //      ... container ...
 //   </DL><p>
 // And a bookmark is presented by a <A> tag:
@@ skipping 29 matching lines @@
 bool ParseMinimumBookmarkFromLine(const std::string& line,
                                   const std::string& charset,
                                   base::string16* title,
                                   GURL* url);
 
 }  // namespace internal
 
 }  // namespace bookmark_html_reader
 
 #endif  // CHROME_UTILITY_IMPORTER_BOOKMARK_HTML_READER_H_