Extract BookmarksFileImporter from Firefox2Importer.

chrome/browser/importer/bookmarks_file_importer.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// 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 "chrome/browser/importer/firefox2_importer.h"
+#include "chrome/browser/importer/bookmarks_file_importer.h"
 
 #include <string>
-#include <vector>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/i18n/icu_string_conversions.h"
-#include "base/message_loop.h"
-#include "base/path_service.h"
-#include "base/stl_util.h"
 #include "base/string_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
-#include "base/utf_string_conversions.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/importer/firefox_importer_utils.h"
 #include "chrome/browser/importer/importer_bridge.h"
 #include "chrome/browser/importer/importer_util.h"
-#include "chrome/browser/importer/mork_reader.h"
-#include "chrome/browser/importer/nss_decryptor.h"
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/common/time_format.h"
 #include "chrome/common/url_constants.h"
-#include "content/public/common/password_form.h"
 #include "googleurl/src/gurl.h"
 #include "grit/generated_resources.h"
 #include "net/base/data_url.h"
+#include "net/base/escape.h"
 
 namespace {
-const char kItemOpen[] = "<DT><A";
-const char kItemClose[] = "</A>";
-const char kFeedURLAttribute[] = "FEEDURL";
-const char kHrefAttribute[] = "HREF";
-const char kIconAttribute[] = "ICON";
-const char kShortcutURLAttribute[] = "SHORTCUTURL";
-const char kAddDateAttribute[] = "ADD_DATE";
-const char kPostDataAttribute[] = "POST_DATA";
+
+// Fetches the given |attribute| value from the |attribute_list|. Returns true
+// if successful, and |value| will contain the value.
+bool GetAttribute(const std::string& attribute_list,
+                  const std::string& attribute,
+                  std::string* value) {
+  const char kQuote[] = "\"";
+
+  size_t begin = attribute_list.find(attribute + "=" + kQuote);
+  if (begin == std::string::npos)
+    return false;  // Can't find the attribute.
+
+  begin = attribute_list.find(kQuote, begin) + 1;
+
+  size_t end = begin + 1;
+  while (end < attribute_list.size()) {
+    if (attribute_list[end] == '"' &&
+        attribute_list[end - 1] != '\\') {
+      break;
+    }
+    end++;
+  }
+
+  if (end == attribute_list.size())
+    return false;  // The value is not quoted.
+
+  *value = attribute_list.substr(begin, end - begin);
+  return true;
 }
 
-Firefox2Importer::Firefox2Importer() : parsing_bookmarks_html_file_(false) {}
+// Given the URL of a page and a favicon data URL, adds an appropriate record
+// to the given favicon usage vector. Will do nothing if the favicon is not
+// valid.
+void DataURLToFaviconUsage(
+    const GURL& link_url,
+    const GURL& favicon_data,
+    std::vector<history::ImportedFaviconUsage>* favicons) {
+  if (!link_url.is_valid() || !favicon_data.is_valid() ||
+      !favicon_data.SchemeIs(chrome::kDataScheme))

[gab, 2013/05/07 17:47:54]

#include "content/public/common/url_constants.h" for chrome::kDataScheme

+    return;
 
-Firefox2Importer::~Firefox2Importer() {}
+  // Parse the data URL.
+  std::string mime_type, char_set, data;
+  if (!net::DataURL::Parse(favicon_data, &mime_type, &char_set, &data) ||
+      data.empty())
+    return;
 
-void Firefox2Importer::StartImport(
+  history::ImportedFaviconUsage usage;
+  if (!importer::ReencodeFavicon(
+          reinterpret_cast<const unsigned char*>(&data[0]),
+          data.size(), &usage.png_data))
+    return;  // Unable to decode.
+
+  // We need to make up a URL for the favicon. We use a version of the page's
+  // URL so that we can be sure it will not collide.
+  usage.favicon_url = GURL(std::string("made-up-favicon:") + link_url.spec());
+
+  // We only have one URL per favicon for Firefox 2 bookmarks.
+  usage.urls.insert(link_url);
+
+  favicons->push_back(usage);
+}
+
+bool ImporterCancelBridge(Importer* importer) {
+  return importer->cancelled();
+}
+
+}  // namespace
+

[gab, 2013/05/07 17:47:54]

Add a section comment:

////////////////////////////////////////////////////////////////////////////////
// BookmarksFileImporter, public:

ref:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

[tfarina, 2013/05/07 18:02:55]

I wouldn't encourage the use of this kind of comment. Ben seems to use it
though.

Of course this up to Avi.

[Avi (use Gerrit), 2013/05/07 18:29:39]

Most code doesn't use comments like that; not going to do it.

[gab, 2013/05/08 16:24:34]

Ok, I'd seen it around and thought it was useful when reading the CC file, but I
won't insist given that it's not a majority of the code that does this.

+BookmarksFileImporter::BookmarksFileImporter() {}
+
+BookmarksFileImporter::~BookmarksFileImporter() {}
+
+void BookmarksFileImporter::StartImport(
     const importer::SourceProfile& source_profile,
     uint16 items,
     ImporterBridge* bridge) {
-  bridge_ = bridge;
-  source_path_ = source_profile.source_path;
-  app_path_ = source_profile.app_path;
+  // The only thing this importer can import is a bookmarks file, aka
+  // "favorites".
+  DCHECK_EQ(importer::FAVORITES, items);
 
-  parsing_bookmarks_html_file_ =
-      (source_profile.importer_type == importer::TYPE_BOOKMARKS_FILE);
+  base::FilePath source_path = source_profile.source_path;

[gab, 2013/05/07 17:47:54]

can be inline on line 122

+  base::FilePath app_path = source_profile.app_path;

[gab, 2013/05/07 17:47:54]

unused.

 
-  // The order here is important!
-  bridge_->NotifyStarted();
-  if ((items & importer::HOME_PAGE) && !cancelled())
-    ImportHomepage();  // Doesn't have a UI item.
+  bridge->NotifyStarted();
+  bridge->NotifyItemStarted(importer::FAVORITES);
 
-  // Note history should be imported before bookmarks because bookmark import
-  // will also import favicons and we store favicon for a URL only if the URL
-  // exist in history or bookmarks.
-  if ((items & importer::HISTORY) && !cancelled()) {
-    bridge_->NotifyItemStarted(importer::HISTORY);
-    ImportHistory();
-    bridge_->NotifyItemEnded(importer::HISTORY);
+  std::vector<ProfileWriter::BookmarkEntry> bookmarks;
+  std::vector<history::ImportedFaviconUsage> favicons;
+  base::Callback<bool(void)> cancellation_callback =
+      base::Bind(ImporterCancelBridge, base::Unretained(this));
+
+  ImportBookmarksFile(source_path,
+                      &cancellation_callback,
+                      &bookmarks,
+                      &favicons);
+
+  // Write data into profile.

[gab, 2013/05/07 17:47:54]

// Send bookmarks over the bridge.
?

(although the code is pretty explicit and I'd be fine with no comments at all)

+  if (!bookmarks.empty() && !cancelled()) {
+    string16 first_folder_name = bridge->GetLocalizedString(IDS_BOOKMARK_GROUP);
+    bridge->AddBookmarks(bookmarks, first_folder_name);
   }
+  if (!favicons.empty())
+    bridge->SetFavicons(favicons);
 
-  if ((items & importer::FAVORITES) && !cancelled()) {
-    bridge_->NotifyItemStarted(importer::FAVORITES);
-    ImportBookmarks();
-    bridge_->NotifyItemEnded(importer::FAVORITES);
-  }
-  if ((items & importer::SEARCH_ENGINES) && !cancelled()) {
-    bridge_->NotifyItemStarted(importer::SEARCH_ENGINES);
-    ImportSearchEngines();
-    bridge_->NotifyItemEnded(importer::SEARCH_ENGINES);
-  }
-  if ((items & importer::PASSWORDS) && !cancelled()) {
-    bridge_->NotifyItemStarted(importer::PASSWORDS);
-    ImportPasswords();
-    bridge_->NotifyItemEnded(importer::PASSWORDS);
-  }
-  bridge_->NotifyEnded();
+  bridge->NotifyItemEnded(importer::FAVORITES);
+  bridge->NotifyEnded();
 }
 
 // static
-void Firefox2Importer::LoadDefaultBookmarks(const base::FilePath& app_path,
-                                            std::set<GURL> *urls) {
-  base::FilePath file = app_path.AppendASCII("defaults")
-                      .AppendASCII("profile")
-                      .AppendASCII("bookmarks.html");
-
-  urls->clear();
-
-  // Read the whole file.
-  std::string content;
-  file_util::ReadFileToString(file, &content);
-  std::vector<std::string> lines;
-  base::SplitString(content, '\n', &lines);
-
-  std::string charset;
-  for (size_t i = 0; i < lines.size(); ++i) {
-    std::string line;
-    TrimString(lines[i], " ", &line);
-
-    // Get the encoding of the bookmark file.
-    if (ParseCharsetFromLine(line, &charset))
-      continue;
-
-    // Get the bookmark.
-    string16 title;
-    GURL url, favicon;
-    string16 shortcut;
-    base::Time add_date;
-    string16 post_data;
-    if (ParseBookmarkFromLine(line, charset, &title, &url,
-                              &favicon, &shortcut, &add_date,
-                              &post_data))
-      urls->insert(url);
-  }
-}
-
-// static
-TemplateURL* Firefox2Importer::CreateTemplateURL(const string16& title,
-                                                 const string16& keyword,
-                                                 const GURL& url) {
-  // Skip if the keyword or url is invalid.
-  if (keyword.empty() || !url.is_valid())
-    return NULL;
-
-  TemplateURLData data;
-  // We set short name by using the title if it exists.
-  // Otherwise, we use the shortcut.
-  data.short_name = title.empty() ? keyword : title;
-  data.SetKeyword(keyword);
-  data.SetURL(TemplateURLRef::DisplayURLToURLRef(UTF8ToUTF16(url.spec())));
-  return new TemplateURL(NULL, data);
-}
-
-// static
-void Firefox2Importer::ImportBookmarksFile(
+void BookmarksFileImporter::ImportBookmarksFile(
     const base::FilePath& file_path,
-    const std::set<GURL>& default_urls,
-    Importer* importer,
+    base::Callback<bool(void)>* cancellation_callback,
     std::vector<ProfileWriter::BookmarkEntry>* bookmarks,
-    std::vector<TemplateURL*>* template_urls,
     std::vector<history::ImportedFaviconUsage>* favicons) {
   std::string content;
   file_util::ReadFileToString(file_path, &content);
   std::vector<std::string> lines;
   base::SplitString(content, '\n', &lines);
 
   string16 last_folder;
   bool last_folder_on_toolbar = false;
   bool last_folder_is_empty = true;
   bool has_subfolder = false;
   base::Time last_folder_add_date;
   std::vector<string16> path;
   size_t toolbar_folder = 0;

[gab, 2013/05/07 17:47:54]

Took me some time to understand what this variable represented.

How about renaming it to toolbar_folder_index (i.e., it seems to be the index of
the toolbar folder in |path|; perhaps adding a comment saying this here could
help too)?

   std::string charset;
-  for (size_t i = 0; i < lines.size() && (!importer || !importer->cancelled());
+  for (size_t i = 0;
+       i < lines.size() &&
+           (!cancellation_callback || !cancellation_callback->Run());
        ++i) {
     std::string line;
     TrimString(lines[i], " ", &line);
 
     // Get the encoding of the bookmark file.
     if (ParseCharsetFromLine(line, &charset))
       continue;
 
     // Get the folder name.
     if (ParseFolderNameFromLine(line, charset, &last_folder,
@@ skipping 13 matching lines @@
     is_bookmark = ParseBookmarkFromLine(line, charset, &title,
                                         &url, &favicon, &shortcut, &add_date,
                                         &post_data) ||
         ParseMinimumBookmarkFromLine(line, charset, &title, &url);
 
     if (is_bookmark)
       last_folder_is_empty = false;
 
     if (is_bookmark &&
         post_data.empty() &&
-        CanImportURL(GURL(url)) &&
-        default_urls.find(url) == default_urls.end()) {
+        CanImportURL(GURL(url))) {
       if (toolbar_folder > path.size() && !path.empty()) {
         NOTREACHED();  // error in parsing.
         break;
       }
 
       ProfileWriter::BookmarkEntry entry;
       entry.creation_time = add_date;
       entry.url = url;
       entry.title = title;
 
       if (toolbar_folder) {
         // The toolbar folder should be at the top level.
         entry.in_toolbar = true;
         entry.path.assign(path.begin() + toolbar_folder - 1, path.end());
       } else {
         // Add this bookmark to the list of |bookmarks|.
         if (!has_subfolder && !last_folder.empty()) {
           path.push_back(last_folder);
           last_folder.clear();
         }
         entry.path.assign(path.begin(), path.end());
       }
       bookmarks->push_back(entry);
 
       // Save the favicon. DataURLToFaviconUsage will handle the case where
       // there is no favicon.
       if (favicons)
         DataURLToFaviconUsage(url, favicon, favicons);
 
-      if (template_urls) {
-        // If there is a SHORTCUT attribute for this bookmark, we
-        // add it as our keywords.

[gab, 2013/05/07 17:47:54]

Why remove support for keywords?

[Avi (use Gerrit), 2013/05/07 18:29:39]

Because it's a feature of the FF2 importer, not of the subset that is the
bookmarks file importer.

This is intended to be no code change from how things work today. Today, the
bookmarks file importer doesn't know about keywords, and no caller expects it
to. So I'm removing it.

-        TemplateURL* t_url = CreateTemplateURL(title, shortcut, url);
-        if (t_url)
-          template_urls->push_back(t_url);
-      }
-
       continue;
     }
 
     // Bookmarks in sub-folder are encapsulated with <DL> tag.
     if (StartsWithASCII(line, "<DL>", false)) {
       has_subfolder = true;
       if (!last_folder.empty()) {
         path.push_back(last_folder);
         last_folder.clear();
       }
@@ skipping 31 matching lines @@
 
         // Parent folder include current one, so it's not empty.
         last_folder_is_empty = false;
       }
 
       if (toolbar_folder > path.size())
         toolbar_folder = 0;
     }
   }
 }
 

[gab, 2013/05/07 17:47:54]

Add a section comment:

////////////////////////////////////////////////////////////////////////////////
// BookmarksFileImporter, private:

ref:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

-void Firefox2Importer::ImportBookmarks() {
-  // Load the default bookmarks.
-  std::set<GURL> default_urls;
-  if (!parsing_bookmarks_html_file_)
-    LoadDefaultBookmarks(app_path_, &default_urls);
-
-  // Parse the bookmarks.html file.
-  std::vector<ProfileWriter::BookmarkEntry> bookmarks, toolbar_bookmarks;
-  std::vector<TemplateURL*> template_urls;
-  std::vector<history::ImportedFaviconUsage> favicons;
-  base::FilePath file = source_path_;
-  if (!parsing_bookmarks_html_file_)
-    file = file.AppendASCII("bookmarks.html");
-
-  ImportBookmarksFile(file, default_urls, this, &bookmarks, &template_urls,
-                      &favicons);
-
-  // Write data into profile.
-  if (!bookmarks.empty() && !cancelled()) {
-    string16 first_folder_name = bridge_->GetLocalizedString(
-        parsing_bookmarks_html_file_ ? IDS_BOOKMARK_GROUP :
-                                       IDS_BOOKMARK_GROUP_FROM_FIREFOX);
-    bridge_->AddBookmarks(bookmarks, first_folder_name);
-  }
-  if (!parsing_bookmarks_html_file_ && !template_urls.empty() && !cancelled())
-    bridge_->SetKeywords(template_urls, false);
-  else
-    STLDeleteElements(&template_urls);
-  if (!favicons.empty())
-    bridge_->SetFavicons(favicons);
-}
-
-void Firefox2Importer::ImportPasswords() {
-  // Initializes NSS3.
-  NSSDecryptor decryptor;
-  if (!decryptor.Init(source_path_, source_path_) &&
-      !decryptor.Init(app_path_, source_path_)) {
-    return;
-  }
-
-  // Firefox 2 uses signons2.txt to store the pssswords. If it doesn't
-  // exist, we try to find its older version.
-  base::FilePath file = source_path_.AppendASCII("signons2.txt");
-  if (!file_util::PathExists(file)) {
-    file = source_path_.AppendASCII("signons.txt");
-  }
-
-  std::string content;
-  file_util::ReadFileToString(file, &content);
-  std::vector<content::PasswordForm> forms;
-  decryptor.ParseSignons(content, &forms);
-
-  if (!cancelled()) {
-    for (size_t i = 0; i < forms.size(); ++i) {
-      bridge_->SetPasswordForm(forms[i]);
-    }
-  }
-}
-
-void Firefox2Importer::ImportHistory() {
-  base::FilePath file = source_path_.AppendASCII("history.dat");
-  ImportHistoryFromFirefox2(file, bridge_);
-}
-
-void Firefox2Importer::ImportSearchEngines() {
-  std::vector<base::FilePath> files;
-  GetSearchEnginesXMLFiles(&files);
-
-  std::vector<TemplateURL*> search_engines;
-  ParseSearchEnginesFromXMLFiles(files, &search_engines);
-
-  bridge_->SetKeywords(search_engines, true);
-}
-
-void Firefox2Importer::ImportHomepage() {
-  GURL home_page = GetHomepage(source_path_);
-  if (home_page.is_valid() && !IsDefaultHomepage(home_page, app_path_)) {
-    bridge_->AddHomePage(home_page);
-  }
-}
-
-void Firefox2Importer::GetSearchEnginesXMLFiles(
-    std::vector<base::FilePath>* files) {
-  // Search engines are contained in XML files in a searchplugins directory that
-  // can be found in 2 locations:
-  // - Firefox install dir (default search engines)
-  // - the profile dir (user added search engines)
-  base::FilePath dir = app_path_.AppendASCII("searchplugins");
-  FindXMLFilesInDir(dir, files);
-
-  base::FilePath profile_dir = source_path_.AppendASCII("searchplugins");
-  FindXMLFilesInDir(profile_dir, files);
-}
-
 // static
-bool Firefox2Importer::ParseCharsetFromLine(const std::string& line,
-                                            std::string* charset) {
+bool BookmarksFileImporter::ParseCharsetFromLine(const std::string& line,
+                                                 std::string* charset) {
   const char kCharset[] = "charset=";
   if (StartsWithASCII(line, "<META", false) &&
       (line.find("CONTENT=\"") != std::string::npos ||
           line.find("content=\"") != std::string::npos)) {
     size_t begin = line.find(kCharset);
     if (begin == std::string::npos)
       return false;
     begin += std::string(kCharset).size();
     size_t end = line.find_first_of('\"', begin);
     *charset = line.substr(begin, end - begin);
     return true;
   }
   return false;
 }
 
 // static
-bool Firefox2Importer::ParseFolderNameFromLine(const std::string& line,
-                                               const std::string& charset,
-                                               string16* folder_name,
-                                               bool* is_toolbar_folder,
-                                               base::Time* add_date) {
+bool BookmarksFileImporter::ParseFolderNameFromLine(const std::string& line,
+                                                    const std::string& charset,
+                                                    string16* folder_name,
+                                                    bool* is_toolbar_folder,
+                                                    base::Time* add_date) {
   const char kFolderOpen[] = "<DT><H3";
   const char kFolderClose[] = "</H3>";
   const char kToolbarFolderAttribute[] = "PERSONAL_TOOLBAR_FOLDER";
   const char kAddDateAttribute[] = "ADD_DATE";
 
   if (!StartsWithASCII(line, kFolderOpen, true))
     return false;
 
   size_t end = line.find(kFolderClose);
   size_t tag_end = line.rfind('>', end) + 1;
   // If no end tag or start tag is broken, we skip to find the folder name.
   if (end == std::string::npos || tag_end < arraysize(kFolderOpen))
     return false;
 
   base::CodepageToUTF16(line.substr(tag_end, end - tag_end), charset.c_str(),
                         base::OnStringConversionError::SKIP, folder_name);
-  HTMLUnescape(folder_name);
+  *folder_name = net::UnescapeForHTML(*folder_name);
 
   std::string attribute_list = line.substr(arraysize(kFolderOpen),
       tag_end - arraysize(kFolderOpen) - 1);
   std::string value;
 
   // Add date
   if (GetAttribute(attribute_list, kAddDateAttribute, &value)) {
     int64 time;
     base::StringToInt64(value, &time);
     // Upper bound it at 32 bits.
     if (0 < time && time < (1LL << 32))
       *add_date = base::Time::FromTimeT(time);
   }
 
   if (GetAttribute(attribute_list, kToolbarFolderAttribute, &value) &&
       LowerCaseEqualsASCII(value, "true"))
     *is_toolbar_folder = true;
   else
     *is_toolbar_folder = false;
 
   return true;
 }
 
 // static
-bool Firefox2Importer::ParseBookmarkFromLine(const std::string& line,
-                                             const std::string& charset,
-                                             string16* title,
-                                             GURL* url,
-                                             GURL* favicon,
-                                             string16* shortcut,
-                                             base::Time* add_date,
-                                             string16* post_data) {
+bool BookmarksFileImporter::ParseBookmarkFromLine(const std::string& line,
+                                                  const std::string& charset,
+                                                  string16* title,
+                                                  GURL* url,
+                                                  GURL* favicon,
+                                                  string16* shortcut,
+                                                  base::Time* add_date,
+                                                  string16* post_data) {
+  const char kItemOpen[] = "<DT><A";

[gab, 2013/05/07 17:47:54]

optional: The compiler should optimize this by putting const char[] in .rodata
and optimizing the pointer out, but I tend to err on the side of caution and
explicitly make local constants static.

(this article explains why this can be a problem at scale:
http://glandium.org/blog/?p=2361).

[tfarina, 2013/05/07 18:05:52]

There was a thread about this:

https://groups.google.com/a/chromium.org/forum/?hl=en&fromgroups#!searchin/ch...

[Avi (use Gerrit), 2013/05/07 18:29:39]

gab: Does that article refer to declarations _inside_ functions? I'm not seeing
it do so.

tfarina: After reading that thread, my head is spinning. If this is really a
problem then it's a problem across chrome and needs to be addressed at that
level.

[gab, 2013/05/08 16:24:34]

Ah right, it refers to file scope, not local method scope.
So I searched through the codebase with the following regexes:
^((?!namespace).*)\{\n( )*static const char [a-zA-Z]*\[\] = \".*\";
and
^((?!namespace).*)\{\n( )*const char [a-zA-Z]*\[\] = \".*\";

(i.e. anywhere where a const char[] is declared on the line following an open
brace (which is not simply the opening of a namespace) -- all of these seem to
match local scope declarations as desired)

Seems the use of static is less common than cases where it's omitted for local
scope const char arrays:
51 matches across 36 files
vs
213 matches across 81 files (although a lot of these are in test code)

FWIW, I also generalized this research to all POD constants:
^((?!namespace).*)\{\n( )*const [a-z0-9]* k[a-zA-Z]*(\[\])? = .*;
622 matches across 317 files
^((?!namespace).*)\{\n( )*static const [a-z0-9]* k[a-zA-Z]*(\[\])? = .*;
176 matches across 111 files

For inlineable constants though it probably makes sense that they not be static
(e.g., int, bool, etc.) to yield a better chance that the compiler will inline
it.

Arguably, const char[] is a simple pointer underneath  and can probably be
considered an inlineable constant as well (**and thus omitting static here is
probably OK**).

For more complex things like const char[][], it makes sense to use static to
avoid extra initialization though (as I just discussed with Etienne Bergeron and
as demonstrated by Albert Wong and summarized by Ami Fischman @
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/pF7nAmUQ2uw/xnJY6...
)

(I'm currently analyzing this further with the help of Etienne Bergeron, looks
like static indeed helps here, but this is far from being the only instance
where the static keyword is omitted so you can feel free to ignore this for this
CL).

Cheers!

[Avi (use Gerrit), 2013/05/08 18:30:27]

I'd like to take a pass on this for now. It would be nice to handle this on a
cross-Chromium level and to have guidelines established once and for all.

+  const char kItemClose[] = "</A>";
+  const char kFeedURLAttribute[] = "FEEDURL";
+  const char kHrefAttribute[] = "HREF";
+  const char kIconAttribute[] = "ICON";
+  const char kShortcutURLAttribute[] = "SHORTCUTURL";
+  const char kAddDateAttribute[] = "ADD_DATE";
+  const char kPostDataAttribute[] = "POST_DATA";
+
   title->clear();
   *url = GURL();
   *favicon = GURL();
   shortcut->clear();
   post_data->clear();
   *add_date = base::Time();
 
   if (!StartsWithASCII(line, kItemOpen, true))
     return false;
 
   size_t end = line.find(kItemClose);
   size_t tag_end = line.rfind('>', end) + 1;
   if (end == std::string::npos || tag_end < arraysize(kItemOpen))
     return false;  // No end tag or start tag is broken.
 
   std::string attribute_list = line.substr(arraysize(kItemOpen),
       tag_end - arraysize(kItemOpen) - 1);
 
   // We don't import Live Bookmark folders, which is Firefox's RSS reading
   // feature, since the user never necessarily bookmarked them and we don't
   // have this feature to update their contents.
   std::string value;
   if (GetAttribute(attribute_list, kFeedURLAttribute, &value))
     return false;
 
   // Title
   base::CodepageToUTF16(line.substr(tag_end, end - tag_end), charset.c_str(),
                         base::OnStringConversionError::SKIP, title);
-  HTMLUnescape(title);
+  *title = net::UnescapeForHTML(*title);
 
   // URL
   if (GetAttribute(attribute_list, kHrefAttribute, &value)) {
     string16 url16;
     base::CodepageToUTF16(value, charset.c_str(),
                           base::OnStringConversionError::SKIP, &url16);
-    HTMLUnescape(&url16);
+    url16 = net::UnescapeForHTML(url16);
 
     *url = GURL(url16);
   }
 
   // Favicon
   if (GetAttribute(attribute_list, kIconAttribute, &value))
     *favicon = GURL(value);
 
   // Keyword
   if (GetAttribute(attribute_list, kShortcutURLAttribute, &value)) {
     base::CodepageToUTF16(value, charset.c_str(),
                           base::OnStringConversionError::SKIP, shortcut);
-    HTMLUnescape(shortcut);
+    *shortcut = net::UnescapeForHTML(*shortcut);
   }
 
   // Add date
   if (GetAttribute(attribute_list, kAddDateAttribute, &value)) {
     int64 time;
     base::StringToInt64(value, &time);
     // Upper bound it at 32 bits.
     if (0 < time && time < (1LL << 32))
       *add_date = base::Time::FromTimeT(time);
   }
 
   // Post data.
   if (GetAttribute(attribute_list, kPostDataAttribute, &value)) {
     base::CodepageToUTF16(value, charset.c_str(),
                           base::OnStringConversionError::SKIP, post_data);
-    HTMLUnescape(post_data);
+    *post_data = net::UnescapeForHTML(*post_data);
   }
 
   return true;
 }
 
 // static
-bool Firefox2Importer::ParseMinimumBookmarkFromLine(const std::string& line,
-                                                    const std::string& charset,
-                                                    string16* title,
-                                                    GURL* url) {
+bool BookmarksFileImporter::ParseMinimumBookmarkFromLine(
+    const std::string& line,
+    const std::string& charset,
+    string16* title,
+    GURL* url) {
   const char kItemOpen[] = "<DT><A";

[gab, 2013/05/07 17:47:54]

optional: static (see comment above).

   const char kItemClose[] = "</";
   const char kHrefAttributeUpper[] = "HREF";
   const char kHrefAttributeLower[] = "href";
 
   title->clear();
   *url = GURL();
 
   // Case-insensitive check of open tag.
   if (!StartsWithASCII(line, kItemOpen, false))
     return false;
 
   // Find any close tag.
   size_t end = line.find(kItemClose);
   size_t tag_end = line.rfind('>', end) + 1;
   if (end == std::string::npos || tag_end < arraysize(kItemOpen))
     return false;  // No end tag or start tag is broken.
 
   std::string attribute_list = line.substr(arraysize(kItemOpen),
       tag_end - arraysize(kItemOpen) - 1);
 
   // Title
   base::CodepageToUTF16(line.substr(tag_end, end - tag_end), charset.c_str(),
                         base::OnStringConversionError::SKIP, title);
-  HTMLUnescape(title);
+  *title = net::UnescapeForHTML(*title);
 
   // URL
   std::string value;
   if (GetAttribute(attribute_list, kHrefAttributeUpper, &value) ||
       GetAttribute(attribute_list, kHrefAttributeLower, &value)) {
     if (charset.length() != 0) {
       string16 url16;
       base::CodepageToUTF16(value, charset.c_str(),
                             base::OnStringConversionError::SKIP, &url16);
-      HTMLUnescape(&url16);
+      url16 = net::UnescapeForHTML(url16);
 
       *url = GURL(url16);
     } else {
       *url = GURL(value);
     }
   }
 
   return true;
 }
-
-// static
-bool Firefox2Importer::GetAttribute(const std::string& attribute_list,
-                                    const std::string& attribute,
-                                    std::string* value) {
-  const char kQuote[] = "\"";
-
-  size_t begin = attribute_list.find(attribute + "=" + kQuote);
-  if (begin == std::string::npos)
-    return false;  // Can't find the attribute.
-
-  begin = attribute_list.find(kQuote, begin) + 1;
-
-  size_t end = begin + 1;
-  while (end < attribute_list.size()) {
-    if (attribute_list[end] == '"' &&
-        attribute_list[end - 1] != '\\') {
-      break;
-    }
-    end++;
-  }
-
-  if (end == attribute_list.size())
-    return false;  // The value is not quoted.
-
-  *value = attribute_list.substr(begin, end - begin);
-  return true;
-}
-
-// static
-void Firefox2Importer::HTMLUnescape(string16* text) {
-  string16 text16 = *text;
-  ReplaceSubstringsAfterOffset(
-      &text16, 0, ASCIIToUTF16("&lt;"), ASCIIToUTF16("<"));
-  ReplaceSubstringsAfterOffset(
-      &text16, 0, ASCIIToUTF16("&gt;"), ASCIIToUTF16(">"));
-  ReplaceSubstringsAfterOffset(
-      &text16, 0, ASCIIToUTF16("&amp;"), ASCIIToUTF16("&"));
-  ReplaceSubstringsAfterOffset(
-      &text16, 0, ASCIIToUTF16("&quot;"), ASCIIToUTF16("\""));
-  ReplaceSubstringsAfterOffset(
-      &text16, 0, ASCIIToUTF16("&#39;"), ASCIIToUTF16("\'"));
-  text->assign(text16);
-}
-
-// static
-void Firefox2Importer::FindXMLFilesInDir(
-    const base::FilePath& dir,
-    std::vector<base::FilePath>* xml_files) {
-  file_util::FileEnumerator file_enum(dir, false,
-                                      file_util::FileEnumerator::FILES,
-                                      FILE_PATH_LITERAL("*.xml"));
-  base::FilePath file(file_enum.Next());
-  while (!file.empty()) {
-    xml_files->push_back(file);
-    file = file_enum.Next();
-  }
-}
-
-// static
-void Firefox2Importer::DataURLToFaviconUsage(
-    const GURL& link_url,
-    const GURL& favicon_data,
-    std::vector<history::ImportedFaviconUsage>* favicons) {
-  if (!link_url.is_valid() || !favicon_data.is_valid() ||
-      !favicon_data.SchemeIs(chrome::kDataScheme))
-    return;
-
-  // Parse the data URL.
-  std::string mime_type, char_set, data;
-  if (!net::DataURL::Parse(favicon_data, &mime_type, &char_set, &data) ||
-      data.empty())
-    return;
-
-  history::ImportedFaviconUsage usage;
-  if (!importer::ReencodeFavicon(
-          reinterpret_cast<const unsigned char*>(&data[0]),
-          data.size(), &usage.png_data))
-    return;  // Unable to decode.
-
-  // We need to make up a URL for the favicon. We use a version of the page's
-  // URL so that we can be sure it will not collide.
-  usage.favicon_url = GURL(std::string("made-up-favicon:") + link_url.spec());
-
-  // We only have one URL per favicon for Firefox 2 bookmarks.
-  usage.urls.insert(link_url);
-
-  favicons->push_back(usage);
-}