Extract BookmarksFileImporter from Firefox2Importer.

chrome/browser/importer/bookmarks_file_importer.cc

+// 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.
+
+#include "chrome/browser/importer/bookmarks_file_importer.h"
+
+#include "base/bind.h"
+#include "chrome/browser/bookmarks/bookmark_html_reader.h"
+#include "chrome/browser/bookmarks/imported_bookmark_entry.h"
+#include "chrome/browser/importer/firefox_importer_utils.h"
+#include "chrome/browser/importer/importer_bridge.h"
+#include "chrome/browser/importer/importer_data_types.h"
+#include "chrome/browser/importer/importer_util.h"
+#include "grit/generated_resources.h"
+
+namespace {
+
+bool ImporterCancelBridge(BookmarksFileImporter* importer) {

[gab, 2013/05/10 20:37:57]

nit: I find this name rather vague, rename to IsImporterCancelled()?

[Avi (use Gerrit), 2013/05/10 21:34:53]

Done.

+  return importer->cancelled();
+}
+
+}  // namespace
+
+BookmarksFileImporter::BookmarksFileImporter() {}

[gab, 2013/05/10 20:37:57]

Inline empty constructors in the header.

[Avi (use Gerrit), 2013/05/10 21:34:53]

Why? We put empty constructors in .cc files all the time.

[gab, 2013/05/10 21:47:44]

Ah ok, I guess that's fine, I thought I'd mostly seen them inlined in the
header; I think not doing that prevents it from being inlined in the caller
resulting in an extra jmp in the assembly.

+
+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);
+
+  bridge->NotifyStarted();
+  bridge->NotifyItemStarted(importer::FAVORITES);
+
+  std::vector<ImportedBookmarkEntry> bookmarks;
+  std::vector<history::ImportedFaviconUsage> dirty_favicons;
+  base::Callback<bool(void)> cancellation_callback =
+      base::Bind(ImporterCancelBridge, base::Unretained(this));
+  base::Callback<bool(const GURL&)> valid_url_callback =
+      base::Bind(CanImportURL);
+
+  bookmark_html_reader::ImportBookmarksFile(&cancellation_callback,
+                                            &valid_url_callback,
+                                            source_profile.source_path,
+                                            &bookmarks,
+                                            &dirty_favicons);
+
+  std::vector<history::ImportedFaviconUsage> clean_favicons;
+  for (size_t i = 0; i < dirty_favicons.size(); ++i) {
+    history::ImportedFaviconUsage& favicon = dirty_favicons[i];

[gab, 2013/05/10 20:37:57]

Use a *, non-const & are not allowed by the style guide.

[Avi (use Gerrit), 2013/05/10 21:34:53]

They're disallowed as parameters; where are they disallowed in function bodies?

[gab, 2013/05/10 21:47:44]

You're right, the style only mentions parameters... I guess this is fine.

+    std::vector<unsigned char> clean_data;
+    if (importer::ReencodeFavicon(&favicon.png_data[0],
+                                  favicon.png_data.size(),
+                                  &clean_data)) {
+      favicon.png_data.swap(clean_data);
+    }
+    clean_favicons.push_back(favicon);
+  }
+
+  if (!bookmarks.empty() && !cancelled()) {
+    base::string16 first_folder_name =
+        bridge->GetLocalizedString(IDS_BOOKMARK_GROUP);
+    bridge->AddBookmarks(bookmarks, first_folder_name);
+  }
+  if (!clean_favicons.empty())
+    bridge->SetFavicons(clean_favicons);
+
+  bridge->NotifyItemEnded(importer::FAVORITES);
+  bridge->NotifyEnded();
+}