summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authorisherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-05-26 18:38:05 +0000
committerisherman@chromium.org <isherman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-05-26 18:38:05 +0000
commit7fc7241105ea8e5532005611b374736a4e0bcba2 (patch)
tree3f0e3a2d0e70d6241c376815e71b1273b72ef461 /chrome
parentb6f8e3831bc67964a85de79a62ab6d680e89a2b8 (diff)
downloadchromium_src-7fc7241105ea8e5532005611b374736a4e0bcba2.zip
chromium_src-7fc7241105ea8e5532005611b374736a4e0bcba2.tar.gz
chromium_src-7fc7241105ea8e5532005611b374736a4e0bcba2.tar.bz2
Many fixes to bookmark importing.
In particular: * All bookmarks are imported to the toolbar -- nothing goes to Other Bookmarks. If there are initially no bookmarks on the toolbar, we try to reduce nesting of the imported bookmarks as much as possible. If there are already bookmarks on the toolbar, all the imported bookmarks end up in a new folder -- e.g. "Imported from Safari" -- on the toolbar. * All importers explicitly include a containing folder for bookmarks in the toolbar. o The ProfileWriter is responsible for stripping this folder off when the bookmarks should be imported directly to the toolbar. * All importers do *not* include a containing folder for the remaining bookmarks. o The ProfileWriter is responsible for creating this folder as appropriate. In fact, this is how things used to work previously, too, since the folder name needed to be uniquified. This CL makes the logic much clearer though (I hope). * All importers should now be able to handle importing empty folders. * The ProfileWriter no longer takes in a bitset of options for importing bookmarks. These options were all either set identically by all clients, or could be more accurately computed locally. * Some implementation details for ProfileWriter have been removed from the header file. Others have just been completely nuked from orbit, and replaced by simpler code (again, I hope). BUG=79427,79433,71351 TEST=unit_tests --gtest_filter=*Import* Review URL: http://codereview.chromium.org/6979007 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86861 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r--chrome/browser/bookmarks/bookmark_html_writer_unittest.cc20
-rw-r--r--chrome/browser/importer/external_process_importer_bridge.cc6
-rw-r--r--chrome/browser/importer/external_process_importer_bridge.h3
-rw-r--r--chrome/browser/importer/external_process_importer_client.cc19
-rw-r--r--chrome/browser/importer/external_process_importer_client.h9
-rw-r--r--chrome/browser/importer/external_process_importer_host.cc10
-rw-r--r--chrome/browser/importer/external_process_importer_host.h3
-rw-r--r--chrome/browser/importer/firefox2_importer.cc53
-rw-r--r--chrome/browser/importer/firefox2_importer.h2
-rw-r--r--chrome/browser/importer/firefox3_importer.cc48
-rw-r--r--chrome/browser/importer/firefox_importer_unittest.cc90
-rw-r--r--chrome/browser/importer/ie_importer.cc19
-rw-r--r--chrome/browser/importer/importer.cc5
-rw-r--r--chrome/browser/importer/importer.h19
-rw-r--r--chrome/browser/importer/importer_bridge.h3
-rw-r--r--chrome/browser/importer/importer_host.cc11
-rw-r--r--chrome/browser/importer/importer_host.h3
-rw-r--r--chrome/browser/importer/importer_unittest.cc37
-rw-r--r--chrome/browser/importer/in_process_importer_bridge.cc5
-rw-r--r--chrome/browser/importer/in_process_importer_bridge.h3
-rw-r--r--chrome/browser/importer/profile_import_process_client.cc1
-rw-r--r--chrome/browser/importer/profile_import_process_client.h1
-rw-r--r--chrome/browser/importer/profile_import_process_host.cc10
-rw-r--r--chrome/browser/importer/profile_import_process_host.h6
-rw-r--r--chrome/browser/importer/profile_import_process_messages.h8
-rw-r--r--chrome/browser/importer/profile_writer.cc239
-rw-r--r--chrome/browser/importer/profile_writer.h68
-rw-r--r--chrome/browser/importer/safari_importer.h4
-rw-r--r--chrome/browser/importer/safari_importer.mm61
-rw-r--r--chrome/browser/importer/safari_importer_unittest.mm81
-rw-r--r--chrome/browser/importer/toolbar_importer.cc4
-rw-r--r--chrome/profile_import/profile_import_thread.cc9
-rw-r--r--chrome/profile_import/profile_import_thread.h6
-rw-r--r--chrome/test/data/safari_import/Safari/Bookmarks.plistbin1296 -> 1645 bytes
34 files changed, 362 insertions, 504 deletions
diff --git a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
index e696aac..2f80465 100644
--- a/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_html_writer_unittest.cc
@@ -50,14 +50,13 @@ class BookmarkHTMLWriterTest : public TestingBrowserProcessTest {
}
// Converts a BookmarkEntry to a string suitable for assertion testing.
- string16 BookmarkEntryToString(
- const ProfileWriter::BookmarkEntry& entry) {
+ string16 BookmarkEntryToString(const ProfileWriter::BookmarkEntry& entry) {
string16 result;
result.append(ASCIIToUTF16("on_toolbar="));
if (entry.in_toolbar)
- result.append(ASCIIToUTF16("false"));
- else
result.append(ASCIIToUTF16("true"));
+ else
+ result.append(ASCIIToUTF16("false"));
result.append(ASCIIToUTF16(" url=") + UTF8ToUTF16(entry.url.spec()));
@@ -87,9 +86,6 @@ class BookmarkHTMLWriterTest : public TestingBrowserProcessTest {
ProfileWriter::BookmarkEntry entry;
entry.in_toolbar = on_toolbar;
entry.url = url;
- // The first path element should always be 'x', as that is what we passed
- // to the importer.
- entry.path.push_back(ASCIIToUTF16("x"));
if (!f1.empty()) {
entry.path.push_back(f1);
if (!f2.empty()) {
@@ -225,8 +221,6 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
std::vector<history::ImportedFaviconUsage> favicons;
Firefox2Importer::ImportBookmarksFile(path_,
std::set<GURL>(),
- false,
- ASCIIToUTF16("x"),
NULL,
&parsed_bookmarks,
NULL,
@@ -249,13 +243,13 @@ TEST_F(BookmarkHTMLWriterTest, Test) {
// Windows and ChromeOS builds use Sentence case.
string16 bookmark_folder_name =
l10n_util::GetStringUTF16(IDS_BOOMARK_BAR_FOLDER_NAME);
- AssertBookmarkEntryEquals(parsed_bookmarks[0], false, url1, url1_title, t1,
+ AssertBookmarkEntryEquals(parsed_bookmarks[0], true, url1, url1_title, t1,
bookmark_folder_name, f1_title, string16());
- AssertBookmarkEntryEquals(parsed_bookmarks[1], false, url2, url2_title, t2,
+ AssertBookmarkEntryEquals(parsed_bookmarks[1], true, url2, url2_title, t2,
bookmark_folder_name, f1_title, f2_title);
- AssertBookmarkEntryEquals(parsed_bookmarks[2], false, url3, url3_title, t3,
+ AssertBookmarkEntryEquals(parsed_bookmarks[2], true, url3, url3_title, t3,
bookmark_folder_name, string16(), string16());
- AssertBookmarkEntryEquals(parsed_bookmarks[3], false, url4, url4_title, t4,
+ AssertBookmarkEntryEquals(parsed_bookmarks[3], true, url4, url4_title, t4,
bookmark_folder_name, string16(), string16());
AssertBookmarkEntryEquals(parsed_bookmarks[4], false, url1, url1_title, t1,
string16(), string16(), string16());
diff --git a/chrome/browser/importer/external_process_importer_bridge.cc b/chrome/browser/importer/external_process_importer_bridge.cc
index 14fca1a..59d30ac 100644
--- a/chrome/browser/importer/external_process_importer_bridge.cc
+++ b/chrome/browser/importer/external_process_importer_bridge.cc
@@ -27,11 +27,9 @@ ExternalProcessImporterBridge::ExternalProcessImporterBridge(
void ExternalProcessImporterBridge::AddBookmarks(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) {
+ const string16& first_folder_name) {
profile_import_thread_->NotifyBookmarksImportReady(bookmarks,
- first_folder_name,
- options);
+ first_folder_name);
}
void ExternalProcessImporterBridge::AddHomePage(const GURL& home_page) {
diff --git a/chrome/browser/importer/external_process_importer_bridge.h b/chrome/browser/importer/external_process_importer_bridge.h
index 5d90697..2aaa244 100644
--- a/chrome/browser/importer/external_process_importer_bridge.h
+++ b/chrome/browser/importer/external_process_importer_bridge.h
@@ -31,8 +31,7 @@ class ExternalProcessImporterBridge : public ImporterBridge {
// Begin ImporterBridge implementation:
virtual void AddBookmarks(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) OVERRIDE;
+ const string16& first_folder_name) OVERRIDE;
virtual void AddHomePage(const GURL& home_page) OVERRIDE;
diff --git a/chrome/browser/importer/external_process_importer_client.cc b/chrome/browser/importer/external_process_importer_client.cc
index 1b32501..858665f 100644
--- a/chrome/browser/importer/external_process_importer_client.cc
+++ b/chrome/browser/importer/external_process_importer_client.cc
@@ -18,17 +18,14 @@ ExternalProcessImporterClient::ExternalProcessImporterClient(
ExternalProcessImporterHost* importer_host,
const importer::SourceProfile& source_profile,
uint16 items,
- InProcessImporterBridge* bridge,
- bool import_to_bookmark_bar)
- : bookmarks_options_(0),
- total_bookmarks_count_(0),
+ InProcessImporterBridge* bridge)
+ : total_bookmarks_count_(0),
total_history_rows_count_(0),
total_favicons_count_(0),
process_importer_host_(importer_host),
profile_import_process_host_(NULL),
source_profile_(source_profile),
items_(items),
- import_to_bookmark_bar_(import_to_bookmark_bar),
bridge_(bridge),
cancelled_(false) {
bridge_->AddRef();
@@ -73,8 +70,8 @@ void ExternalProcessImporterClient::StartProcessOnIOThread(
BrowserThread::ID thread_id) {
profile_import_process_host_ =
new ProfileImportProcessHost(this, thread_id);
- profile_import_process_host_->StartProfileImportProcess(
- source_profile_, items_, import_to_bookmark_bar_);
+ profile_import_process_host_->StartProfileImportProcess(source_profile_,
+ items_);
}
void ExternalProcessImporterClient::Cancel() {
@@ -168,13 +165,11 @@ void ExternalProcessImporterClient::OnHomePageImportReady(
void ExternalProcessImporterClient::OnBookmarksImportStart(
const string16& first_folder_name,
- int options,
size_t total_bookmarks_count) {
if (cancelled_)
return;
bookmarks_first_folder_name_ = first_folder_name;
- bookmarks_options_ = options;
total_bookmarks_count_ = total_bookmarks_count;
bookmarks_.reserve(total_bookmarks_count);
}
@@ -188,10 +183,8 @@ void ExternalProcessImporterClient::OnBookmarksImportGroup(
// total_bookmarks_count_:
bookmarks_.insert(bookmarks_.end(), bookmarks_group.begin(),
bookmarks_group.end());
- if (bookmarks_.size() == total_bookmarks_count_) {
- bridge_->AddBookmarks(bookmarks_, bookmarks_first_folder_name_,
- bookmarks_options_);
- }
+ if (bookmarks_.size() == total_bookmarks_count_)
+ bridge_->AddBookmarks(bookmarks_, bookmarks_first_folder_name_);
}
void ExternalProcessImporterClient::OnFaviconsImportStart(
diff --git a/chrome/browser/importer/external_process_importer_client.h b/chrome/browser/importer/external_process_importer_client.h
index 69abb98..2e5bdd5 100644
--- a/chrome/browser/importer/external_process_importer_client.h
+++ b/chrome/browser/importer/external_process_importer_client.h
@@ -34,8 +34,7 @@ class ExternalProcessImporterClient : public ProfileImportProcessClient {
ExternalProcessImporterClient(ExternalProcessImporterHost* importer_host,
const importer::SourceProfile& source_profile,
uint16 items,
- InProcessImporterBridge* bridge,
- bool import_to_bookmark_bar);
+ InProcessImporterBridge* bridge);
virtual ~ExternalProcessImporterClient();
// Cancel import process on IO thread.
@@ -78,10 +77,8 @@ class ExternalProcessImporterClient : public ProfileImportProcessClient {
// First message received when importing bookmarks.
// |first_folder_name| can be NULL.
- // |options| is described in ProfileWriter::BookmarkOptions.
// |total_bookmarks_count| is the total number of bookmarks to be imported.
virtual void OnBookmarksImportStart(const string16& first_folder_name,
- int options,
size_t total_bookmarks_count) OVERRIDE;
// Called when a group of bookmarks has been received.
@@ -121,9 +118,6 @@ class ExternalProcessImporterClient : public ProfileImportProcessClient {
// under which imported bookmarks will be placed.
string16 bookmarks_first_folder_name_;
- // Determines how bookmarks should be added (ProfileWriter::BookmarkOptions).
- int bookmarks_options_;
-
// Total number of bookmarks to import.
size_t total_bookmarks_count_;
@@ -146,7 +140,6 @@ class ExternalProcessImporterClient : public ProfileImportProcessClient {
// Data to be passed from the importer host to the external importer.
const importer::SourceProfile& source_profile_;
uint16 items_;
- bool import_to_bookmark_bar_;
// Takes import data coming over IPC and delivers it to be written by the
// ProfileWriter. Released by ExternalProcessImporterClient in its
diff --git a/chrome/browser/importer/external_process_importer_host.cc b/chrome/browser/importer/external_process_importer_host.cc
index 5560405..0891e34 100644
--- a/chrome/browser/importer/external_process_importer_host.cc
+++ b/chrome/browser/importer/external_process_importer_host.cc
@@ -10,7 +10,6 @@
ExternalProcessImporterHost::ExternalProcessImporterHost()
: items_(0),
- import_to_bookmark_bar_(false),
cancelled_(false),
import_process_launched_(false) {
}
@@ -36,7 +35,6 @@ void ExternalProcessImporterHost::StartImportSettings(
ImporterHost::AddRef(); // Balanced in ImporterHost::NotifyImportEnded.
- import_to_bookmark_bar_ = ShouldImportToBookmarkBar(first_run);
CheckForFirefoxLock(source_profile, items, first_run);
CheckForLoadedModels(items);
@@ -55,8 +53,8 @@ void ExternalProcessImporterHost::InvokeTaskIfDone() {
// and will delete it.
InProcessImporterBridge* bridge =
new InProcessImporterBridge(writer_.get(), this);
- client_ = new ExternalProcessImporterClient(
- this, *source_profile_, items_, bridge, import_to_bookmark_bar_);
+ client_ = new ExternalProcessImporterClient(this, *source_profile_, items_,
+ bridge);
import_process_launched_ = true;
client_->Start();
}
@@ -67,9 +65,5 @@ void ExternalProcessImporterHost::Loaded(BookmarkModel* model) {
waiting_for_bookmarkbar_model_ = false;
installed_bookmark_observer_ = false;
- // Because the import process is running externally, the decision whether
- // to import to the bookmark bar must be stored here so that it can be
- // passed to the importer when the import task is invoked.
- import_to_bookmark_bar_ = (!model->HasBookmarks());
InvokeTaskIfDone();
}
diff --git a/chrome/browser/importer/external_process_importer_host.h b/chrome/browser/importer/external_process_importer_host.h
index 3184a5d..6bbd0e3 100644
--- a/chrome/browser/importer/external_process_importer_host.h
+++ b/chrome/browser/importer/external_process_importer_host.h
@@ -47,9 +47,6 @@ class ExternalProcessImporterHost : public ImporterHost {
// Bitmask of items to be imported (see importer::ImportItem enum).
uint16 items_;
- // Whether to import bookmarks to the bookmark bar.
- bool import_to_bookmark_bar_;
-
// True if the import process has been cancelled.
bool cancelled_;
diff --git a/chrome/browser/importer/firefox2_importer.cc b/chrome/browser/importer/firefox2_importer.cc
index b14105a..0e1899e6 100644
--- a/chrome/browser/importer/firefox2_importer.cc
+++ b/chrome/browser/importer/firefox2_importer.cc
@@ -148,8 +148,6 @@ TemplateURL* Firefox2Importer::CreateTemplateURL(const string16& title,
void Firefox2Importer::ImportBookmarksFile(
const FilePath& file_path,
const std::set<GURL>& default_urls,
- bool import_to_bookmark_bar,
- const string16& first_folder_name,
Importer* importer,
std::vector<ProfileWriter::BookmarkEntry>* bookmarks,
std::vector<TemplateURL*>* template_urls,
@@ -160,7 +158,7 @@ void Firefox2Importer::ImportBookmarksFile(
base::SplitString(content, '\n', &lines);
std::vector<ProfileWriter::BookmarkEntry> toolbar_bookmarks;
- string16 last_folder = first_folder_name;
+ string16 last_folder;
bool last_folder_on_toolbar = false;
bool last_folder_is_empty = true;
bool has_subfolder = false;
@@ -214,22 +212,18 @@ void Firefox2Importer::ImportBookmarksFile(
entry.url = url;
entry.title = title;
- if (import_to_bookmark_bar && toolbar_folder) {
- // Flatten the items in toolbar.
+ if (toolbar_folder) {
+ // The toolbar folder should be at the top level.
entry.in_toolbar = true;
- entry.path.assign(path.begin() + toolbar_folder, path.end());
+ entry.path.assign(path.begin() + toolbar_folder - 1, path.end());
toolbar_bookmarks.push_back(entry);
} else {
- // Insert the item into the "Imported from Firefox" folder.
+ // Add this bookmark to the list of |bookmarks|.
if (!has_subfolder && !last_folder.empty()) {
path.push_back(last_folder);
last_folder.clear();
}
entry.path.assign(path.begin(), path.end());
- if (import_to_bookmark_bar) {
- DCHECK(!entry.path.empty());
- entry.path.erase(entry.path.begin());
- }
bookmarks->push_back(entry);
}
@@ -252,7 +246,7 @@ void Firefox2Importer::ImportBookmarksFile(
// Bookmarks in sub-folder are encapsulated with <DL> tag.
if (StartsWithASCII(line, "<DL>", false)) {
has_subfolder = true;
- if (last_folder.length() != 0) {
+ if (!last_folder.empty()) {
path.push_back(last_folder);
last_folder.clear();
}
@@ -274,18 +268,17 @@ void Firefox2Importer::ImportBookmarksFile(
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);
+ if (toolbar_folder) {
+ // The toolbar folder should be at the top level.
+ // Make sure we don't add the toolbar folder itself if it is empty.
+ if (toolbar_folder <= path.size()) {
+ entry.in_toolbar = true;
+ entry.path.assign(path.begin() + toolbar_folder - 1, path.end());
+ toolbar_bookmarks.push_back(entry);
+ }
} else {
- // Insert the folder into the "Imported from Firefox" folder.
+ // Add this folder to the list of |bookmarks|.
entry.path.assign(path.begin(), path.end());
- if (import_to_bookmark_bar) {
- DCHECK(!entry.path.empty());
- entry.path.erase(entry.path.begin());
- }
bookmarks->push_back(entry);
}
@@ -315,22 +308,16 @@ void Firefox2Importer::ImportBookmarks() {
FilePath file = source_path_;
if (!parsing_bookmarks_html_file_)
file = file.AppendASCII("bookmarks.html");
- string16 first_folder_name = bridge_->GetLocalizedString(
- parsing_bookmarks_html_file_ ? IDS_BOOKMARK_GROUP :
- IDS_BOOKMARK_GROUP_FROM_FIREFOX);
- ImportBookmarksFile(file, default_urls, import_to_bookmark_bar(),
- first_folder_name, this, &bookmarks, &template_urls,
+ ImportBookmarksFile(file, default_urls, this, &bookmarks, &template_urls,
&favicons);
// Write data into profile.
if (!bookmarks.empty() && !cancelled()) {
- int options = 0;
- if (import_to_bookmark_bar())
- options |= ProfileWriter::IMPORT_TO_BOOKMARK_BAR;
- if (bookmark_bar_disabled())
- options |= ProfileWriter::BOOKMARK_BAR_DISABLED;
- bridge_->AddBookmarks(bookmarks, first_folder_name, options);
+ 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()) {
diff --git a/chrome/browser/importer/firefox2_importer.h b/chrome/browser/importer/firefox2_importer.h
index 86b68c2..62fcac3 100644
--- a/chrome/browser/importer/firefox2_importer.h
+++ b/chrome/browser/importer/firefox2_importer.h
@@ -46,8 +46,6 @@ class Firefox2Importer : public Importer {
static void ImportBookmarksFile(
const FilePath& file_path,
const std::set<GURL>& default_urls,
- bool import_to_bookmark_bar,
- const string16& first_folder_name,
Importer* importer,
std::vector<ProfileWriter::BookmarkEntry>* bookmarks,
std::vector<TemplateURL*>* template_urls,
diff --git a/chrome/browser/importer/firefox3_importer.cc b/chrome/browser/importer/firefox3_importer.cc
index 606549f..0411dc7 100644
--- a/chrome/browser/importer/firefox3_importer.cc
+++ b/chrome/browser/importer/firefox3_importer.cc
@@ -208,8 +208,8 @@ void Firefox3Importer::ImportBookmarks() {
BookmarkItem* item = list[i];
if (item->type == TYPE_FOLDER) {
- // Folders are added implicitly on adding children,
- // so now we pass only empty folders to add them explicitly.
+ // Folders are added implicitly on adding children, so we only explicitly
+ // add empty folders.
if (!item->empty_folder)
continue;
} else if (item->type == TYPE_BOOKMARK) {
@@ -232,31 +232,28 @@ void Firefox3Importer::ImportBookmarks() {
bool is_in_toolbar = false;
while (child->parent >= 0) {
BookmarkItem* parent = list[child->parent];
- if (parent->id == toolbar_folder_id) {
- // This bookmark entry should be put in the bookmark bar.
- // But we put it in the Firefox group after first run, so
- // that do not mess up the bookmark bar.
- if (import_to_bookmark_bar()) {
- is_in_toolbar = true;
- } else {
- path.insert(path.begin(), parent->title);
- path.insert(path.begin(), firefox_folder);
- }
- found_path = true;
+ if (livemark_id.find(parent->id) != livemark_id.end()) {
+ // Don't import live bookmarks.
break;
- } else if (parent->id == menu_folder_id ||
- parent->id == unsorted_folder_id) {
- // After the first run, the item will be placed in a folder in
- // the "Other bookmarks".
- if (!import_to_bookmark_bar())
- path.insert(path.begin(), firefox_folder);
+ }
+
+ if (parent->id != menu_folder_id) {
+ // To avoid excessive nesting, omit the name for the bookmarks menu
+ // folder.
+ path.insert(path.begin(), parent->title);
+ }
+
+ if (parent->id == toolbar_folder_id)
+ is_in_toolbar = true;
+
+ if (parent->id == toolbar_folder_id ||
+ parent->id == menu_folder_id ||
+ parent->id == unsorted_folder_id) {
+ // We've reached a root node, hooray!
found_path = true;
break;
- } else if (livemark_id.find(parent->id) != livemark_id.end()) {
- // If the entry is under a livemark folder, we don't import it.
- break;
}
- path.insert(path.begin(), parent->title);
+
child = parent;
}
@@ -291,10 +288,7 @@ void Firefox3Importer::ImportBookmarks() {
if (!bookmarks.empty() && !cancelled()) {
const string16& first_folder_name =
bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP_FROM_FIREFOX);
- 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);
}
if (!template_urls.empty() && !cancelled()) {
bridge_->SetKeywords(template_urls, -1, false);
diff --git a/chrome/browser/importer/firefox_importer_unittest.cc b/chrome/browser/importer/firefox_importer_unittest.cc
index 4d2179f..5788dac 100644
--- a/chrome/browser/importer/firefox_importer_unittest.cc
+++ b/chrome/browser/importer/firefox_importer_unittest.cc
@@ -192,14 +192,12 @@ TEST(FirefoxImporterTest, Firefox2BookmarkFileImport) {
// 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;
- string16 first_folder_name = ASCIIToUTF16("xyzzy");
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()));
+ importer->ImportBookmarksFile(empty_folder_path, default_urls,
+ importer, &bookmarks, NULL, NULL);
+ EXPECT_EQ(3U, bookmarks.size());
std::vector<ProfileWriter::BookmarkEntry>::iterator it;
ProfileWriter::BookmarkEntry entry;
std::vector<string16>::iterator path_it;
@@ -209,68 +207,52 @@ TEST(FirefoxImporterTest, Firefox2BookmarkFileImport) {
EXPECT_EQ(ASCIIToUTF16("Empty"), entry.title);
EXPECT_TRUE(entry.is_folder);
EXPECT_EQ(base::Time::FromTimeT(1295938143), entry.creation_time);
- EXPECT_EQ(2, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 2) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it++);
- EXPECT_EQ(ASCIIToUTF16("Empty's Parent"), *path_it);
- }
+ EXPECT_EQ(1U, entry.path.size());
+ if (entry.path.size() == 1)
+ EXPECT_EQ(ASCIIToUTF16("Empty's Parent"), entry.path.front());
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("[Tamura Yukari.com]"), entry.title);
EXPECT_FALSE(entry.is_folder);
EXPECT_EQ(base::Time::FromTimeT(1234567890), entry.creation_time);
- EXPECT_EQ(2, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 2) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it++);
- EXPECT_EQ(ASCIIToUTF16("Not Empty"), *path_it);
- }
+ EXPECT_EQ(1U, entry.path.size());
+ if (entry.path.size() == 1)
+ EXPECT_EQ(ASCIIToUTF16("Not Empty"), entry.path.front());
EXPECT_EQ("http://www.tamurayukari.com/", entry.url.spec());
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("Google"), entry.title);
EXPECT_FALSE(entry.is_folder);
EXPECT_EQ(base::Time::FromTimeT(0000000000), entry.creation_time);
- EXPECT_EQ(2, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 2) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it++);
- EXPECT_EQ(ASCIIToUTF16("Not Empty But Default"), *path_it);
- }
+ EXPECT_EQ(1U, entry.path.size());
+ if (entry.path.size() == 1)
+ EXPECT_EQ(ASCIIToUTF16("Not Empty But Default"), entry.path.front());
EXPECT_EQ("http://www.google.com/", entry.url.spec());
}
// 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()));
+ importer->ImportBookmarksFile(empty_folder_path, default_urls,
+ importer, &bookmarks, NULL, NULL);
+ EXPECT_EQ(2U, bookmarks.size());
if (bookmarks.size() == 2) {
it = bookmarks.begin();
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("Empty"), entry.title);
EXPECT_TRUE(entry.is_folder);
EXPECT_EQ(base::Time::FromTimeT(1295938143), entry.creation_time);
- EXPECT_EQ(2, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 2) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it++);
- EXPECT_EQ(ASCIIToUTF16("Empty's Parent"), *path_it);
- }
+ EXPECT_EQ(1U, entry.path.size());
+ if (entry.path.size() == 1)
+ EXPECT_EQ(ASCIIToUTF16("Empty's Parent"), entry.path.front());
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("[Tamura Yukari.com]"), entry.title);
EXPECT_FALSE(entry.is_folder);
EXPECT_EQ(base::Time::FromTimeT(1234567890), entry.creation_time);
- EXPECT_EQ(2, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 2) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it++);
- EXPECT_EQ(ASCIIToUTF16("Not Empty"), *path_it);
- }
+ EXPECT_EQ(1U, entry.path.size());
+ if (entry.path.size() == 1)
+ EXPECT_EQ(ASCIIToUTF16("Not Empty"), entry.path.front());
EXPECT_EQ("http://www.tamurayukari.com/", entry.url.spec());
}
@@ -278,47 +260,37 @@ TEST(FirefoxImporterTest, Firefox2BookmarkFileImport) {
FilePath epiphany_path = path.AppendASCII("epiphany.html");
bookmarks.clear();
default_urls.clear();
- importer->ImportBookmarksFile(epiphany_path, default_urls, false,
- first_folder_name, importer, &bookmarks,
- NULL, NULL);
- EXPECT_EQ(2, static_cast<int>(bookmarks.size()));
+ importer->ImportBookmarksFile(epiphany_path, default_urls,
+ importer, &bookmarks, NULL, NULL);
+ EXPECT_EQ(2U, bookmarks.size());
if (bookmarks.size() == 2) {
it = bookmarks.begin();
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("[Tamura Yukari.com]"), entry.title);
EXPECT_EQ("http://www.tamurayukari.com/", entry.url.spec());
- EXPECT_EQ(1, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 1) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it);
- }
+ EXPECT_EQ(0U, entry.path.size());
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("Google"), entry.title);
EXPECT_EQ("http://www.google.com/", entry.url.spec());
- EXPECT_EQ(1, static_cast<int>(entry.path.size()));
- if (entry.path.size() == 1) {
- path_it = entry.path.begin();
- EXPECT_EQ(ASCIIToUTF16("xyzzy"), *path_it);
- }
+ EXPECT_EQ(0U, entry.path.size());
}
// Import Epiphany bookmarks from a file to bookmark bar.
bookmarks.clear();
default_urls.clear();
- importer->ImportBookmarksFile(epiphany_path, default_urls, true,
- first_folder_name, importer, &bookmarks,
- NULL, NULL);
- EXPECT_EQ(2, static_cast<int>(bookmarks.size()));
+ importer->ImportBookmarksFile(epiphany_path, default_urls,
+ importer, &bookmarks, NULL, NULL);
+ EXPECT_EQ(2U, bookmarks.size());
if (bookmarks.size() == 2) {
it = bookmarks.begin();
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("[Tamura Yukari.com]"), entry.title);
EXPECT_EQ("http://www.tamurayukari.com/", entry.url.spec());
- EXPECT_EQ(0, static_cast<int>(entry.path.size()));
+ EXPECT_EQ(0U, entry.path.size());
entry = *it++;
EXPECT_EQ(ASCIIToUTF16("Google"), entry.title);
EXPECT_EQ("http://www.google.com/", entry.url.spec());
- EXPECT_EQ(0, static_cast<int>(entry.path.size()));
+ EXPECT_EQ(0U, entry.path.size());
}
importer->Release();
diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc
index dbecbeb..eb242a7 100644
--- a/chrome/browser/importer/ie_importer.cc
+++ b/chrome/browser/importer/ie_importer.cc
@@ -137,10 +137,7 @@ void IEImporter::ImportFavorites() {
if (!bookmarks.empty() && !cancelled()) {
const string16& first_folder_name =
l10n_util::GetStringUTF16(IDS_BOOKMARK_GROUP_FROM_IE);
- 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);
}
}
@@ -529,8 +526,6 @@ bool IEImporter::GetFavoritesInfo(IEImporter::FavoritesInfo* info) {
void IEImporter::ParseFavoritesFolder(const FavoritesInfo& info,
BookmarkVector* bookmarks) {
- std::wstring ie_folder =
- UTF16ToWide(l10n_util::GetStringUTF16(IDS_BOOKMARK_GROUP_FROM_IE));
BookmarkVector toolbar_bookmarks;
FilePath file;
std::vector<FilePath::StringType> file_list;
@@ -575,18 +570,12 @@ void IEImporter::ParseFavoritesFolder(const FavoritesInfo& info,
if (!relative_path.empty())
relative_path.GetComponents(&entry.path);
- // Flatten the bookmarks in Link folder onto bookmark toolbar. Otherwise,
- // put it into "Other bookmarks".
- if (import_to_bookmark_bar() &&
- (!entry.path.empty() && entry.path[0] == info.links_folder)) {
+ // Add the bookmark.
+ if (!entry.path.empty() && entry.path[0] == info.links_folder) {
+ // Bookmarks in the Link folder should be imported to the toolbar.
entry.in_toolbar = true;
- entry.path.erase(entry.path.begin());
toolbar_bookmarks.push_back(entry);
} else {
- // We put the bookmarks in a "Imported From IE"
- // folder, so that we don't mess up the "Other bookmarks".
- if (!import_to_bookmark_bar())
- entry.path.insert(entry.path.begin(), ie_folder);
bookmarks->push_back(entry);
}
}
diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc
index 7bbc3df..68a8fe9 100644
--- a/chrome/browser/importer/importer.cc
+++ b/chrome/browser/importer/importer.cc
@@ -16,10 +16,7 @@ void Importer::Cancel() {
cancelled_ = true;
}
-Importer::Importer()
- : import_to_bookmark_bar_(false),
- bookmark_bar_disabled_(false),
- cancelled_(false) {
+Importer::Importer() : cancelled_(false) {
}
Importer::~Importer() {
diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h
index 9b86109..b4a5b6e 100644
--- a/chrome/browser/importer/importer.h
+++ b/chrome/browser/importer/importer.h
@@ -31,15 +31,6 @@ class Importer : public base::RefCountedThreadSafe<Importer> {
// Cancels the import process.
virtual void Cancel();
- void set_import_to_bookmark_bar(bool import_to_bookmark_bar) {
- import_to_bookmark_bar_ = import_to_bookmark_bar;
- }
-
- void set_bookmark_bar_disabled(bool bookmark_bar_disabled) {
- bookmark_bar_disabled_ = bookmark_bar_disabled;
- }
-
- bool bookmark_bar_disabled() { return bookmark_bar_disabled_; }
bool cancelled() const { return cancelled_; }
protected:
@@ -55,19 +46,9 @@ class Importer : public base::RefCountedThreadSafe<Importer> {
size_t src_len,
std::vector<unsigned char>* png_data);
- bool import_to_bookmark_bar() const { return import_to_bookmark_bar_; }
-
scoped_refptr<ImporterBridge> bridge_;
private:
- // True if the importer is created in the first run UI.
- bool import_to_bookmark_bar_;
-
- // Whether bookmark bar is disabled (not shown) for importer. This is set
- // true during first run to prevent out of process bookmark importer from
- // updating bookmark bar settings.
- bool bookmark_bar_disabled_;
-
// True if the caller cancels the import process.
bool cancelled_;
diff --git a/chrome/browser/importer/importer_bridge.h b/chrome/browser/importer/importer_bridge.h
index f28f33f..cc60703 100644
--- a/chrome/browser/importer/importer_bridge.h
+++ b/chrome/browser/importer/importer_bridge.h
@@ -24,8 +24,7 @@ class ImporterBridge : public base::RefCountedThreadSafe<ImporterBridge> {
virtual void AddBookmarks(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) = 0;
+ const string16& first_folder_name) = 0;
virtual void AddHomePage(const GURL& home_page) = 0;
diff --git a/chrome/browser/importer/importer_host.cc b/chrome/browser/importer/importer_host.cc
index c81094c..a012b8c 100644
--- a/chrome/browser/importer/importer_host.cc
+++ b/chrome/browser/importer/importer_host.cc
@@ -117,8 +117,6 @@ void ImporterHost::StartImportSettings(
}
importer_->AddRef();
- importer_->set_import_to_bookmark_bar(ShouldImportToBookmarkBar(first_run));
- importer_->set_bookmark_bar_disabled(first_run);
scoped_refptr<InProcessImporterBridge> bridge(
new InProcessImporterBridge(writer_.get(), this));
@@ -172,14 +170,6 @@ ImporterHost::~ImporterHost() {
}
}
-bool ImporterHost::ShouldImportToBookmarkBar(bool first_run) {
- bool import_to_bookmark_bar = first_run;
- if (profile_ && profile_->GetBookmarkModel()->IsLoaded()) {
- import_to_bookmark_bar = (!profile_->GetBookmarkModel()->HasBookmarks());
- }
- return import_to_bookmark_bar;
-}
-
void ImporterHost::CheckForFirefoxLock(
const importer::SourceProfile& source_profile,
uint16 items,
@@ -232,7 +222,6 @@ void ImporterHost::Loaded(BookmarkModel* model) {
waiting_for_bookmarkbar_model_ = false;
installed_bookmark_observer_ = false;
- importer_->set_import_to_bookmark_bar(!model->HasBookmarks());
InvokeTaskIfDone();
}
diff --git a/chrome/browser/importer/importer_host.h b/chrome/browser/importer/importer_host.h
index 90825ba..35f0fe1 100644
--- a/chrome/browser/importer/importer_host.h
+++ b/chrome/browser/importer/importer_host.h
@@ -83,9 +83,6 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
protected:
virtual ~ImporterHost();
- // Returns true if importer should import to bookmark bar.
- bool ShouldImportToBookmarkBar(bool first_run);
-
// Make sure that Firefox isn't running, if import browser is Firefox. Show
// to the user a dialog that notifies that is necessary to close Firefox
// prior to continue.
diff --git a/chrome/browser/importer/importer_unittest.cc b/chrome/browser/importer/importer_unittest.cc
index 424f21e..066c5ba 100644
--- a/chrome/browser/importer/importer_unittest.cc
+++ b/chrome/browser/importer/importer_unittest.cc
@@ -153,10 +153,10 @@ bool FindBookmarkEntry(const ProfileWriter::BookmarkEntry& entry,
#if defined(OS_WIN)
static const BookmarkList kIEBookmarks[] = {
- {true, 0, {},
+ {true, 1, {L"Links"},
L"TheLink",
"http://www.links-thelink.com/"},
- {true, 1, {L"SubFolderOfLinks"},
+ {true, 2, {L"Links", L"SubFolderOfLinks"},
L"SubLink",
"http://www.links-sublink.com/"},
{false, 0, {},
@@ -238,12 +238,11 @@ class TestObserver : public ProfileWriter,
EXPECT_EQ(history::SOURCE_IE_IMPORTED, visit_source);
}
- virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmark,
- const string16& first_folder_name,
- int options) OVERRIDE {
+ virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
+ const string16& top_level_folder_name) OVERRIDE {
// Importer should import the IE Favorites folder the same as the list.
- for (size_t i = 0; i < bookmark.size(); ++i) {
- if (FindBookmarkEntry(bookmark[i], kIEBookmarks,
+ for (size_t i = 0; i < bookmarks.size(); ++i) {
+ if (FindBookmarkEntry(bookmarks[i], kIEBookmarks,
arraysize(kIEBookmarks)))
++bookmark_count_;
}
@@ -465,10 +464,10 @@ TEST_F(ImporterTest, IE7Importer) {
#endif // defined(OS_WIN)
static const BookmarkList kFirefox2Bookmarks[] = {
- {true, 1, {L"Folder"},
+ {true, 2, {L"Bookmarks Toolbar Folder", L"Folder"},
L"On Toolbar's Subfolder",
"http://on.toolbar/bookmark/folder"},
- {true, 0, {},
+ {true, 1, {L"Bookmarks Toolbar Folder"},
L"On Bookmark Toolbar",
"http://on.toolbar/bookmark"},
{false, 1, {L"Folder"},
@@ -604,11 +603,10 @@ class FirefoxObserver : public ProfileWriter,
++history_count_;
}
- virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmark,
- const string16& first_folder_name,
- int options) OVERRIDE {
- for (size_t i = 0; i < bookmark.size(); ++i) {
- if (FindBookmarkEntry(bookmark[i], kFirefox2Bookmarks,
+ virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
+ const string16& top_level_folder_name) OVERRIDE {
+ for (size_t i = 0; i < bookmarks.size(); ++i) {
+ if (FindBookmarkEntry(bookmarks[i], kFirefox2Bookmarks,
arraysize(kFirefox2Bookmarks)))
++bookmark_count_;
}
@@ -701,7 +699,7 @@ TEST_F(ImporterTest, MAYBE(Firefox2Importer)) {
}
static const BookmarkList kFirefox3Bookmarks[] = {
- {true, 0, {},
+ {true, 1, {L"Bookmarks Toolbar"},
L"Toolbar",
"http://site/"},
{false, 0, {},
@@ -814,11 +812,10 @@ class Firefox3Observer : public ProfileWriter,
++history_count_;
}
- virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmark,
- const string16& first_folder_name,
- int options) OVERRIDE {
- for (size_t i = 0; i < bookmark.size(); ++i) {
- if (FindBookmarkEntry(bookmark[i], kFirefox3Bookmarks,
+ virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
+ const string16& top_level_folder_name) OVERRIDE {
+ for (size_t i = 0; i < bookmarks.size(); ++i) {
+ if (FindBookmarkEntry(bookmarks[i], kFirefox3Bookmarks,
arraysize(kFirefox3Bookmarks)))
++bookmark_count_;
}
diff --git a/chrome/browser/importer/in_process_importer_bridge.cc b/chrome/browser/importer/in_process_importer_bridge.cc
index 65cdb79..d20f5c8 100644
--- a/chrome/browser/importer/in_process_importer_bridge.cc
+++ b/chrome/browser/importer/in_process_importer_bridge.cc
@@ -22,12 +22,11 @@ InProcessImporterBridge::InProcessImporterBridge(ProfileWriter* writer,
void InProcessImporterBridge::AddBookmarks(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) {
+ const string16& first_folder_name) {
BrowserThread::PostTask(
BrowserThread::UI, FROM_HERE,
NewRunnableMethod(writer_, &ProfileWriter::AddBookmarks, bookmarks,
- first_folder_name, options));
+ first_folder_name));
}
void InProcessImporterBridge::AddHomePage(const GURL &home_page) {
diff --git a/chrome/browser/importer/in_process_importer_bridge.h b/chrome/browser/importer/in_process_importer_bridge.h
index 2cd042e..9311630 100644
--- a/chrome/browser/importer/in_process_importer_bridge.h
+++ b/chrome/browser/importer/in_process_importer_bridge.h
@@ -25,8 +25,7 @@ class InProcessImporterBridge : public ImporterBridge {
// Begin ImporterBridge implementation:
virtual void AddBookmarks(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) OVERRIDE;
+ const string16& first_folder_name) OVERRIDE;
virtual void AddHomePage(const GURL &home_page) OVERRIDE;
diff --git a/chrome/browser/importer/profile_import_process_client.cc b/chrome/browser/importer/profile_import_process_client.cc
index 21311f6..9e88bef 100644
--- a/chrome/browser/importer/profile_import_process_client.cc
+++ b/chrome/browser/importer/profile_import_process_client.cc
@@ -50,7 +50,6 @@ void ProfileImportProcessClient::OnHomePageImportReady(const GURL& home_page) {
void ProfileImportProcessClient::OnBookmarksImportStart(
const string16& first_folder_name,
- int options,
size_t total_bookmarks_count) {
}
diff --git a/chrome/browser/importer/profile_import_process_client.h b/chrome/browser/importer/profile_import_process_client.h
index 9607a4f..b87b701 100644
--- a/chrome/browser/importer/profile_import_process_client.h
+++ b/chrome/browser/importer/profile_import_process_client.h
@@ -57,7 +57,6 @@ class ProfileImportProcessClient
virtual void OnHomePageImportReady(const GURL& home_page);
virtual void OnBookmarksImportStart(const string16& first_folder_name,
- int options,
size_t total_bookmarks_count);
virtual void OnBookmarksImportGroup(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks);
diff --git a/chrome/browser/importer/profile_import_process_host.cc b/chrome/browser/importer/profile_import_process_host.cc
index 8df0a2f..82c7955 100644
--- a/chrome/browser/importer/profile_import_process_host.cc
+++ b/chrome/browser/importer/profile_import_process_host.cc
@@ -29,8 +29,7 @@ ProfileImportProcessHost::~ProfileImportProcessHost() {
bool ProfileImportProcessHost::StartProfileImportProcess(
const importer::SourceProfile& source_profile,
- uint16 items,
- bool import_to_bookmark_bar) {
+ uint16 items) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
if (!StartProcess())
return false;
@@ -53,9 +52,12 @@ bool ProfileImportProcessHost::StartProfileImportProcess(
localized_strings.SetString(
base::IntToString(IDS_IMPORT_FROM_SAFARI),
l10n_util::GetStringUTF8(IDS_IMPORT_FROM_SAFARI));
+ localized_strings.SetString(
+ base::IntToString(IDS_BOOMARK_BAR_FOLDER_NAME),
+ l10n_util::GetStringUTF8(IDS_BOOMARK_BAR_FOLDER_NAME));
- Send(new ProfileImportProcessMsg_StartImport(
- source_profile, items, localized_strings, import_to_bookmark_bar));
+ Send(new ProfileImportProcessMsg_StartImport(source_profile, items,
+ localized_strings));
return true;
}
diff --git a/chrome/browser/importer/profile_import_process_host.h b/chrome/browser/importer/profile_import_process_host.h
index d8656ab..9403b37 100644
--- a/chrome/browser/importer/profile_import_process_host.h
+++ b/chrome/browser/importer/profile_import_process_host.h
@@ -38,11 +38,9 @@ class ProfileImportProcessHost : public BrowserChildProcessHost {
BrowserThread::ID thread_id);
virtual ~ProfileImportProcessHost();
- // |source_profile|, |items|, and |import_to_bookmark_bar| are all needed by
- // the external importer process.
+ // |source_profile| and |items| are needed by the external importer process.
bool StartProfileImportProcess(const importer::SourceProfile& source_profile,
- uint16 items,
- bool import_to_bookmark_bar);
+ uint16 items);
// Cancel the external import process.
bool CancelProfileImportProcess();
diff --git a/chrome/browser/importer/profile_import_process_messages.h b/chrome/browser/importer/profile_import_process_messages.h
index 1fbf73d..d7d9048 100644
--- a/chrome/browser/importer/profile_import_process_messages.h
+++ b/chrome/browser/importer/profile_import_process_messages.h
@@ -373,11 +373,10 @@ struct ParamTraits<TemplateURL> {
//-----------------------------------------------------------------------------
// ProfileImportProcess messages
// These are messages sent from the browser to the profile import process.
-IPC_MESSAGE_CONTROL4(ProfileImportProcessMsg_StartImport,
+IPC_MESSAGE_CONTROL3(ProfileImportProcessMsg_StartImport,
importer::SourceProfile,
int /* Bitmask of items to import. */,
- DictionaryValue /* Localized strings. */,
- bool /* Import to bookmark bar. */)
+ DictionaryValue /* Localized strings. */)
IPC_MESSAGE_CONTROL0(ProfileImportProcessMsg_CancelImport)
@@ -414,9 +413,8 @@ IPC_MESSAGE_CONTROL2(ProfileImportProcessHostMsg_NotifyHistoryImportGroup,
IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyHomePageImportReady,
GURL /* GURL of home page */)
-IPC_MESSAGE_CONTROL3(ProfileImportProcessHostMsg_NotifyBookmarksImportStart,
+IPC_MESSAGE_CONTROL2(ProfileImportProcessHostMsg_NotifyBookmarksImportStart,
string16 /* first folder name */,
- int /* options */,
int /* total number of bookmarks */)
IPC_MESSAGE_CONTROL1(ProfileImportProcessHostMsg_NotifyBookmarksImportGroup,
diff --git a/chrome/browser/importer/profile_writer.cc b/chrome/browser/importer/profile_writer.cc
index 6f3d222..da63976 100644
--- a/chrome/browser/importer/profile_writer.cc
+++ b/chrome/browser/importer/profile_writer.cc
@@ -4,6 +4,9 @@
#include "chrome/browser/importer/profile_writer.h"
+#include <string>
+
+#include "base/string_number_conversions.h"
#include "base/stringprintf.h"
#include "base/threading/thread.h"
#include "base/utf_string_conversions.h"
@@ -16,6 +19,54 @@
#include "chrome/common/pref_names.h"
#include "content/common/notification_service.h"
+namespace {
+
+// Generates a unique folder name. If |folder_name| is not unique, then this
+// repeatedly tests for '|folder_name| + (i)' until a unique name is found.
+string16 GenerateUniqueFolderName(BookmarkModel* model,
+ const string16& folder_name) {
+ // Build a set containing the bookmark bar folder names.
+ std::set<string16> existing_folder_names;
+ const BookmarkNode* bookmark_bar = model->GetBookmarkBarNode();
+ for (int i = 0; i < bookmark_bar->child_count(); ++i) {
+ const BookmarkNode* node = bookmark_bar->GetChild(i);
+ if (node->is_folder())
+ existing_folder_names.insert(node->GetTitle());
+ }
+
+ // If the given name is unique, use it.
+ if (existing_folder_names.find(folder_name) == existing_folder_names.end())
+ return folder_name;
+
+ // Otherwise iterate until we find a unique name.
+ for (size_t i = 1; i <= existing_folder_names.size(); ++i) {
+ string16 name = folder_name + ASCIIToUTF16(" (") + base::IntToString16(i) +
+ ASCIIToUTF16(")");
+ if (existing_folder_names.find(name) == existing_folder_names.end())
+ return name;
+ }
+
+ NOTREACHED();
+ return folder_name;
+}
+
+// Shows the bookmarks toolbar.
+void ShowBookmarkBar(Profile* profile) {
+ PrefService* prefs = profile->GetPrefs();
+ // Check whether the bookmark bar is shown in current pref.
+ if (!prefs->GetBoolean(prefs::kShowBookmarkBar)) {
+ // Set the pref and notify the notification service.
+ prefs->SetBoolean(prefs::kShowBookmarkBar, true);
+ prefs->ScheduleSavePersistentPrefs();
+ Source<Profile> source(profile);
+ NotificationService::current()->Notify(
+ NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, source,
+ NotificationService::NoDetails());
+ }
+}
+
+} // namespace
+
ProfileWriter::BookmarkEntry::BookmarkEntry()
: in_toolbar(false),
is_folder(false) {}
@@ -61,72 +112,89 @@ void ProfileWriter::AddHomepage(const GURL& home_page) {
}
void ProfileWriter::AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) {
+ const string16& top_level_folder_name) {
if (bookmarks.empty())
return;
BookmarkModel* model = profile_->GetBookmarkModel();
DCHECK(model->IsLoaded());
- bool import_to_bookmark_bar = ((options & IMPORT_TO_BOOKMARK_BAR) != 0);
- string16 real_first_folder = import_to_bookmark_bar ? first_folder_name :
- GenerateUniqueFolderName(model, first_folder_name);
+ // If the bookmark bar is currently empty, we should import directly to it.
+ // Otherwise, we should import everything to a subfolder.
+ const BookmarkNode* bookmark_bar = model->GetBookmarkBarNode();
+ bool import_to_top_level = bookmark_bar->child_count() == 0;
- bool show_bookmark_toolbar = false;
- std::set<const BookmarkNode*> folders_added_to;
+ // If the user currently has no bookmarks in the bookmark bar, make sure that
+ // at least some of the imported bookmarks end up there. Otherwise, we'll end
+ // up with just a single folder containing the imported bookmarks, which makes
+ // for unnecessary nesting.
+ bool add_all_to_top_level = import_to_top_level;
+ for (std::vector<BookmarkEntry>::const_iterator it = bookmarks.begin();
+ it != bookmarks.end() && add_all_to_top_level; ++it) {
+ if (it->in_toolbar)
+ add_all_to_top_level = false;
+ }
model->BeginImportMode();
- for (std::vector<BookmarkEntry>::const_iterator it = bookmarks.begin();
- it != bookmarks.end(); ++it) {
- // Don't insert this url if it isn't valid.
- if (!it->is_folder && !it->url.is_valid())
+ std::set<const BookmarkNode*> folders_added_to;
+ const BookmarkNode* top_level_folder = NULL;
+ for (std::vector<BookmarkEntry>::const_iterator bookmark = bookmarks.begin();
+ bookmark != bookmarks.end(); ++bookmark) {
+ // Disregard any bookmarks with invalid urls.
+ if (!bookmark->is_folder && !bookmark->url.is_valid())
continue;
- // We suppose that bookmarks are unique by Title, URL, and Folder. Since
- // checking for uniqueness may not be always the user's intention we have
- // this as an option.
- if (options & ADD_IF_UNIQUE && DoesBookmarkExist(model, *it,
- real_first_folder, import_to_bookmark_bar))
- continue;
+ const BookmarkNode* parent = NULL;
+ if (import_to_top_level && (add_all_to_top_level || bookmark->in_toolbar)) {
+ // Add directly to the bookmarks bar.
+ parent = bookmark_bar;
+ } else {
+ // Add to a folder that will contain all the imported bookmarks not added
+ // to the bar. The first time we do so, create the folder.
+ if (!top_level_folder) {
+ string16 name = GenerateUniqueFolderName(model, top_level_folder_name);
+ top_level_folder = model->AddFolder(bookmark_bar,
+ bookmark_bar->child_count(),
+ name);
+ }
+ parent = top_level_folder;
+ }
- // Set up folders in BookmarkModel in such a way that path[i] is
- // the subfolder of path[i-1]. Finally they construct a path in the
- // model:
- // path[0] \ path[1] \ ... \ path[size() - 1]
- const BookmarkNode* parent =
- (it->in_toolbar ? model->GetBookmarkBarNode() : model->other_node());
- for (std::vector<string16>::const_iterator i = it->path.begin();
- i != it->path.end(); ++i) {
- const BookmarkNode* child = NULL;
- const string16& folder_name = (!import_to_bookmark_bar &&
- !it->in_toolbar && (i == it->path.begin())) ? real_first_folder : *i;
+ // Ensure any enclosing folders are present in the model. The bookmark's
+ // enclosing folder structure should be
+ // path[0] > path[1] > ... > path[size() - 1]
+ for (std::vector<string16>::const_iterator folder_name =
+ bookmark->path.begin();
+ folder_name != bookmark->path.end(); ++folder_name) {
+ if (bookmark->in_toolbar && parent == bookmark_bar &&
+ folder_name == bookmark->path.begin()) {
+ // If we're importing directly to the bookmarks bar, skip over the
+ // folder named "Bookmarks Toolbar" (or any non-Firefox equivalent).
+ continue;
+ }
+ const BookmarkNode* child = NULL;
for (int index = 0; index < parent->child_count(); ++index) {
const BookmarkNode* node = parent->GetChild(index);
- if (node->is_folder() && node->GetTitle() == folder_name) {
+ if (node->is_folder() && node->GetTitle() == *folder_name) {
child = node;
break;
}
}
if (!child)
- child = model->AddFolder(parent, parent->child_count(), folder_name);
+ child = model->AddFolder(parent, parent->child_count(), *folder_name);
parent = child;
}
folders_added_to.insert(parent);
- if (it->is_folder) {
- model->AddFolder(parent, parent->child_count(), it->title);
+ if (bookmark->is_folder) {
+ model->AddFolder(parent, parent->child_count(), bookmark->title);
} else {
model->AddURLWithCreationTime(parent, parent->child_count(),
- it->title, it->url, it->creation_time);
+ bookmark->title, bookmark->url,
+ bookmark->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.
- if (it->in_toolbar)
- show_bookmark_toolbar = true;
}
// In order to keep the imported-to folders from appearing in the 'recently
@@ -139,8 +207,9 @@ void ProfileWriter::AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
model->EndImportMode();
- if (show_bookmark_toolbar && !(options & BOOKMARK_BAR_DISABLED))
- ShowBookmarkBar();
+ // If the user was previously using a toolbar, we should show the bar.
+ if (import_to_top_level && !add_all_to_top_level)
+ ShowBookmarkBar(profile_);
}
void ProfileWriter::AddFavicons(
@@ -265,94 +334,4 @@ void ProfileWriter::AddKeywords(const std::vector<TemplateURL*>& template_urls,
}
}
-void ProfileWriter::ShowBookmarkBar() {
- DCHECK(profile_);
-
- PrefService* prefs = profile_->GetPrefs();
- // Check whether the bookmark bar is shown in current pref.
- if (!prefs->GetBoolean(prefs::kShowBookmarkBar)) {
- // Set the pref and notify the notification service.
- prefs->SetBoolean(prefs::kShowBookmarkBar, true);
- prefs->ScheduleSavePersistentPrefs();
- Source<Profile> source(profile_);
- NotificationService::current()->Notify(
- NotificationType::BOOKMARK_BAR_VISIBILITY_PREF_CHANGED, source,
- NotificationService::NoDetails());
- }
-}
-
ProfileWriter::~ProfileWriter() {}
-
-string16 ProfileWriter::GenerateUniqueFolderName(
- BookmarkModel* model,
- const string16& folder_name) {
- // Build a set containing the folder names of the other folder.
- std::set<string16> other_folder_names;
- const BookmarkNode* other = model->other_node();
-
- for (int i = 0, child_count = other->child_count(); i < child_count; ++i) {
- const BookmarkNode* node = other->GetChild(i);
- if (node->is_folder())
- other_folder_names.insert(node->GetTitle());
- }
-
- if (other_folder_names.find(folder_name) == other_folder_names.end())
- return folder_name; // Name is unique, use it.
-
- // Otherwise iterate until we find a unique name.
- for (int i = 1; i < 100; ++i) {
- string16 name = folder_name + UTF8ToUTF16(base::StringPrintf(" (%d)", i));
- if (other_folder_names.find(name) == other_folder_names.end())
- return name;
- }
-
- return folder_name;
-}
-
-bool ProfileWriter::DoesBookmarkExist(
- BookmarkModel* model,
- const BookmarkEntry& entry,
- const string16& first_folder_name,
- bool import_to_bookmark_bar) {
- std::vector<const BookmarkNode*> nodes_with_same_url;
- model->GetNodesByURL(entry.url, &nodes_with_same_url);
- if (nodes_with_same_url.empty())
- return false;
-
- for (size_t i = 0; i < nodes_with_same_url.size(); ++i) {
- const BookmarkNode* node = nodes_with_same_url[i];
- if (entry.title != node->GetTitle())
- continue;
-
- // Does the path match?
- bool found_match = true;
- const BookmarkNode* parent = node->parent();
- for (std::vector<string16>::const_reverse_iterator path_it =
- entry.path.rbegin();
- (path_it != entry.path.rend()) && found_match; ++path_it) {
- const string16& folder_name =
- (!import_to_bookmark_bar && path_it + 1 == entry.path.rend()) ?
- first_folder_name : *path_it;
- if (NULL == parent || *path_it != folder_name)
- found_match = false;
- else
- parent = parent->parent();
- }
-
- // We need a post test to differentiate checks such as
- // /home/hello and /hello. The parent should either by the other folder
- // node, or the bookmarks bar, depending upon import_to_bookmark_bar and
- // entry.in_toolbar.
- if (found_match &&
- ((import_to_bookmark_bar && entry.in_toolbar && parent !=
- model->GetBookmarkBarNode()) ||
- ((!import_to_bookmark_bar || !entry.in_toolbar) &&
- parent != model->other_node()))) {
- found_match = false;
- }
-
- if (found_match)
- return true; // Found a match with the same url path and title.
- }
- return false;
-}
diff --git a/chrome/browser/importer/profile_writer.h b/chrome/browser/importer/profile_writer.h
index 4a88db7..2e2830b 100644
--- a/chrome/browser/importer/profile_writer.h
+++ b/chrome/browser/importer/profile_writer.h
@@ -10,6 +10,7 @@
#include "base/basictypes.h"
#include "base/memory/ref_counted.h"
+#include "base/string16.h"
#include "base/time.h"
#include "build/build_config.h"
#include "chrome/browser/history/history_types.h"
@@ -31,21 +32,6 @@ struct IE7PasswordInfo;
// This object must be invoked on UI thread.
class ProfileWriter : public base::RefCountedThreadSafe<ProfileWriter> {
public:
- // Used to identify how the bookmarks are added.
- enum BookmarkOptions {
- // Indicates the bookmark should only be added if unique. Uniqueness is done
- // by title, url and path. That is, if this is passed to AddBookmarks, the
- // bookmark is added only if there is no other URL with the same url, path
- // and title.
- ADD_IF_UNIQUE = 1 << 0,
-
- // Indicates the bookmarks should be added to the bookmark bar.
- IMPORT_TO_BOOKMARK_BAR = 1 << 1,
-
- // Indicates the bookmark bar is not shown.
- BOOKMARK_BAR_DISABLED = 1 << 2
- };
-
struct BookmarkEntry {
BookmarkEntry();
~BookmarkEntry();
@@ -78,21 +64,26 @@ class ProfileWriter : public base::RefCountedThreadSafe<ProfileWriter> {
virtual void AddHomepage(const GURL& homepage);
- // Adds the bookmarks to the BookmarkModel.
- // |options| is a bitmask of BookmarkOptions and dictates how and
- // which bookmarks are added. If the bitmask contains IMPORT_TO_BOOKMARK_BAR,
- // then any entries with a value of true for in_toolbar are added to
- // the bookmark bar. If the bitmask does not contain IMPORT_TO_BOOKMARK_BAR
- // then the folder name the bookmarks are added to is uniqued based on
- // |first_folder_name|. For example, if |first_folder_name| is 'foo'
- // and a folder with the name 'foo' already exists in the other
- // bookmarks folder, then the folder name 'foo 2' is used.
- // If |options| contains ADD_IF_UNIQUE, then the bookmark is added only
- // if another bookmarks does not exist with the same title, path and
- // url.
- virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmark,
- const string16& first_folder_name,
- int options);
+ // Adds the |bookmarks| to the bookmark model.
+ //
+ // (a) If the bookmarks bar is empty:
+ // (i) If |bookmarks| includes at least one bookmark that was originally
+ // located in a toolbar, all such bookmarks are imported directly to
+ // the toolbar; any other bookmarks are imported to a subfolder in
+ // the toolbar.
+ // (i) If |bookmarks| includes no bookmarks that were originally located
+ // in a toolbar, all bookmarks are imported directly to the toolbar.
+ // (b) If the bookmarks bar is not empty, all bookmarks are imported to a
+ // subfolder in the toolbar.
+ //
+ // In either case, if a subfolder is created, the name will be the value of
+ // |top_level_folder_name|, unless a folder with this name already exists.
+ // If a folder with this name already exists, then the name is uniquified.
+ // For example, if |first_folder_name| is 'Imported from IE' and a folder with
+ // the name 'Imported from IE' already exists in the bookmarks toolbar, then
+ // we will instead create a subfolder named 'Imported from IE (1)'.
+ virtual void AddBookmarks(const std::vector<BookmarkEntry>& bookmarks,
+ const string16& top_level_folder_name);
virtual void AddFavicons(
const std::vector<history::ImportedFaviconUsage>& favicons);
@@ -107,33 +98,16 @@ class ProfileWriter : public base::RefCountedThreadSafe<ProfileWriter> {
// If unique_on_host_and_path a TemplateURL is only added if there is not an
// existing TemplateURL that has a replaceable search url with the same
// host+path combination.
-
virtual void AddKeywords(const std::vector<TemplateURL*>& template_urls,
int default_keyword_index,
bool unique_on_host_and_path);
- // Shows the bookmarks toolbar.
- void ShowBookmarkBar();
-
protected:
friend class base::RefCountedThreadSafe<ProfileWriter>;
virtual ~ProfileWriter();
private:
- // Generates a unique folder name. If folder_name is not unique, then this
- // repeatedly tests for '|folder_name| + (i)' until a unique name is found.
- string16 GenerateUniqueFolderName(BookmarkModel* model,
- const string16& folder_name);
-
- // Returns true if a bookmark exists with the same url, title and path
- // as |entry|. |first_folder_name| is the name to use for the first
- // path entry if |import_to_bookmark_bar| is true.
- bool DoesBookmarkExist(BookmarkModel* model,
- const BookmarkEntry& entry,
- const string16& first_folder_name,
- bool import_to_bookmark_bar);
-
Profile* const profile_;
DISALLOW_COPY_AND_ASSIGN(ProfileWriter);
diff --git a/chrome/browser/importer/safari_importer.h b/chrome/browser/importer/safari_importer.h
index 47dfd92..2c34bc5 100644
--- a/chrome/browser/importer/safari_importer.h
+++ b/chrome/browser/importer/safari_importer.h
@@ -72,7 +72,8 @@ class SafariImporter : public Importer {
void ImportHistory();
// Parse Safari's stored bookmarks.
- void ParseBookmarks(std::vector<ProfileWriter::BookmarkEntry>* bookmarks);
+ void ParseBookmarks(const string16& toolbar_name,
+ std::vector<ProfileWriter::BookmarkEntry>* bookmarks);
// Function to recursively read Bookmarks out of Safari plist.
// |bookmark_folder| The dictionary containing a folder to parse.
@@ -83,6 +84,7 @@ class SafariImporter : public Importer {
NSDictionary* bookmark_folder,
const std::vector<string16>& parent_path_elements,
bool is_in_toolbar,
+ const string16& toolbar_name,
std::vector<ProfileWriter::BookmarkEntry>* out_bookmarks);
// Converts history time stored by Safari as a double serialized as a string,
diff --git a/chrome/browser/importer/safari_importer.mm b/chrome/browser/importer/safari_importer.mm
index fd4e126..bdd0309 100644
--- a/chrome/browser/importer/safari_importer.mm
+++ b/chrome/browser/importer/safari_importer.mm
@@ -97,17 +97,16 @@ void SafariImporter::StartImport(const importer::SourceProfile& source_profile,
}
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.
@@ -193,6 +192,7 @@ 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);
@@ -208,33 +208,44 @@ void SafariImporter::RecursiveReadBookmarksFolder(
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"];
@@ -246,11 +257,12 @@ void SafariImporter::RecursiveReadBookmarksFolder(
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;
@@ -275,6 +287,7 @@ void SafariImporter::RecursiveReadBookmarksFolder(
}
void SafariImporter::ParseBookmarks(
+ const string16& toolbar_name,
std::vector<ProfileWriter::BookmarkEntry>* bookmarks) {
DCHECK(bookmarks);
@@ -295,7 +308,7 @@ void SafariImporter::ParseBookmarks(
// Recursively read in bookmarks.
std::vector<string16> parent_path_elements;
RecursiveReadBookmarksFolder(bookmarks_dict, parent_path_elements, false,
- bookmarks);
+ toolbar_name, bookmarks);
}
void SafariImporter::ImportPasswords() {
diff --git a/chrome/browser/importer/safari_importer_unittest.mm b/chrome/browser/importer/safari_importer_unittest.mm
index efd9714..edd5c50 100644
--- a/chrome/browser/importer/safari_importer_unittest.mm
+++ b/chrome/browser/importer/safari_importer_unittest.mm
@@ -10,6 +10,7 @@
#include "base/file_util.h"
#include "base/path_service.h"
#include "base/scoped_temp_dir.h"
+#include "base/string_util.h"
#include "base/sys_string_conversions.h"
#include "base/utf_string_conversions.h"
#include "chrome/browser/history/history_types.h"
@@ -76,39 +77,73 @@ TEST_F(SafariImporterTest, BookmarkImport) {
const struct {
bool in_toolbar;
GURL url;
- // If the path array for an element is entry set this to true.
- bool path_is_empty;
- // We ony support one level of nesting in paths, this makes testing a little
- // easier.
- std::wstring path;
- std::wstring title;
+ // We store the path with levels of nesting delimited by forward slashes.
+ string16 path;
+ string16 title;
} kImportedBookmarksData[] = {
- {true, GURL("http://www.apple.com/"), true, L"", L"Apple"},
- {true, GURL("http://www.yahoo.com/"), true, L"", L"Yahoo!"},
- {true, GURL("http://www.cnn.com/"), false, L"News", L"CNN"},
- {true, GURL("http://www.nytimes.com/"), false, L"News",
- L"The New York Times"},
- {false, GURL("http://www.reddit.com/"), true, L"",
- L"reddit.com: what's new online!"},
+ {
+ true,
+ GURL("http://www.apple.com/"),
+ ASCIIToUTF16("Toolbar/"),
+ ASCIIToUTF16("Apple")
+ },
+ {
+ true,
+ GURL("http://www.yahoo.com/"),
+ ASCIIToUTF16("Toolbar/"),
+ ASCIIToUTF16("Yahoo!")
+ },
+ {
+ true,
+ GURL("http://www.cnn.com/"),
+ ASCIIToUTF16("Toolbar/News"),
+ ASCIIToUTF16("CNN")
+ },
+ {
+ true,
+ GURL("http://www.nytimes.com/"),
+ ASCIIToUTF16("Toolbar/News"),
+ ASCIIToUTF16("The New York Times")
+ },
+ {
+ false,
+ GURL("http://www.reddit.com/"),
+ ASCIIToUTF16(""),
+ ASCIIToUTF16("reddit.com: what's new online!")
+ },
+ {
+ false,
+ GURL(),
+ ASCIIToUTF16(""),
+ ASCIIToUTF16("Empty Folder")
+ },
+ {
+ false,
+ GURL("http://www.webkit.org/blog/"),
+ ASCIIToUTF16(""),
+ ASCIIToUTF16("Surfin' Safari - The WebKit Blog")
+ },
};
scoped_refptr<SafariImporter> importer(GetSafariImporter());
std::vector<ProfileWriter::BookmarkEntry> bookmarks;
- importer->ParseBookmarks(&bookmarks);
+ importer->ParseBookmarks(ASCIIToUTF16("Toolbar"), &bookmarks);
size_t num_bookmarks = bookmarks.size();
- EXPECT_EQ(num_bookmarks, ARRAYSIZE_UNSAFE(kImportedBookmarksData));
+ ASSERT_EQ(ARRAYSIZE_UNSAFE(kImportedBookmarksData), num_bookmarks);
for (size_t i = 0; i < num_bookmarks; ++i) {
ProfileWriter::BookmarkEntry& entry = bookmarks[i];
- EXPECT_EQ(entry.in_toolbar, kImportedBookmarksData[i].in_toolbar);
- EXPECT_EQ(entry.url, kImportedBookmarksData[i].url);
- if (kImportedBookmarksData[i].path_is_empty) {
- EXPECT_EQ(entry.path.size(), 0U);
- } else {
- EXPECT_EQ(entry.path.size(), 1U);
- EXPECT_EQ(UTF16ToWideHack(entry.path[0]), kImportedBookmarksData[i].path);
- EXPECT_EQ(UTF16ToWideHack(entry.title), kImportedBookmarksData[i].title);
+ EXPECT_EQ(kImportedBookmarksData[i].in_toolbar, entry.in_toolbar);
+ EXPECT_EQ(kImportedBookmarksData[i].url, entry.url);
+
+ std::vector<string16> path;
+ Tokenize(kImportedBookmarksData[i].path, ASCIIToUTF16("/"), &path);
+ ASSERT_EQ(path.size(), entry.path.size());
+ for (size_t j = 0; j < path.size(); ++j) {
+ EXPECT_EQ(path[j], entry.path[j]);
}
+
+ EXPECT_EQ(kImportedBookmarksData[i].title, entry.title);
}
}
diff --git a/chrome/browser/importer/toolbar_importer.cc b/chrome/browser/importer/toolbar_importer.cc
index eeea95c..44a512d 100644
--- a/chrome/browser/importer/toolbar_importer.cc
+++ b/chrome/browser/importer/toolbar_importer.cc
@@ -556,8 +556,6 @@ void Toolbar5Importer::AddBookmarksToChrome(
if (!bookmarks.empty() && !cancelled()) {
const string16& first_folder_name =
bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP_FROM_GOOGLE_TOOLBAR);
- int options = ProfileWriter::ADD_IF_UNIQUE |
- (import_to_bookmark_bar() ? ProfileWriter::IMPORT_TO_BOOKMARK_BAR : 0);
- bridge_->AddBookmarks(bookmarks, first_folder_name, options);
+ bridge_->AddBookmarks(bookmarks, first_folder_name);
}
}
diff --git a/chrome/profile_import/profile_import_thread.cc b/chrome/profile_import/profile_import_thread.cc
index 2472974..d98a2b0 100644
--- a/chrome/profile_import/profile_import_thread.cc
+++ b/chrome/profile_import/profile_import_thread.cc
@@ -54,8 +54,7 @@ bool ProfileImportThread::OnControlMessageReceived(const IPC::Message& msg) {
void ProfileImportThread::OnImportStart(
const importer::SourceProfile& source_profile,
uint16 items,
- const DictionaryValue& localized_strings,
- bool import_to_bookmark_bar) {
+ const DictionaryValue& localized_strings) {
bridge_ = new ExternalProcessImporterBridge(this, localized_strings);
importer_ = importer::CreateImporterByType(source_profile.importer_type);
if (!importer_) {
@@ -64,7 +63,6 @@ void ProfileImportThread::OnImportStart(
return;
}
- importer_->set_import_to_bookmark_bar(import_to_bookmark_bar);
items_to_import_ = items;
// Create worker thread in which importer runs.
@@ -137,10 +135,9 @@ void ProfileImportThread::NotifyHomePageImportReady(
void ProfileImportThread::NotifyBookmarksImportReady(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options) {
+ const string16& first_folder_name) {
Send(new ProfileImportProcessHostMsg_NotifyBookmarksImportStart(
- first_folder_name, options, bookmarks.size()));
+ first_folder_name, bookmarks.size()));
std::vector<ProfileWriter::BookmarkEntry>::const_iterator it;
for (it = bookmarks.begin(); it < bookmarks.end();
diff --git a/chrome/profile_import/profile_import_thread.h b/chrome/profile_import/profile_import_thread.h
index a81b8164..3347d40 100644
--- a/chrome/profile_import/profile_import_thread.h
+++ b/chrome/profile_import/profile_import_thread.h
@@ -56,8 +56,7 @@ class ProfileImportThread : public ChildThread {
void NotifyHomePageImportReady(const GURL& home_page);
void NotifyBookmarksImportReady(
const std::vector<ProfileWriter::BookmarkEntry>& bookmarks,
- const string16& first_folder_name,
- int options);
+ const string16& first_folder_name);
void NotifyFaviconsImportReady(
const std::vector<history::ImportedFaviconUsage>& favicons);
void NotifyPasswordFormReady(const webkit_glue::PasswordForm& form);
@@ -76,8 +75,7 @@ class ProfileImportThread : public ChildThread {
void OnImportStart(
const importer::SourceProfile& source_profile,
uint16 items,
- const DictionaryValue& localized_strings,
- bool import_to_bookmark_bar);
+ const DictionaryValue& localized_strings);
// Calls cleanup to stop the import operation.
void OnImportCancel();
diff --git a/chrome/test/data/safari_import/Safari/Bookmarks.plist b/chrome/test/data/safari_import/Safari/Bookmarks.plist
index df95780..9ca7ed3 100644
--- a/chrome/test/data/safari_import/Safari/Bookmarks.plist
+++ b/chrome/test/data/safari_import/Safari/Bookmarks.plist
Binary files differ