Current firefox2_importer and firefox3_importer could not import empty folder.

This changelist makes it possible to import initially empty folders.

This patch is contributed by Takashi Toyoshima <toyoshim@google.com>.
Original code review: http://codereview.chromium.org/6373009/

BUG=25863
TEST=add and fix some tests to unit_tests for firefox2_importer and pass it.

Review URL: http://codereview.chromium.org/6402003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72933 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 148 insertions, 24 deletions

diff --git a/chrome/browser/importer/firefox2_importer.cc b/chrome/browser/importer/firefox2_importer.cc
index 59f9ff2..9a73f28 100644
--- a/chrome/browser/importer/firefox2_importer.cc
+++ b/chrome/browser/importer/firefox2_importer.cc
@@ -165,6 +165,8 @@ void Firefox2Importer::ImportBookmarksFile(
   std::vector<ProfileWriter::BookmarkEntry> toolbar_bookmarks;
   std::wstring last_folder = first_folder_name;
   bool last_folder_on_toolbar = false;
+  bool last_folder_is_empty = true;
+  Time last_folder_add_date;
   std::vector<std::wstring> path;
   size_t toolbar_folder = 0;
   std::string charset;
@@ -179,7 +181,8 @@ void Firefox2Importer::ImportBookmarksFile(
 
     // Get the folder name.
     if (ParseFolderNameFromLine(line, charset, &last_folder,
-                                &last_folder_on_toolbar))
+                                &last_folder_on_toolbar,
+                                &last_folder_add_date))
       continue;
 
     // Get the bookmark entry.
