Extract BookmarksFileImporter from Firefox2Importer.

chrome/browser/importer/bookmarks_file_importer.cc

Index: chrome/browser/importer/bookmarks_file_importer.cc
diff --git a/chrome/browser/importer/firefox2_importer.cc b/chrome/browser/importer/bookmarks_file_importer.cc
similarity index 54%
copy from chrome/browser/importer/firefox2_importer.cc
copy to chrome/browser/importer/bookmarks_file_importer.cc
index 9d70c538dde1e761a253976af7e346dac24fb839..b867de06743a060af79cd9b8baeda77692e40ff6 100644
--- a/chrome/browser/importer/firefox2_importer.cc
+++ b/chrome/browser/importer/bookmarks_file_importer.cc
@@ -1,155 +1,146 @@
-// 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";
-}
 
-Firefox2Importer::Firefox2Importer() : parsing_bookmarks_html_file_(false) {}
+// 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[] = "\"";
 
-Firefox2Importer::~Firefox2Importer() {}
+  size_t begin = attribute_list.find(attribute + "=" + kQuote);
+  if (begin == std::string::npos)
+    return false;  // Can't find the attribute.
 
-void Firefox2Importer::StartImport(
-    const importer::SourceProfile& source_profile,
-    uint16 items,
-    ImporterBridge* bridge) {
-  bridge_ = bridge;
-  source_path_ = source_profile.source_path;
-  app_path_ = source_profile.app_path;
-
-  parsing_bookmarks_html_file_ =
-      (source_profile.importer_type == importer::TYPE_BOOKMARKS_FILE);
-
-  // The order here is important!
-  bridge_->NotifyStarted();
-  if ((items & importer::HOME_PAGE) && !cancelled())
-    ImportHomepage();  // Doesn't have a UI item.
-
-  // 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);
-  }
+  begin = attribute_list.find(kQuote, begin) + 1;
 
-  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);
+  size_t end = begin + 1;
+  while (end < attribute_list.size()) {
+    if (attribute_list[end] == '"' &&
+        attribute_list[end - 1] != '\\') {
+      break;
+    }
+    end++;
   }
-  bridge_->NotifyEnded();
+
+  if (end == attribute_list.size())
+    return false;  // The value is not quoted.
+
+  *value = attribute_list.substr(begin, end - begin);
+  return true;
 }
 
-// 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");
+// 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;
 
-  urls->clear();
+  // 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;
 
-  // Read the whole file.
-  std::string content;
-  file_util::ReadFileToString(file, &content);
-  std::vector<std::string> lines;
-  base::SplitString(content, '\n', &lines);
+  history::ImportedFaviconUsage usage;
+  if (!importer::ReencodeFavicon(
+          reinterpret_cast<const unsigned char*>(&data[0]),
+          data.size(), &usage.png_data))
+    return;  // Unable to decode.
 
-  std::string charset;
-  for (size_t i = 0; i < lines.size(); ++i) {
-    std::string line;
-    TrimString(lines[i], " ", &line);
+  // 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());
 
-    // Get the encoding of the bookmark file.
-    if (ParseCharsetFromLine(line, &charset))
-      continue;
+  // We only have one URL per favicon for Firefox 2 bookmarks.
+  usage.urls.insert(link_url);
 
-    // 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);
-  }
+  favicons->push_back(usage);
 }
 
-// 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);
+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) {
+  // The only thing this importer can import is a bookmarks file, aka
+  // "favorites".
+  DCHECK_EQ(importer::FAVORITES, items);
+
+  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.

+
+  bridge->NotifyStarted();
+  bridge->NotifyItemStarted(importer::FAVORITES);
+
+  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);
+
+  bridge->NotifyItemEnded(importer::FAVORITES);
+  bridge->NotifyEnded();
 }
 
 // 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);
@@ -164,7 +155,9 @@ void Firefox2Importer::ImportBookmarksFile(
   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);
@@ -198,8 +191,7 @@ void Firefox2Importer::ImportBookmarksFile(
 
     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;
@@ -229,14 +221,6 @@ void Firefox2Importer::ImportBookmarksFile(
       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;
     }
 
@@ -289,103 +273,9 @@ void Firefox2Importer::ImportBookmarksFile(
   }
 }
 

[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 ||
@@ -402,11 +292,11 @@ bool Firefox2Importer::ParseCharsetFromLine(const std::string& line,
 }
 
 // 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";
@@ -423,7 +313,7 @@ bool Firefox2Importer::ParseFolderNameFromLine(const std::string& line,
 
   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);
@@ -448,14 +338,23 @@ bool Firefox2Importer::ParseFolderNameFromLine(const std::string& line,
 }
 
 // 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();
@@ -484,14 +383,14 @@ bool Firefox2Importer::ParseBookmarkFromLine(const std::string& line,
   // 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);
   }
@@ -504,7 +403,7 @@ bool Firefox2Importer::ParseBookmarkFromLine(const std::string& line,
   if (GetAttribute(attribute_list, kShortcutURLAttribute, &value)) {
     base::CodepageToUTF16(value, charset.c_str(),
                           base::OnStringConversionError::SKIP, shortcut);
-    HTMLUnescape(shortcut);
+    *shortcut = net::UnescapeForHTML(*shortcut);
   }
 
   // Add date
@@ -520,17 +419,18 @@ bool Firefox2Importer::ParseBookmarkFromLine(const std::string& line,
   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";
@@ -555,7 +455,7 @@ bool Firefox2Importer::ParseMinimumBookmarkFromLine(const std::string& line,
   // 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;
@@ -565,7 +465,7 @@ bool Firefox2Importer::ParseMinimumBookmarkFromLine(const std::string& line,
       string16 url16;
       base::CodepageToUTF16(value, charset.c_str(),
                             base::OnStringConversionError::SKIP, &url16);
-      HTMLUnescape(&url16);
+      url16 = net::UnescapeForHTML(url16);
 
       *url = GURL(url16);
     } else {
@@ -575,92 +475,3 @@ bool Firefox2Importer::ParseMinimumBookmarkFromLine(const std::string& line,
 
   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);
-}