Many fixes to bookmark importing.

chrome/browser/importer/safari_importer.mm

 // Copyright (c) 2011 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 <Cocoa/Cocoa.h>
 
 #include "chrome/browser/importer/safari_importer.h"
 
 #include <map>
 #include <vector>
@@ skipping 79 matching lines @@
   }
   if ((items & importer::PASSWORDS) && !cancelled()) {
     bridge_->NotifyItemStarted(importer::PASSWORDS);
     ImportPasswords();
     bridge_->NotifyItemEnded(importer::PASSWORDS);
   }
   bridge_->NotifyEnded();
 }
 
 void SafariImporter::ImportBookmarks() {
+  string16 toolbar_name =
+      bridge_->GetLocalizedString(IDS_BOOMARK_BAR_FOLDER_NAME);
   std::vector<ProfileWriter::BookmarkEntry> bookmarks;
-  ParseBookmarks(&bookmarks);
+  ParseBookmarks(toolbar_name, &bookmarks);
 
   // Write bookmarks into profile.
   if (!bookmarks.empty() && !cancelled()) {
     const string16& first_folder_name =
         bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP_FROM_SAFARI);
-    int options = 0;
-    if (import_to_bookmark_bar())
-      options = ProfileWriter::IMPORT_TO_BOOKMARK_BAR;
-    bridge_->AddBookmarks(bookmarks, first_folder_name, options);
+    bridge_->AddBookmarks(bookmarks, first_folder_name);
   }
 
   // Import favicons.
   sql::Connection db;
   if (!OpenDatabase(&db))
     return;
 
   FaviconMap favicon_map;
   ImportFaviconURLs(&db, &favicon_map);
   // Write favicons into profile.