@@ -187,11 +190,16 @@ void Firefox2Importer::ImportBookmarksFile(
     GURL url, favicon;
     Time add_date;
     std::wstring post_data;
+    bool is_bookmark;
     // TODO(jcampan): http://b/issue?id=1196285 we do not support POST based
     //                keywords yet.
-    if (ParseBookmarkFromLine(line, charset, &title,
-                              &url, &favicon, &shortcut, &add_date,
-                              &post_data) &&
+    is_bookmark = ParseBookmarkFromLine(line, charset, &title,
+                                        &url, &favicon, &shortcut, &add_date,
+                                        &post_data);
+    if (is_bookmark)
+      last_folder_is_empty = false;
+
+    if (is_bookmark &&
         post_data.empty() &&
         CanImportURL(GURL(url)) &&
         default_urls.find(url) == default_urls.end()) {
@@ -240,10 +248,39 @@ void Firefox2Importer::ImportBookmarksFile(
       last_folder.clear();
       if (last_folder_on_toolbar && !toolbar_folder)
         toolbar_folder = path.size();
+
+      // Mark next folder empty as initial state.
+      last_folder_is_empty = true;
     } else if (StartsWithASCII(line, "</DL>", true)) {
       if (path.empty())
         break;  // Mismatch <DL>.
+
+      std::wstring folder_title = path.back();
       path.pop_back();
+
+      if (last_folder_is_empty) {
+        // Empty folder should be added explicitly.
+        ProfileWriter::BookmarkEntry entry;
+        entry.is_folder = true;
+        entry.creation_time = last_folder_add_date;
+        entry.title = folder_title;
+        if (import_to_bookmark_bar && toolbar_folder) {
+          // Flatten the folder in toolbar.
+          entry.in_toolbar = true;
+          entry.path.assign(path.begin() + toolbar_folder, path.end());
+          toolbar_bookmarks.push_back(entry);
+        } else {
+          // Insert the folder into the "Imported from Firefox" folder.
+          entry.path.assign(path.begin(), path.end());
+          if (import_to_bookmark_bar)
+            entry.path.erase(entry.path.begin());
+          bookmarks->push_back(entry);
+        }
+
+        // Parent folder include current one, so it's not empty.
+        last_folder_is_empty = false;
+      }
+
       if (toolbar_folder > path.size())
         toolbar_folder = 0;
     }
@@ -382,10 +419,12 @@ bool Firefox2Importer::ParseCharsetFromLine(const std::string& line,
 bool Firefox2Importer::ParseFolderNameFromLine(const std::string& line,
                                                const std::string& charset,
                                                std::wstring* folder_name,
-                                               bool* is_toolbar_folder) {
+                                               bool* is_toolbar_folder,
+                                               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;
@@ -403,6 +442,16 @@ bool Firefox2Importer::ParseFolderNameFromLine(const std::string& line,
   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 = Time::FromTimeT(time);
+  }
+
   if (GetAttribute(attribute_list, kToolbarFolderAttribute, &value) &&
       LowerCaseEqualsASCII(value, "true"))
     *is_toolbar_folder = true;
diff --git a/chrome/browser/importer/firefox2_importer.h b/chrome/browser/importer/firefox2_importer.h
index 5264c556..8901e6f 100644
--- a/chrome/browser/importer/firefox2_importer.h
+++ b/chrome/browser/importer/firefox2_importer.h
@@ -52,6 +52,7 @@ class Firefox2Importer : public Importer {
  private:
   FRIEND_TEST_ALL_PREFIXES(FirefoxImporterTest, Firefox2BookmarkParse);
   FRIEND_TEST_ALL_PREFIXES(FirefoxImporterTest, Firefox2CookesParse);
+  FRIEND_TEST_ALL_PREFIXES(FirefoxImporterTest, Firefox2BookmarkFileImport);
 
   virtual ~Firefox2Importer();
 
@@ -84,7 +85,8 @@ class Firefox2Importer : public Importer {
   static bool ParseFolderNameFromLine(const std::string& line,
                                       const std::string& charset,
                                       std::wstring* folder_name,
-                                      bool* is_toolbar_folder);
+                                      bool* is_toolbar_folder,
+                                      base::Time* add_date);
   // See above, this will also put the data: URL of the favicon into *favicon
   // if there is a favicon given.  |post_data| is set for POST base keywords to
   // the contents of the actual POST (with %s for the search term).
diff --git a/chrome/browser/importer/firefox3_importer.cc b/chrome/browser/importer/firefox3_importer.cc
index 6442c87..6ec18cc 100644
--- a/chrome/browser/importer/firefox3_importer.cc
+++ b/chrome/browser/importer/firefox3_importer.cc
@@ -35,15 +35,25 @@ using importer::ProfileInfo;
 using importer::SEARCH_ENGINES;
 using webkit_glue::PasswordForm;
 
+// Original definition is in http://mxr.mozilla.org/firefox/source/toolkit/
+//  components/places/public/nsINavBookmarksService.idl
+enum BookmarkItemType {
+  TYPE_BOOKMARK = 1,
+  TYPE_FOLDER = 2,
+  TYPE_SEPARATOR = 3,
+  TYPE_DYNAMIC_CONTAINER = 4
+};
+
 struct Firefox3Importer::BookmarkItem {
   int parent;
   int id;
   GURL url;
   std::wstring title;
-  int type;
+  BookmarkItemType type;
   std::string keyword;
   base::Time date_added;
   int64 favicon;
+  bool empty_folder;
 };
 
 Firefox3Importer::Firefox3Importer() {
@@ -180,7 +190,7 @@ void Firefox3Importer::ImportBookmarks() {
   GetTopBookmarkFolder(db.get(), unsorted_folder_id, &list);
   size_t count = list.size();
   for (size_t i = 0; i < count; ++i)
-    GetWholeBookmarkFolder(db.get(), &list, i);
+    GetWholeBookmarkFolder(db.get(), &list, i, NULL);
 
   std::vector<ProfileWriter::BookmarkEntry> bookmarks;
   std::vector<TemplateURL*> template_urls;
@@ -207,13 +217,21 @@ void Firefox3Importer::ImportBookmarks() {
   for (size_t i = 0; i < list.size(); ++i) {
     BookmarkItem* item = list[i];
 
-    // The type of bookmark items is 1.
-    if (item->type != 1)
+    if (item->type == TYPE_FOLDER) {
+      // Folders are added implicitly on adding children,
+      // so now we pass only empty folders to add them explicitly.
+      if (!item->empty_folder)
+        continue;
+    } else if (item->type == TYPE_BOOKMARK) {
+      // Import only valid bookmarks
+      if (!CanImportURL(item->url))
+        continue;
+    } else {
       continue;
+    }
 
     // Skip the default bookmarks and unwanted URLs.
-    if (!CanImportURL(item->url) ||
-        default_urls.find(item->url) != default_urls.end() ||
+    if (default_urls.find(item->url) != default_urls.end() ||
         post_keyword_ids.find(item->id) != post_keyword_ids.end())
       continue;
 
@@ -261,6 +279,7 @@ void Firefox3Importer::ImportBookmarks() {
     entry.url = item->url;
     entry.path = path;
     entry.in_toolbar = is_in_toolbar;
+    entry.is_folder = item->type == TYPE_FOLDER;
 
     bookmarks.push_back(entry);
 
@@ -488,14 +507,16 @@ void Firefox3Importer::GetTopBookmarkFolder(sqlite3* db, int folder_id,
     item->parent = -1;  // The top level folder has no parent.
     item->id = folder_id;
     item->title = s.column_wstring(0);
-    item->type = 2;
+    item->type = TYPE_FOLDER;
     item->favicon = 0;
+    item->empty_folder = true;
     list->push_back(item);
   }
 }
 
 void Firefox3Importer::GetWholeBookmarkFolder(sqlite3* db, BookmarkList* list,
-                                              size_t position) {
+                                              size_t position,
+                                              bool* empty_folder) {
   if (position >= list->size()) {
     NOTREACHED();
     return;
@@ -520,12 +541,15 @@ void Firefox3Importer::GetWholeBookmarkFolder(sqlite3* db, BookmarkList* list,
     item->id = s.column_int(0);
     item->url = GURL(s.column_string(1));
     item->title = s.column_wstring(2);
-    item->type = s.column_int(3);
+    item->type = static_cast<BookmarkItemType>(s.column_int(3));
     item->keyword = s.column_string(4);
     item->date_added = Time::FromTimeT(s.column_int64(5)/1000000);
     item->favicon = s.column_int64(6);
+    item->empty_folder = true;
 
     temp_list.push_back(item);
+    if (empty_folder != NULL)
+      *empty_folder = false;
   }
 
   // Appends all items to the list.
@@ -533,8 +557,8 @@ void Firefox3Importer::GetWholeBookmarkFolder(sqlite3* db, BookmarkList* list,
        i != temp_list.end(); ++i) {
     list->push_back(*i);
     // Recursive add bookmarks in sub-folders.
-    if ((*i)->type == 2)
-      GetWholeBookmarkFolder(db, list, list->size() - 1);
+    if ((*i)->type == TYPE_FOLDER)
+      GetWholeBookmarkFolder(db, list, list->size() - 1, &(*i)->empty_folder);
   }
 }
 
diff --git a/chrome/browser/importer/firefox3_importer.h b/chrome/browser/importer/firefox3_importer.h
index fe3deb7..6530edc 100644
--- a/chrome/browser/importer/firefox3_importer.h
+++ b/chrome/browser/importer/firefox3_importer.h
@@ -62,7 +62,7 @@ class Firefox3Importer : public Importer {
 
   // Loads all children of the given folder, and appends them to the |list|.
   void GetWholeBookmarkFolder(sqlite3* db, BookmarkList* list,
-                              size_t position);
+                              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.
diff --git a/chrome/browser/importer/firefox_importer_unittest.cc b/chrome/browser/importer/firefox_importer_unittest.cc
index 73c70a4..dc6d44d 100644
--- a/chrome/browser/importer/firefox_importer_unittest.cc
+++ b/chrome/browser/importer/firefox_importer_unittest.cc
@@ -87,18 +87,20 @@ TEST(FirefoxImporterTest, Firefox2BookmarkParse) {
   // Escaped characters in name.
   std::wstring folder_name;
   bool is_toolbar_folder;
+  Time folder_add_date;
   result = Firefox2Importer::ParseFolderNameFromLine(
       "<DT><H3 ADD_DATE=\"1207558707\" >&lt; &gt;"
       " &amp; &quot; &#39; \\ /</H3>",
-      charset, &folder_name, &is_toolbar_folder);
+      charset, &folder_name, &is_toolbar_folder, &folder_add_date);
   EXPECT_TRUE(result);
   EXPECT_EQ(L"< > & \" ' \\ /", folder_name);
   EXPECT_FALSE(is_toolbar_folder);
+  EXPECT_TRUE(Time::FromTimeT(1207558707) == folder_add_date);
 
   // Empty name and toolbar folder attribute.
   result = Firefox2Importer::ParseFolderNameFromLine(
       "<DT><H3 PERSONAL_TOOLBAR_FOLDER=\"true\"></H3>",
-      charset, &folder_name, &is_toolbar_folder);
+      charset, &folder_name, &is_toolbar_folder, &folder_add_date);
   EXPECT_TRUE(result);
   EXPECT_EQ(L"", folder_name);
   EXPECT_TRUE(is_toolbar_folder);
@@ -177,3 +179,43 @@ TEST(FirefoxImporterTest, Firefox2BookmarkParse) {
   EXPECT_EQ(L"", post_data);
   EXPECT_TRUE(Time() == add_date);
 }
+
+TEST(FirefoxImporterTest, Firefox2BookmarkFileImport) {
+  FilePath path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
+  path = path.AppendASCII("firefox2_importer");
+
+  // Import all bookmarks from a file which include an empty folder entry.
+  FilePath empty_folder_path = path.AppendASCII("empty_folder.html");
+  std::set<GURL> default_urls;
+  std::wstring first_folder_name;
+  Firefox2Importer* importer = new Firefox2Importer();
+  importer->AddRef();
+  std::vector<ProfileWriter::BookmarkEntry> bookmarks;
+  importer->ImportBookmarksFile(empty_folder_path, default_urls, false,
+                                first_folder_name, importer, &bookmarks,
+                                NULL, NULL);
+  EXPECT_EQ(3, static_cast<int>(bookmarks.size()));
+  std::vector<ProfileWriter::BookmarkEntry>::iterator it = bookmarks.begin();
+  ProfileWriter::BookmarkEntry entry = *it++;
+  EXPECT_EQ(L"Empty", entry.title);
+  entry = *it++;
+  EXPECT_EQ(L"[Tamura Yukari.com]", entry.title);
+  entry = *it++;
+  EXPECT_EQ(L"Google", entry.title);
+
+  // Import non-default bookmarks from a file.
+  bookmarks.clear();
+  default_urls.insert(GURL("http://www.google.com/"));
+  importer->ImportBookmarksFile(empty_folder_path, default_urls, false,
+                                first_folder_name, importer, &bookmarks,
+                                NULL, NULL);
+  EXPECT_EQ(2, static_cast<int>(bookmarks.size()));
+  it = bookmarks.begin();
+  entry = *it++;
+  EXPECT_EQ(L"Empty", entry.title);
+  entry = *it++;
+  EXPECT_EQ(L"[Tamura Yukari.com]", entry.title);
+
+  importer->Release();
+}
diff --git a/chrome/browser/importer/profile_writer.cc b/chrome/browser/importer/profile_writer.cc
index e6eefdf..1ea2c46 100644
--- a/chrome/browser/importer/profile_writer.cc
+++ b/chrome/browser/importer/profile_writer.cc
@@ -19,7 +19,8 @@
 
 using webkit_glue::PasswordForm;
 
-ProfileWriter::BookmarkEntry::BookmarkEntry() : in_toolbar(false) {}
+ProfileWriter::BookmarkEntry::BookmarkEntry() : in_toolbar(false),
+    is_folder(false) {}
 
 ProfileWriter::BookmarkEntry::~BookmarkEntry() {}
 
@@ -82,7 +83,7 @@ void ProfileWriter::AddBookmarkEntry(
   for (std::vector<BookmarkEntry>::const_iterator it = bookmark.begin();
        it != bookmark.end(); ++it) {
     // Don't insert this url if it isn't valid.
-    if (!it->url.is_valid())
+    if (!it->is_folder && !it->url.is_valid())
       continue;
 
     // We suppose that bookmarks are unique by Title, URL, and Folder.  Since
@@ -119,8 +120,13 @@ void ProfileWriter::AddBookmarkEntry(
       parent = child;
     }
     groups_added_to.insert(parent);
-    model->AddURLWithCreationTime(parent, parent->GetChildCount(),
-        WideToUTF16Hack(it->title), it->url, it->creation_time);
+    if (it->is_folder) {
+      model->AddGroup(parent, parent->GetChildCount(),
+                      WideToUTF16Hack(it->title));
+    } else {
+      model->AddURLWithCreationTime(parent, parent->GetChildCount(),
+          WideToUTF16Hack(it->title), it->url, it->creation_time);
+    }
 
     // If some items are put into toolbar, it looks like the user was using
     // it in their last browser. We turn on the bookmarks toolbar.
diff --git a/chrome/browser/importer/profile_writer.h b/chrome/browser/importer/profile_writer.h
index 5c0e25c..3002b1a 100644
--- a/chrome/browser/importer/profile_writer.h
+++ b/chrome/browser/importer/profile_writer.h
@@ -56,6 +56,7 @@ class ProfileWriter : public base::RefCountedThreadSafe<ProfileWriter> {
     ~BookmarkEntry();
 
     bool in_toolbar;
+    bool is_folder;
     GURL url;
     std::vector<std::wstring> path;
     std::wstring title;