Fix bookmark importing from Firefox

Bug: 638977
Change-Id: I3336f64fc9497417831afa24f8403a63b77797bb
Reviewed-on: https://chromium-review.googlesource.com/817435
Commit-Queue: Tomasz Moniuszko <tmoniuszko@opera.com>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524685}

chrome/utility/importer/firefox_importer.cc

@@ -80,6 +80,23 @@ bool CanImportURL(const GURL& url) {
   return true;
 }
 
+// Initializes |favicon_url| and |png_data| members of given FaviconUsageData
+// structure with provided favicon data. Returns true if data is valid.
+bool SetFaviconData(const std::string& icon_url,
+                    const std::vector<unsigned char>& icon_data,
+                    favicon_base::FaviconUsageData* usage_data) {
+  usage_data->favicon_url = GURL(icon_url);
+
+  // Don't bother importing favicons with invalid URLs.
+  if (!usage_data->favicon_url.is_valid())
+    return false;
+
+  // Data must be valid.
+  return !icon_data.empty() &&
+         importer::ReencodeFavicon(&icon_data[0], icon_data.size(),
+                                   &usage_data->png_data);
+}
+
 }  // namespace
 
 struct FirefoxImporter::BookmarkItem {
@@ -208,6 +225,13 @@ void FirefoxImporter::ImportBookmarks() {
   if (!db.Open(file))
     return;
 
+  // |moz_favicons| table has been introduced in Firefox 55 and is not available
+  // in older Firefox profiles.
+  FaviconsLocation favicons_location =
+      db.IsSQLValid("SELECT count(*) FROM moz_favicons")
+          ? FaviconsLocation::kPlacesDatabase
+          : FaviconsLocation::kFaviconsDatabase;
+
   // Get the bookmark folders that we are interested in.
   int toolbar_folder_id = LoadNodeIDByGUID(&db, "toolbar_____");
   int menu_folder_id = LoadNodeIDByGUID(&db, "menu________");
@@ -227,7 +251,7 @@ void FirefoxImporter::ImportBookmarks() {
   GetTopBookmarkFolder(&db, unsorted_folder_id, &list);
   size_t count = list.size();
   for (size_t i = 0; i < count; ++i)
-    GetWholeBookmarkFolder(&db, &list, i, NULL);
+    GetWholeBookmarkFolder(&db, &list, i, favicons_location, nullptr);
 
   std::vector<ImportedBookmarkEntry> bookmarks;
   std::vector<importer::SearchEngineInfo> search_engines;
@@ -343,10 +367,17 @@ void FirefoxImporter::ImportBookmarks() {
   if (!search_engines.empty() && !cancelled()) {
     bridge_->SetKeywords(search_engines, false);
   }
-  if (!favicon_map.empty() && !cancelled()) {
+
+  if (!cancelled()) {
     favicon_base::FaviconUsageDataList favicons;
-    LoadFavicons(&db, favicon_map, &favicons);
-    bridge_->SetFavicons(favicons);
+    if (favicons_location == FaviconsLocation::kFaviconsDatabase) {
+      DCHECK(favicon_map.empty());
+      LoadFavicons(bookmarks, &favicons);
+    } else if (!favicon_map.empty()) {
+      LoadFavicons(&db, favicon_map, &favicons);
+    }
+    if (!favicons.empty())
+      bridge_->SetFavicons(favicons);
   }
 }
 
@@ -705,21 +736,25 @@ void FirefoxImporter::GetTopBookmarkFolder(sql::Connection* db,
 void FirefoxImporter::GetWholeBookmarkFolder(sql::Connection* db,
                                              BookmarkList* list,
                                              size_t position,
+                                             FaviconsLocation favicons_location,
                                              bool* empty_folder) {
   if (position >= list->size()) {
     NOTREACHED();
     return;
   }
 
-  const char query[] =
+  std::string query =
       "SELECT b.id, h.url, COALESCE(b.title, h.title), "
-      "b.type, k.keyword, b.dateAdded, h.favicon_id "
+      "b.type, k.keyword, b.dateAdded ";
+  if (favicons_location == FaviconsLocation::kPlacesDatabase)
+    query += ", h.favicon_id ";
+  query +=
       "FROM moz_bookmarks b "
       "LEFT JOIN moz_places h ON b.fk = h.id "
       "LEFT JOIN moz_keywords k ON k.id = b.keyword_id "
       "WHERE b.type IN (1,2) AND b.parent = ? "
       "ORDER BY b.position";
-  sql::Statement s(db->GetUniqueStatement(query));
+  sql::Statement s(db->GetUniqueStatement(query.c_str()));
   s.BindInt(0, (*list)[position]->id);
 
   BookmarkList temp_list;
@@ -732,7 +767,9 @@ void FirefoxImporter::GetWholeBookmarkFolder(sql::Connection* db,
     item->type = static_cast<BookmarkItemType>(s.ColumnInt(3));
     item->keyword = s.ColumnString(4);
     item->date_added = base::Time::FromTimeT(s.ColumnInt64(5)/1000000);
-    item->favicon = s.ColumnInt64(6);
+    item->favicon = favicons_location == FaviconsLocation::kPlacesDatabase
+                        ? s.ColumnInt64(6)
+                        : 0;
     item->empty_folder = true;
 
     temp_list.push_back(std::move(item));
@@ -745,7 +782,7 @@ void FirefoxImporter::GetWholeBookmarkFolder(sql::Connection* db,
     list->push_back(std::move(bookmark));
     // Recursive add bookmarks in sub-folders.
     if (list->back()->type == TYPE_FOLDER) {
-      GetWholeBookmarkFolder(db, list, list->size() - 1,
+      GetWholeBookmarkFolder(db, list, list->size() - 1, favicons_location,
                              &list->back()->empty_folder);
     }
   }
@@ -765,22 +802,68 @@ void FirefoxImporter::LoadFavicons(
        i != favicon_map.end(); ++i) {
     s.BindInt64(0, i->first);
     if (s.Step()) {
-      favicon_base::FaviconUsageData usage;
+      std::vector<unsigned char> data;
+      if (!s.ColumnBlobAsVector(1, &data))
+        continue;
 
-      usage.favicon_url = GURL(s.ColumnString(0));
-      if (!usage.favicon_url.is_valid())
-        continue;  // Don't bother importing favicons with invalid URLs.
+      favicon_base::FaviconUsageData usage_data;
+      if (!SetFaviconData(s.ColumnString(0), data, &usage_data))
+        continue;
+
+      usage_data.urls = i->second;
+      favicons->push_back(usage_data);
+    }
+    s.Reset(true);
+  }
+}
+
+void FirefoxImporter::LoadFavicons(
+    const std::vector<ImportedBookmarkEntry>& bookmarks,
+    favicon_base::FaviconUsageDataList* favicons) {
+  base::FilePath file = source_path_.AppendASCII("favicons.sqlite");
+  if (!base::PathExists(file))
+    return;
+
+  sql::Connection db;
+  if (!db.Open(file))
+    return;
+
+  sql::Statement s(db.GetUniqueStatement(
+      "SELECT moz_icons.id, moz_icons.icon_url, moz_icons.data "
+      "FROM moz_icons "
+      "INNER JOIN moz_icons_to_pages "
+      "ON moz_icons.id = moz_icons_to_pages.icon_id "
+      "INNER JOIN moz_pages_w_icons "
+      "ON moz_pages_w_icons.id = moz_icons_to_pages.page_id "
+      "WHERE moz_pages_w_icons.page_url = ?"));
+  if (!s.is_valid())
+    return;
+
+  // A map from icon id to the corresponding index in the |favicons| vector.
+  std::map<uint64_t, size_t> icon_cache;
+
+  for (const auto& entry : bookmarks) {
+    s.BindString(0, entry.url.spec());
+    if (s.Step()) {
+      uint64_t icon_id = s.ColumnInt64(0);
+      auto it = icon_cache.find(icon_id);
+      if (it != icon_cache.end()) {
+        // A favicon that's used for multiple URLs. Append this URL to the list.
+        (*favicons)[it->second].urls.insert(entry.url);
+        continue;
+      }
 
       std::vector<unsigned char> data;
-      s.ColumnBlobAsVector(1, &data);
-      if (data.empty())
-        continue;  // Data definitely invalid.
+      if (!s.ColumnBlobAsVector(2, &data))
+        continue;
 
-      if (!importer::ReencodeFavicon(&data[0], data.size(), &usage.png_data))
-        continue;  // Unable to decode.
+      favicon_base::FaviconUsageData usage_data;
+      if (!SetFaviconData(s.ColumnString(1), data, &usage_data))
+        continue;
 
-      usage.urls = i->second;
-      favicons->push_back(usage);
+      usage_data.urls.insert(entry.url);
+      favicons->push_back(usage_data);
+      icon_cache[icon_id] = favicons->size() - 1;
     }
     s.Reset(true);
   }

chrome/utility/importer/firefox_importer.h

@@ -19,6 +19,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "build/build_config.h"
+#include "chrome/common/importer/imported_bookmark_entry.h"
 #include "chrome/utility/importer/importer.h"
 #include "components/favicon_base/favicon_usage_data.h"
 
@@ -41,6 +42,16 @@ class FirefoxImporter : public Importer {
                    ImporterBridge* bridge) override;
 
  private:
+  // Location of favicons in Firefox profile. It may vary depending on Firefox
+  // version.
+  enum class FaviconsLocation {
+    // Favicons are stored in places.sqlite database (older Firefox versions).
+    kPlacesDatabase,
+
+    // Favicons are stored in favicons.sqlite (Firefox 55 and newer).
+    kFaviconsDatabase,
+  };
+
   using FaviconMap = std::map<int64_t, std::set<GURL>>;
 
   ~FirefoxImporter() override;
@@ -76,15 +87,25 @@ class FirefoxImporter : public Importer {
                             BookmarkList* list);
 
   // Loads all children of the given folder, and appends them to the |list|.
-  void GetWholeBookmarkFolder(sql::Connection* db, BookmarkList* list,
-                              size_t position, bool* empty_folder);
-
-  // Loads the favicons given in the map from the database, loads the data,
-  // and converts it into FaviconUsage structures.
+  void GetWholeBookmarkFolder(sql::Connection* db,
+                              BookmarkList* list,
+                              size_t position,
+                              FaviconsLocation favicons_location,
+                              bool* empty_folder);
+
+  // Loads the favicons given in the map from places.sqlite database, loads the
+  // data, and converts it into FaviconUsageData structures.
+  // This function supports older Firefox profiles (up to version 54).
   void LoadFavicons(sql::Connection* db,
                     const FaviconMap& favicon_map,
                     favicon_base::FaviconUsageDataList* favicons);
 
+  // Loads the favicons for |bookmarks| from favicons.sqlite database, loads the
+  // data, and converts it into FaviconUsageData structures.
+  // This function supports newer Firefox profiles (Firefox 55 and later).
+  void LoadFavicons(const std::vector<ImportedBookmarkEntry>& bookmarks,
+                    favicon_base::FaviconUsageDataList* favicons);
+
   base::FilePath source_path_;
   base::FilePath app_path_;
 

chrome/utility/importer/firefox_importer_unittest.cc

@@ -2,7 +2,11 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <string>
+#include <vector>
+
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
@@ -19,6 +23,38 @@
 #include "sql/connection.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+// Imports bookmarks from Firefox profile files into |bookmarks| and |favicons|
+// containers. |firefox_version| must match the name of subdirectory where test
+// files are stored.
+void ImportBookmarksFromVersion(base::StringPiece firefox_version,
+                                std::vector<ImportedBookmarkEntry>* bookmarks,
+                                favicon_base::FaviconUsageDataList* favicons) {
+  using ::testing::_;
+  base::FilePath places_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &places_path));
+  places_path =
+      places_path.AppendASCII("import").AppendASCII("firefox").AppendASCII(
+          firefox_version);
+  ASSERT_TRUE(base::DirectoryExists(places_path));
+  scoped_refptr<FirefoxImporter> importer = new FirefoxImporter;
+  importer::SourceProfile profile;
+  profile.source_path = places_path;
+  scoped_refptr<MockImporterBridge> bridge = new MockImporterBridge;
+  EXPECT_CALL(*bridge, NotifyStarted());
+  EXPECT_CALL(*bridge, NotifyItemStarted(importer::FAVORITES));
+  EXPECT_CALL(*bridge, AddBookmarks(_, _))
+      .WillOnce(::testing::SaveArg<0>(bookmarks));
+  EXPECT_CALL(*bridge, SetFavicons(_))
+      .WillOnce(::testing::SaveArg<0>(favicons));
+  EXPECT_CALL(*bridge, NotifyItemEnded(importer::FAVORITES));
+  EXPECT_CALL(*bridge, NotifyEnded());
+  importer->StartImport(profile, importer::FAVORITES, bridge.get());
+}
+
+}  // namespace
+
 // TODO(jschuh): Disabled on Win64 build. http://crbug.com/179688
 #if defined(OS_WIN) && defined(ARCH_CPU_X86_64)
 #define MAYBE_NSS(x) DISABLED_##x
@@ -124,28 +160,11 @@ TEST(FirefoxImporterTest, MAYBE_NSS(FirefoxNSSDecryptorDeduceAuthScheme)) {
   EXPECT_EQ(autofill::PasswordForm::SCHEME_HTML, forms[1].scheme);
 }
 
-TEST(FirefoxImporterTest, ImportBookmarks) {
-  using ::testing::_;
-  base::FilePath places_path;
-  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &places_path));
-  places_path =
-      places_path.AppendASCII("import").AppendASCII("firefox").AppendASCII(
-          "48.0.2");
-  scoped_refptr<FirefoxImporter> importer = new FirefoxImporter;
-  importer::SourceProfile profile;
-  profile.source_path = places_path;
-  scoped_refptr<MockImporterBridge> bridge = new MockImporterBridge;
+TEST(FirefoxImporterTest, ImportBookmarks_Firefox48) {
   std::vector<ImportedBookmarkEntry> bookmarks;
   favicon_base::FaviconUsageDataList favicons;
-  EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyItemStarted(importer::FAVORITES));
-  EXPECT_CALL(*bridge, AddBookmarks(_, _))
-      .WillOnce(::testing::SaveArg<0>(&bookmarks));
-  EXPECT_CALL(*bridge, SetFavicons(_))
-      .WillOnce(::testing::SaveArg<0>(&favicons));
-  EXPECT_CALL(*bridge, NotifyItemEnded(importer::FAVORITES));
-  EXPECT_CALL(*bridge, NotifyEnded());
-  importer->StartImport(profile, importer::FAVORITES, bridge.get());
+  ImportBookmarksFromVersion("48.0.2", &bookmarks, &favicons);
+
   ASSERT_EQ(6u, bookmarks.size());
   EXPECT_EQ("https://www.mozilla.org/en-US/firefox/central/",
             bookmarks[0].url.spec());
@@ -157,6 +176,7 @@ TEST(FirefoxImporterTest, ImportBookmarks) {
             bookmarks[3].url.spec());
   EXPECT_EQ("https://www.mozilla.org/en-US/about/", bookmarks[4].url.spec());
   EXPECT_EQ("https://www.google.com/", bookmarks[5].url.spec());
+
   ASSERT_EQ(5u, favicons.size());
   EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/0-1473403921346",
             favicons[0].favicon_url.spec());
@@ -170,6 +190,36 @@ TEST(FirefoxImporterTest, ImportBookmarks) {
             favicons[4].favicon_url.spec());
 }
 
+TEST(FirefoxImporterTest, ImportBookmarks_Firefox57) {
+  std::vector<ImportedBookmarkEntry> bookmarks;
+  favicon_base::FaviconUsageDataList favicons;
+  ImportBookmarksFromVersion("57.0.1", &bookmarks, &favicons);
+
+  ASSERT_EQ(6u, bookmarks.size());
+  EXPECT_EQ("https://www.mozilla.org/en-US/firefox/central/",
+            bookmarks[0].url.spec());
+  EXPECT_EQ("https://support.mozilla.org/en-US/products/firefox",
+            bookmarks[1].url.spec());
+  EXPECT_EQ("https://www.mozilla.org/en-US/firefox/customize/",
+            bookmarks[2].url.spec());
+  EXPECT_EQ("https://www.mozilla.org/en-US/contribute/",
+            bookmarks[3].url.spec());
+  EXPECT_EQ("https://www.mozilla.org/en-US/about/", bookmarks[4].url.spec());
+  EXPECT_EQ("https://www.google.com/", bookmarks[5].url.spec());
+
+  ASSERT_EQ(5u, favicons.size());
+  EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/0-1513248843421",
+            favicons[0].favicon_url.spec());
+  EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/1-1513248843424",
+            favicons[1].favicon_url.spec());
+  EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/2-1513248843426",
+            favicons[2].favicon_url.spec());
+  EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/3-1513248843427",
+            favicons[3].favicon_url.spec());
+  EXPECT_EQ("http://www.mozilla.org/2005/made-up-favicon/4-1513248843429",
+            favicons[4].favicon_url.spec());
+}
+
 TEST(FirefoxImporterTest, ImportHistorySchema) {
   using ::testing::_;
   base::FilePath places_path;