@@ skipping 65 matching lines @@
       usage.urls = i->second;
       favicons->push_back(usage);
     }
   }
 }
 
 void SafariImporter::RecursiveReadBookmarksFolder(
     NSDictionary* bookmark_folder,
     const std::vector<string16>& parent_path_elements,
     bool is_in_toolbar,
+    const string16& toolbar_name,
     std::vector<ProfileWriter::BookmarkEntry>* out_bookmarks) {
   DCHECK(bookmark_folder);
 
   NSString* type = [bookmark_folder objectForKey:@"WebBookmarkType"];
   NSString* title = [bookmark_folder objectForKey:@"Title"];
 
   // Are we the dictionary that contains all other bookmarks?
   // We need to know this so we don't add it to the path.
   bool is_top_level_bookmarks_container = [bookmark_folder
       objectForKey:@"WebBookmarkFileVersion"] != nil;
 
   // We're expecting a list of bookmarks here, if that isn't what we got, fail.
   if (!is_top_level_bookmarks_container) {
     // Top level containers sometimes don't have title attributes.
     if (![type isEqualToString:@"WebBookmarkTypeList"] || !title) {
-      DCHECK(false) << "Type =("
-      << (type ? base::SysNSStringToUTF8(type) : "Null Type")
-      << ") Title=(" << (title ? base::SysNSStringToUTF8(title) : "Null title")
-      << ")";
+      NOTREACHED() << "Type=("
+                   << (type ? base::SysNSStringToUTF8(type) : "Null type")
+                   << ") Title=("
+                   << (title ? base::SysNSStringToUTF8(title) : "Null title")
+                   << ")";
       return;
     }
   }
 
+  NSArray* elements = [bookmark_folder objectForKey:@"Children"];
+  if (!elements && (!parent_path_elements.empty() || !is_in_toolbar)) {
+    // This is an empty folder, so add it explicitly; but don't add the toolbar
+    // folder if it is empty.  Note that all non-empty folders are added
+    // implicitly when their children are added.
+    ProfileWriter::BookmarkEntry entry;
+    // Safari doesn't specify a creation time for the folder.
+    entry.creation_time = base::Time::Now();
+    entry.title = base::SysNSStringToUTF16(title);
+    entry.path = parent_path_elements;
+    entry.in_toolbar = is_in_toolbar;
+    entry.is_folder = true;
+
+    out_bookmarks->push_back(entry);
+    return;
+  }
+
   std::vector<string16> path_elements(parent_path_elements);
-  // Is this the toolbar folder?
-  if ([title isEqualToString:@"BookmarksBar"]) {
-    // Be defensive, the toolbar items shouldn't have a prepended path.
-    path_elements.clear();
+  // Create a folder for the toolbar, but not for the bookmarks menu.
+  if (path_elements.empty() && [title isEqualToString:@"BookmarksBar"]) {
     is_in_toolbar = true;
-  } else if ([title isEqualToString:@"BookmarksMenu"]) {
-    // top level container for normal bookmarks.
-    path_elements.clear();
-  } else if (!is_top_level_bookmarks_container) {
+    path_elements.push_back(toolbar_name);
+  } else if (!is_top_level_bookmarks_container &&
+             !(path_elements.empty() &&
+               [title isEqualToString:@"BookmarksMenu"])) {
     if (title)
       path_elements.push_back(base::SysNSStringToUTF16(title));
   }
 
-  NSArray* elements = [bookmark_folder objectForKey:@"Children"];
-  // TODO(jeremy) Does Chrome support importing empty folders?
-  if (!elements)
-    return;
-
   // Iterate over individual bookmarks.
   for (NSDictionary* bookmark in elements) {
     NSString* type = [bookmark objectForKey:@"WebBookmarkType"];
     if (!type)
       continue;
 
     // If this is a folder, recurse.
     if ([type isEqualToString:@"WebBookmarkTypeList"]) {
       RecursiveReadBookmarksFolder(bookmark,
                                    path_elements,
                                    is_in_toolbar,
+                                   toolbar_name,
                                    out_bookmarks);
     }
 
     // If we didn't see a bookmark folder, then we're expecting a bookmark
-    // item, if that's not what we got then ignore it.
+    // item.  If that's not what we got then ignore it.
     if (![type isEqualToString:@"WebBookmarkTypeLeaf"])
       continue;
 
     NSString* url = [bookmark objectForKey:@"URLString"];
     NSString* title = [[bookmark objectForKey:@"URIDictionary"]
         objectForKey:@"title"];
 
     if (!url || !title)
       continue;
 
     // Output Bookmark.
     ProfileWriter::BookmarkEntry entry;
     // Safari doesn't specify a creation time for the bookmark.
     entry.creation_time = base::Time::Now();
     entry.title = base::SysNSStringToUTF16(title);
     entry.url = GURL(base::SysNSStringToUTF8(url));
     entry.path = path_elements;
     entry.in_toolbar = is_in_toolbar;
 
     out_bookmarks->push_back(entry);
   }
 }
 
 void SafariImporter::ParseBookmarks(
+    const string16& toolbar_name,
     std::vector<ProfileWriter::BookmarkEntry>* bookmarks) {
   DCHECK(bookmarks);
 
   // Construct ~/Library/Safari/Bookmarks.plist path
   NSString* library_dir = [NSString
       stringWithUTF8String:library_dir_.value().c_str()];
   NSString* safari_dir = [library_dir
       stringByAppendingPathComponent:@"Safari"];
   NSString* bookmarks_plist = [safari_dir
     stringByAppendingPathComponent:@"Bookmarks.plist"];
 
   // Load the plist file.
   NSDictionary* bookmarks_dict = [NSDictionary
       dictionaryWithContentsOfFile:bookmarks_plist];
   if (!bookmarks_dict)
     return;
 
   // Recursively read in bookmarks.
   std::vector<string16> parent_path_elements;
   RecursiveReadBookmarksFolder(bookmarks_dict, parent_path_elements, false,
-                               bookmarks);
+                               toolbar_name, bookmarks);
 }
 
 void SafariImporter::ImportPasswords() {
   // Safari stores it's passwords in the Keychain, same as us so we don't need
   // to import them.
   // Note: that we don't automatically pick them up, there is some logic around
   // the user needing to explicitly input his username in a page and blurring
   // the field before we pick it up, but the details of that are beyond the
   // scope of this comment.
 }
@@ skipping 71 matching lines @@
     if (!last_visit_str)
       continue;
 
     // Convert Safari's last visit time to Unix Epoch time.
     double seconds_since_unix_epoch = HistoryTimeToEpochTime(last_visit_str);
     row.set_last_visit(base::Time::FromDoubleT(seconds_since_unix_epoch));
 
     history_items->push_back(row);
   }
 }