Toolbar Importer Pass2

Implements the following features
1.  Toolbar import always available after first run
2.  Deduping of imported favorites
3.  Removes attempt to import favicons (not supported through Toolbar UI atm)
4.  Always the user to cancel from the UI safely
5.  Unittests for parsing
(TODO Better UI for login)
Review URL: http://codereview.chromium.org/9414

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

12 files changed, 774 insertions, 259 deletions

diff --git a/chrome/browser/importer/firefox2_importer.cc b/chrome/browser/importer/firefox2_importer.cc
index 0594001..e85d347 100644
--- a/chrome/browser/importer/firefox2_importer.cc
+++ b/chrome/browser/importer/firefox2_importer.cc
@@ -30,6 +30,7 @@ Firefox2Importer::~Firefox2Importer() {
 
 void Firefox2Importer::StartImport(ProfileInfo profile_info,
                                    uint16 items, ProfileWriter* writer,
+                                   MessageLoop* delagate_loop,
                                    ImporterHost* host) {
   writer_ = writer;
   source_path_ = profile_info.source_path;
@@ -225,7 +226,7 @@ void Firefox2Importer::ImportBookmarks() {
                    toolbar_bookmarks.end());
   if (!bookmarks.empty() && !cancelled()) {
     main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
-        &ProfileWriter::AddBookmarkEntry, bookmarks));
+        &ProfileWriter::AddBookmarkEntry, bookmarks, false));
   }
   if (!template_urls.empty() && !cancelled()) {
     main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
diff --git a/chrome/browser/importer/firefox2_importer.h b/chrome/browser/importer/firefox2_importer.h
index 4c55eb0..f14438d 100644
--- a/chrome/browser/importer/firefox2_importer.h
+++ b/chrome/browser/importer/firefox2_importer.h
@@ -19,6 +19,7 @@ class Firefox2Importer : public Importer {
   virtual void StartImport(ProfileInfo profile_info,
                            uint16 items,
                            ProfileWriter* writer,
+                           MessageLoop* delagate_loop,
                            ImporterHost* host);
 
   // Loads the default bookmarks in the Firefox installed at |firefox_app_path|,
diff --git a/chrome/browser/importer/firefox3_importer.cc b/chrome/browser/importer/firefox3_importer.cc
index 07f9683..965ba15 100644
--- a/chrome/browser/importer/firefox3_importer.cc
+++ b/chrome/browser/importer/firefox3_importer.cc
@@ -33,6 +33,7 @@ class DBClose {
 
 void Firefox3Importer::StartImport(ProfileInfo profile_info,
                                    uint16 items, ProfileWriter* writer,
+                                   MessageLoop* delagate_loop,
                                    ImporterHost* host) {
   writer_ = writer;
   source_path_ = profile_info.source_path;
@@ -246,7 +247,7 @@ void Firefox3Importer::ImportBookmarks() {
   // Write into profile.
   if (!bookmarks.empty() && !cancelled()) {
     main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
-        &ProfileWriter::AddBookmarkEntry, bookmarks));
+        &ProfileWriter::AddBookmarkEntry, bookmarks, false));
   }
   if (!template_urls.empty() && !cancelled()) {
     main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
diff --git a/chrome/browser/importer/firefox3_importer.h b/chrome/browser/importer/firefox3_importer.h
index 0accadf..dc378bf 100644
--- a/chrome/browser/importer/firefox3_importer.h
+++ b/chrome/browser/importer/firefox3_importer.h
@@ -26,6 +26,7 @@ class Firefox3Importer : public Importer {
   virtual void StartImport(ProfileInfo profile_info,
                            uint16 items,
                            ProfileWriter* writer,
+                           MessageLoop* delagate_loop_,
                            ImporterHost* host);
 
  private:
diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc
index 19f60a1..c952603 100644
--- a/chrome/browser/importer/ie_importer.cc
+++ b/chrome/browser/importer/ie_importer.cc
@@ -56,6 +56,7 @@ const GUID IEImporter::kUnittestGUID = { 0xa79029d6, 0x753e, 0x4e27,
 void IEImporter::StartImport(ProfileInfo profile_info,
                              uint16 items,
                              ProfileWriter* writer,
+                             MessageLoop* delagate_loop,
                              ImporterHost* host) {
   writer_ = writer;
   source_path_ = profile_info.source_path;
@@ -108,7 +109,7 @@ void IEImporter::ImportFavorites() {
 
   if (!bookmarks.empty() && !cancelled()) {
     main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
-        &ProfileWriter::AddBookmarkEntry, bookmarks));
+        &ProfileWriter::AddBookmarkEntry, bookmarks, false));
   }
 }
 
diff --git a/chrome/browser/importer/ie_importer.h b/chrome/browser/importer/ie_importer.h
index 9e55e6d..11303b8 100644
--- a/chrome/browser/importer/ie_importer.h
+++ b/chrome/browser/importer/ie_importer.h
@@ -16,6 +16,7 @@ class IEImporter : public Importer {
   virtual void StartImport(ProfileInfo browser_info,
                            uint16 items,
                            ProfileWriter* writer,
+                           MessageLoop* delagate_loop,
                            ImporterHost* host);
 
  private:
diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc
index c2fa2a6..16552e3 100644
--- a/chrome/browser/importer/importer.cc
+++ b/chrome/browser/importer/importer.cc
@@ -80,7 +80,8 @@ void ProfileWriter::AddHomepage(const GURL& home_page) {
 }
 
 void ProfileWriter::AddBookmarkEntry(
-    const std::vector<BookmarkEntry>& bookmark) {
+    const std::vector<BookmarkEntry>& bookmark,
+    bool check_uniqueness) {
   BookmarkModel* model = profile_->GetBookmarkModel();
   DCHECK(model->IsLoaded());
 
@@ -91,6 +92,50 @@ void ProfileWriter::AddBookmarkEntry(
     // Don't insert this url if it isn't valid.
     if (!it->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 (check_uniqueness) {
+      std::vector<BookmarkModel::TitleMatch> matches;
+      model->GetBookmarksMatchingText((*it).title, 32, &matches); // 32 enough?
+      if (!matches.empty()) {
+        bool found_match = false;
+        for (std::vector<BookmarkModel::TitleMatch>::iterator match_it =
+                matches.begin();
+             match_it != matches.end() && !found_match;
+             ++match_it) {
+          if ((*it).title != (*match_it).node->GetTitle())
+            continue;
+          if ((*it).url != (*match_it).node->GetURL())
+            continue;
+
+          // Check the folder path for uniqueness as well
+          found_match = true;
+          BookmarkNode* node = (*match_it).node->GetParent();
+          for(std::vector<std::wstring>::const_reverse_iterator path_it =
+                  (*it).path.rbegin();
+              (path_it != (*it).path.rend()) && found_match;
+              ++path_it) {
+            if (NULL == node || (*path_it != node->GetTitle()))
+              found_match = false;
+            if (found_match)
+              node = node->GetParent();
+          }
+
+          // We need a post test to differentiate checks such as
+          // /home/hello and /hello.  Note that here the current parent
+          // should be the "Other bookmarks" node, its parent should be the
+          // root with title "", and it's parent is finally NULL.
+          if (NULL == node->GetParent() ||
+              NULL != node->GetParent()->GetParent())
+            found_match = false;
+        }
+
+        if (found_match)
+          continue;
+      }      
+    }
 
     // Set up groups in BookmarkModel in such a way that path[i] is
     // the subgroup of path[i-1]. Finally they construct a path in the
@@ -396,7 +441,7 @@ void ImporterHost::StartImportSettings(const ProfileInfo& profile_info,
   importer_->AddRef();
   importer_->set_first_run(first_run);
   task_ = NewRunnableMethod(importer_, &Importer::StartImport,
-      profile_info, items, writer_.get(), this);
+      profile_info, items, writer_.get(), file_loop_, this);
 
   // We should lock the Firefox profile directory to prevent corruption.
   if (profile_info.browser_type == FIREFOX2 ||
@@ -612,25 +657,14 @@ void ImporterHost::DetectFirefoxProfiles() {
 }
 
 void ImporterHost::DetectGoogleToolbarProfiles() {
-  if (ToolbarImporterUtils::IsToolbarInstalled()) {
-    TOOLBAR_VERSION version = ToolbarImporterUtils::GetToolbarVersion();
-    if (DEPRECATED != version) {
-      ProfileInfo* google_toolbar = new ProfileInfo();
-      switch (version) {
-        // TODO(brg): Support other toolbar version after 1.0.
-        case VERSION_5:
-          google_toolbar->browser_type = GOOGLE_TOOLBAR5;
-          break;
-        default:
-          NOTREACHED() << "Supported Google Toolbar version not implemented.";
-          break;
-      }
-      google_toolbar->description = l10n_util::GetString(
-                                    IDS_IMPORT_FROM_GOOGLE_TOOLBAR);
-      google_toolbar->source_path.clear();
-      google_toolbar->app_path.clear();
-      google_toolbar->services_supported = FAVORITES;
-      source_profiles_.push_back(google_toolbar);
-    }
+  if (!FirstRun::IsChromeFirstRun()) {
+    ProfileInfo* google_toolbar = new ProfileInfo();
+    google_toolbar->browser_type = GOOGLE_TOOLBAR5;
+    google_toolbar->description = l10n_util::GetString(
+                                  IDS_IMPORT_FROM_GOOGLE_TOOLBAR);
+    google_toolbar->source_path.clear();
+    google_toolbar->app_path.clear();
+    google_toolbar->services_supported = FAVORITES;
+    source_profiles_.push_back(google_toolbar);
   }
 }
diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h
index dba4cd4..a597879 100644
--- a/chrome/browser/importer/importer.h
+++ b/chrome/browser/importer/importer.h
@@ -86,7 +86,9 @@ class ProfileWriter : public base::RefCounted<ProfileWriter> {
   virtual void AddIE7PasswordInfo(const IE7PasswordInfo& info);
   virtual void AddHistoryPage(const std::vector<history::URLRow>& page);
   virtual void AddHomepage(const GURL& homepage);
-  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark);
+  virtual void AddBookmarkEntry(
+      const std::vector<BookmarkEntry>& bookmark, 
+      bool check_uniqueness);
   virtual void AddFavicons(
       const std::vector<history::ImportedFavIconUsage>& favicons);
   // Add the TemplateURLs in |template_urls| to the local store and make the
@@ -280,16 +282,18 @@ class Importer : public base::RefCounted<Importer> {
   virtual void StartImport(ProfileInfo profile_info,
                            uint16 items,
                            ProfileWriter* writer,
+                           MessageLoop* delegate_loop,
                            ImporterHost* host) = 0;
 
   // Cancels the import process.
-  void Cancel() { cancelled_ = true; }
+  virtual void Cancel() { cancelled_ = true; }
 
   void set_first_run(bool first_run) { first_run_ = first_run; }
 
  protected:
   Importer()
       : main_loop_(MessageLoop::current()),
+        delagate_loop_(NULL),
         importer_host_(NULL),
         cancelled_(false) {}
 
@@ -332,6 +336,9 @@ class Importer : public base::RefCounted<Importer> {
   // The importer should know the main thread so that ProfileWriter
   // will be invoked in thread instead.
   MessageLoop* main_loop_;
+  
+  // The message loop in which the importer operates.
+  MessageLoop* delagate_loop_;
 
   // The coordinator host for this importer.
   ImporterHost* importer_host_;
diff --git a/chrome/browser/importer/importer_unittest.cc b/chrome/browser/importer/importer_unittest.cc
index d30cdb8..f75aa4f 100644
--- a/chrome/browser/importer/importer_unittest.cc
+++ b/chrome/browser/importer/importer_unittest.cc
@@ -189,7 +189,9 @@ class TestObserver : public ProfileWriter,
         ++history_count_;
   }
 
-  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark) {
+  virtual void AddBookmarkEntry(
+      const std::vector<BookmarkEntry>& bookmark,
+      bool check_duplicates) {
     // 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,
@@ -542,7 +544,8 @@ class FirefoxObserver : public ProfileWriter,
     ++history_count_;
   }
 
-  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark) {
+  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark,
+                                bool check_duplicates) {
     for (size_t i = 0; i < bookmark.size(); ++i) {
       if (FindBookmarkEntry(bookmark[i], kFirefox2Bookmarks,
                             arraysize(kFirefox2Bookmarks)))
@@ -741,7 +744,8 @@ class Firefox3Observer : public ProfileWriter,
     ++history_count_;
   }
 
-  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark) {
+  virtual void AddBookmarkEntry(const std::vector<BookmarkEntry>& bookmark,
+                                bool check_duplicates) {
     for (size_t i = 0; i < bookmark.size(); ++i) {
       if (FindBookmarkEntry(bookmark[i], kFirefox3Bookmarks,
                             arraysize(kFirefox3Bookmarks)))
diff --git a/chrome/browser/importer/toolbar_importer.cc b/chrome/browser/importer/toolbar_importer.cc
index 6428a92..8813807 100644
--- a/chrome/browser/importer/toolbar_importer.cc
+++ b/chrome/browser/importer/toolbar_importer.cc
@@ -8,6 +8,7 @@
 #include "base/string_util.h"
 #include "base/rand_util.h"
 #include "base/registry.h"
+#include "chrome/browser/first_run.h"
 #include "chrome/common/l10n_util.h"
 #include "chrome/common/libxml_utils.h"
 #include "net/base/data_url.h"
@@ -15,85 +16,34 @@
 
 #include "generated_resources.h"
 
-
 //
 // ToolbarImporterUtils
 //
-const HKEY ToolbarImporterUtils::kToolbarInstallRegistryRoots[] =
-    {HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE};
-const TCHAR* ToolbarImporterUtils::kToolbarRootRegistryFolder =
-    L"Software\\Google\\Google Toolbar";
-const TCHAR* ToolbarImporterUtils::kToolbarVersionRegistryFolder =
-    L"SOFTWARE\\Google\\Google Toolbar\\Component";
-const TCHAR* ToolbarImporterUtils::kToolbarVersionRegistryKey =
-    L"CurrentVersion";
-
-
-bool ToolbarImporterUtils::IsToolbarInstalled() {
-  for (int index = 0;
-      index < arraysize(kToolbarInstallRegistryRoots);
-      ++index) {
-    RegKey key(kToolbarInstallRegistryRoots[index],
-               kToolbarRootRegistryFolder);
-    if (key.Valid())
-      return true;
-  }
-  return false;
-}
+static const std::string kGoogleDomainUrl = "http://.google.com/";
+static const wchar_t kSplitStringToken = L';';
+static const std::string kGoogleDomainSecureCookieId = "SID=";
 
 bool ToolbarImporterUtils::IsGoogleGAIACookieInstalled() {
   URLRequestContext* context = Profile::GetDefaultRequestContext();
   net::CookieMonster* store= context->cookie_store();
-  GURL url("http://.google.com/");
+  GURL url(kGoogleDomainUrl);
   net::CookieMonster::CookieOptions options = net::CookieMonster::NORMAL;
   std::string cookies = store->GetCookiesWithOptions(url, options);
   std::vector<std::string> cookie_list;
-  SplitString(cookies, L';', &cookie_list);
+  SplitString(cookies, kSplitStringToken, &cookie_list);
   for (std::vector<std::string>::iterator current = cookie_list.begin();
        current != cookie_list.end();
        ++current) {
-    size_t position = (*current).find("SID=");
+    size_t position = (*current).find(kGoogleDomainSecureCookieId);
     if (0 == position)
         return true;
   }
   return false;
 }
 
-TOOLBAR_VERSION ToolbarImporterUtils::GetToolbarVersion() {
-  TOOLBAR_VERSION toolbar_version = NO_VERSION;
-  for (int index = 0;
-       (index < arraysize(kToolbarInstallRegistryRoots)) &&
-       NO_VERSION == toolbar_version;
-       ++index) {
-    RegKey key(kToolbarInstallRegistryRoots[index],
-               kToolbarVersionRegistryFolder);
-    if (key.Valid() && key.ValueExists(kToolbarVersionRegistryKey)) {
-      TCHAR version_buffer[128];
-      DWORD version_buffer_length = sizeof(version_buffer);
-      if (key.ReadValue(kToolbarVersionRegistryKey,
-                        &version_buffer,
-                        &version_buffer_length)) {
-        int version_value = _wtoi(version_buffer);
-        switch (version_value) {
-          case 5: {
-            toolbar_version = VERSION_5;
-            break;
-           }
-          default: {
-            toolbar_version = DEPRECATED;
-            break;
-          }
-        }
-      }
-    }
-  }
-  return toolbar_version;
-}
-
 //
 // Toolbar5Importer
 //
-
 const std::string Toolbar5Importer::kXmlApiReplyXmlTag = "xml_api_reply";
 const std::string Toolbar5Importer::kBookmarksXmlTag = "bookmarks";
 const std::string Toolbar5Importer::kBookmarkXmlTag = "bookmark";
@@ -101,12 +51,9 @@ const std::string Toolbar5Importer::kTitleXmlTag = "title";
 const std::string Toolbar5Importer::kUrlXmlTag = "url";
 const std::string Toolbar5Importer::kTimestampXmlTag = "timestamp";
 const std::string Toolbar5Importer::kLabelsXmlTag = "labels";
+const std::string Toolbar5Importer::kLabelsXmlCloseTag = "/labels";
 const std::string Toolbar5Importer::kLabelXmlTag = "label";
 const std::string Toolbar5Importer::kAttributesXmlTag = "attributes";
-const std::string Toolbar5Importer::kAttributeXmlTag = "attribute";
-const std::string Toolbar5Importer::kNameXmlTag = "name";
-const std::string Toolbar5Importer::kValueXmlTag = "favicon";
-const std::string Toolbar5Importer::kFaviconAttributeXmlName = "favicon_url";
 
 const std::string Toolbar5Importer::kRandomNumberToken = "{random_number}";
 const std::string Toolbar5Importer::kAuthorizationToken = "{auth_token}";
@@ -120,9 +67,6 @@ const std::string Toolbar5Importer::kT5AuthorizationTokenUrl =
 const std::string Toolbar5Importer::kT5FrontEndUrlTemplate =
     "http://www.google.com/notebook/toolbar?cmd=list&tok={auth_token}& "
     "num={max_num}&min={max_timestamp}&all=0&zx={random_number}";
-const std::string Toolbar5Importer::kT4FrontEndUrlTemplate =
-    "http://www.google.com/bookmarks/?output=xml&num={max_num}&"
-    "min={max_timestamp}&all=0&zx={random_number}";
 
 // Importer methods.
 Toolbar5Importer::Toolbar5Importer() : writer_(NULL),
@@ -140,11 +84,13 @@ Toolbar5Importer::~Toolbar5Importer() {
 void Toolbar5Importer::StartImport(ProfileInfo profile_info,
                                    uint16 items,
                                    ProfileWriter* writer,
+                                   MessageLoop* delagate_loop,
                                    ImporterHost* host) {
   DCHECK(writer);
   DCHECK(host);
 
   importer_host_ = host;
+  delagate_loop_ = delagate_loop;
   writer_ = writer;
   items_to_import_ = items;
   state_ = INITIALIZED;
@@ -153,6 +99,28 @@ void Toolbar5Importer::StartImport(ProfileInfo profile_info,
   ContinueImport();
 }
 
+// The public cancel method servers two functions, as a callback from the UI
+// as well as an internal callback in case of cancel.  An internal callback
+// is required since the URLFetcher must be destroyed from the thread it was
+// created.
+void Toolbar5Importer::Cancel() {
+  // In the case when the thread is not importing messages we are to
+  // cancel as soon as possible.
+  Importer::Cancel();
+  
+  // If we are conducting network operations, post a message to the importer 
+  // thread for synchronization.
+  if (NULL != delagate_loop_) {
+    if (delagate_loop_ != MessageLoop::current()) {
+      delagate_loop_->PostTask(
+          FROM_HERE, 
+          NewRunnableMethod(this, &Toolbar5Importer::Cancel));
+    } else {
+      EndImport();
+    }
+  }
+}
+
 void Toolbar5Importer::OnURLFetchComplete(
     const URLFetcher* source,
     const GURL& url,
@@ -160,6 +128,11 @@ void Toolbar5Importer::OnURLFetchComplete(
     int response_code,
     const ResponseCookies& cookies,
     const std::string& data) {
+  if (cancelled()) {
+    EndImport();
+    return;
+  }
+  
   if (200 != response_code) {  // HTTP/Ok
     // Display to the user an error dialog and cancel the import
     EndImportBookmarks(false);
@@ -198,20 +171,23 @@ void Toolbar5Importer::ContinueImport() {
 }
 
 void Toolbar5Importer::EndImport() {
-  // By spec the fetcher's must be destroyed within the same
-  // thread they are created.  The importer is destroyed in the ui_thread
-  // so when we complete in the file_thread we destroy them first.
-  if (NULL != token_fetcher_) {
-    delete token_fetcher_;
-    token_fetcher_ = NULL;
-  }
+  if (state_ != DONE) {
+    state_ = DONE;
+    // By spec the fetcher's must be destroyed within the same
+    // thread they are created.  The importer is destroyed in the ui_thread
+    // so when we complete in the file_thread we destroy them first.
+    if (NULL != token_fetcher_) {
+      delete token_fetcher_;
+      token_fetcher_ = NULL;
+    }
 
-  if (NULL != data_fetcher_) {
-    delete data_fetcher_;
-    data_fetcher_ = NULL;
-  }
+    if (NULL != data_fetcher_) {
+      delete data_fetcher_;
+      data_fetcher_ = NULL;
+    }
 
-  NotifyEnded();
+    NotifyEnded();
+  }
 }
 
 void Toolbar5Importer::BeginImportBookmarks() {
@@ -227,6 +203,11 @@ void Toolbar5Importer::EndImportBookmarks(bool success) {
 
 // Notebook FE connection managers.
 void Toolbar5Importer::GetAuthenticationFromServer() {
+  if (cancelled()) {
+    EndImport();
+    return;
+  }
+      
   // Authentication is a token string retreived from the authentication server
   // To access it we call the url below with a random number replacing the
   // value in the string.
@@ -249,6 +230,11 @@ void Toolbar5Importer::GetAuthenticationFromServer() {
 }
 
 void Toolbar5Importer::GetBookmarkDataFromServer(const std::string& response) {
+  if (cancelled()) {
+    EndImport();
+    return;
+  }
+  
   state_ = GET_BOOKMARKS;
 
   // Parse and verify the authorization token from the response.
@@ -278,16 +264,20 @@ void Toolbar5Importer::GetBookmarkDataFromServer(const std::string& response) {
 
 void Toolbar5Importer::GetBookmarsFromServerDataResponse(
     const std::string& response) {
+  if (cancelled()) {
+    EndImport();
+    return;
+  }
+  
+  state_ = PARSE_BOOKMARKS;
+  
   bool retval = false;
   XmlReader reader;
   if (reader.Load(response) && !cancelled()) {
     // Construct Bookmarks
-    std::vector< ProfileWriter::BookmarkEntry > bookmarks;
-    std::vector< history::ImportedFavIconUsage > favicons;
-    retval = ParseBookmarksFromReader(&reader, &bookmarks, &favicons);
-    if (retval && !cancelled()) {
-      AddBookMarksToChrome(bookmarks, favicons);
-    }
+    std::vector<ProfileWriter::BookmarkEntry> bookmarks;
+    retval = ParseBookmarksFromReader(&reader, &bookmarks);
+    if (retval) AddBookMarksToChrome(bookmarks);
   }
   EndImportBookmarks(retval);
 }
@@ -295,7 +285,7 @@ void Toolbar5Importer::GetBookmarsFromServerDataResponse(
 bool Toolbar5Importer::ParseAuthenticationTokenResponse(
     const std::string& response,
     std::string* token) {
-  DCHECK(token);
+  DCHECK(token);  
 
   *token = response;
   size_t position = token->find(kAuthorizationTokenPrefix);
@@ -314,11 +304,9 @@ bool Toolbar5Importer::ParseAuthenticationTokenResponse(
 // Parsing
 bool Toolbar5Importer::ParseBookmarksFromReader(
     XmlReader* reader,
-    std::vector< ProfileWriter::BookmarkEntry >* bookmarks,
-    std::vector< history::ImportedFavIconUsage >* favicons) {
+    std::vector<ProfileWriter::BookmarkEntry>* bookmarks) {
   DCHECK(reader);
   DCHECK(bookmarks);
-  DCHECK(favicons);
 
   // The XML blob returned from the server is described in the
   // Toolbar-Notebook/Bookmarks Protocol document located at
@@ -334,36 +322,75 @@ bool Toolbar5Importer::ParseBookmarksFromReader(
     return false;
 
   // Parse each |bookmark| blob
-  while (LocateNextTagByName(reader, kBookmarkXmlTag)) {
+  while (LocateNextTagWithStopByName(reader, kBookmarkXmlTag, 
+                                     kBookmarksXmlTag)) {
     ProfileWriter::BookmarkEntry bookmark_entry;
-    history::ImportedFavIconUsage favicon_entry;
-    if (ExtractBookmarkInformation(reader, &bookmark_entry, &favicon_entry)) {
-      bookmarks->push_back(bookmark_entry);
-      if (!favicon_entry.favicon_url.is_empty())
-        favicons->push_back(favicon_entry);
+    std::vector<BOOKMARK_FOLDER> folders;
+    if (ExtractBookmarkInformation(reader, &bookmark_entry, &folders)) {
+      // For each folder we create a new bookmark entry.  Duplicates will
+      // be detected whence we attempt to creaete the bookmark in the profile.
+      for(std::vector<BOOKMARK_FOLDER>::iterator folder = folders.begin();
+          folder != folders.end();
+          ++folder) {
+        bookmark_entry.path = *folder;
+        bookmarks->push_back(bookmark_entry);
+      }
     }
   }
 
+  if (0 == bookmarks->size())
+    return false;
+
+  return true;
+}
+
+bool Toolbar5Importer::LocateNextOpenTag(XmlReader* reader) {
+  DCHECK(reader);
+
+  while(!reader->SkipToElement()) {
+    if (!reader->Read())
+      return false;
+  }
   return true;
 }
 
 bool Toolbar5Importer::LocateNextTagByName(XmlReader* reader,
                                            const std::string& tag) {
+  DCHECK(reader);
+
   // Locate the |tag| blob.
   while (tag != reader->NodeName()) {
-    if (!reader->Read())
+    if (!reader->Read() || !LocateNextOpenTag(reader))
       return false;
   }
   return true;
 }
 
+bool Toolbar5Importer::LocateNextTagWithStopByName(XmlReader* reader,
+                                                   const std::string& tag,
+                                                   const std::string& stop) {
+  DCHECK(reader);
+
+  DCHECK_NE(tag, stop);
+  // Locate the |tag| blob.
+  while (tag != reader->NodeName()) {
+   // move to the next open tag
+   if (!reader->Read() || !LocateNextOpenTag(reader))
+     return false;
+   // if we encounter the stop word return false
+   if (stop == reader->NodeName())
+     return false;
+  }
+  return true;
+}
+
 bool Toolbar5Importer::ExtractBookmarkInformation(
     XmlReader* reader,
     ProfileWriter::BookmarkEntry* bookmark_entry,
-    history::ImportedFavIconUsage* favicon_entry) {
+    std::vector<BOOKMARK_FOLDER>* bookmark_folders) {
   DCHECK(reader);
   DCHECK(bookmark_entry);
-  DCHECK(favicon_entry);
+  DCHECK(bookmark_folders);
 
   // The following is a typical bookmark entry.
   // The reader should be pointing to the <title> tag at the moment.
@@ -397,13 +424,11 @@ bool Toolbar5Importer::ExtractBookmarkInformation(
   // <name>section_name</name>
   // <value>My section 0</value>
   // </attribute>
-  // <attribute>
   // </attributes>
   // </bookmark>
   //
   // We parse the blob in order, title->url->timestamp etc.  Any failure
-  // causes us to skip this bookmark.  Note Favicons are optional so failure
-  // to find them is not a failure to parse the blob.
+  // causes us to skip this bookmark.
 
   if (!ExtractTitleFromXmlReader(reader, bookmark_entry))
     return false;
@@ -411,9 +436,8 @@ bool Toolbar5Importer::ExtractBookmarkInformation(
     return false;
   if (!ExtractTimeFromXmlReader(reader, bookmark_entry))
     return false;
-  if (!ExtractFolderFromXmlReader(reader, bookmark_entry))
+  if (!ExtractFoldersFromXmlReader(reader, bookmark_folders))
     return false;
-  ExtractFaviconFromXmlReader(reader, bookmark_entry, favicon_entry);
 
   return true;
 }
@@ -437,7 +461,7 @@ bool Toolbar5Importer::ExtractTitleFromXmlReader(
   DCHECK(reader);
   DCHECK(entry);
 
-  if (!LocateNextTagByName(reader, kTitleXmlTag))
+  if (!LocateNextTagWithStopByName(reader, kTitleXmlTag, kUrlXmlTag))
     return false;
   std::string buffer;
   if (!ExtractNamedValueFromXmlReader(reader, kTitleXmlTag, &buffer)) {
@@ -453,7 +477,7 @@ bool Toolbar5Importer::ExtractUrlFromXmlReader(
   DCHECK(reader);
   DCHECK(entry);
 
-  if (!LocateNextTagByName(reader, kUrlXmlTag))
+  if (!LocateNextTagWithStopByName(reader, kUrlXmlTag, kTimestampXmlTag))
     return false;
   std::string buffer;
   if (!ExtractNamedValueFromXmlReader(reader, kUrlXmlTag, &buffer)) {
@@ -468,7 +492,7 @@ bool Toolbar5Importer::ExtractTimeFromXmlReader(
     ProfileWriter::BookmarkEntry* entry) {
   DCHECK(reader);
   DCHECK(entry);
-  if (!LocateNextTagByName(reader, kTimestampXmlTag))
+  if (!LocateNextTagWithStopByName(reader, kTimestampXmlTag, kLabelsXmlTag))
     return false;
   std::string buffer;
   if (!ExtractNamedValueFromXmlReader(reader, kTimestampXmlTag, &buffer)) {
@@ -482,97 +506,69 @@ bool Toolbar5Importer::ExtractTimeFromXmlReader(
   return true;
 }
 
-bool Toolbar5Importer::ExtractFolderFromXmlReader(
+bool Toolbar5Importer::ExtractFoldersFromXmlReader(
     XmlReader* reader,
-    ProfileWriter::BookmarkEntry* entry) {
+    std::vector<BOOKMARK_FOLDER>* bookmark_folders) {
   DCHECK(reader);
-  DCHECK(entry);
+  DCHECK(bookmark_folders);
 
-  if (!LocateNextTagByName(reader, kLabelsXmlTag))
+  // Read in the labels for this bookmark from the xml.  There may be many
+  // labels for any one bookmark.
+  if (!LocateNextTagWithStopByName(reader, kLabelsXmlTag, kAttributesXmlTag))
     return false;
-  if (!LocateNextTagByName(reader, kLabelXmlTag))
+    
+  // It is within scope to have an empty labels section, so we do not
+  // return false if the labels are empty.
+  if (!reader->Read() || !LocateNextOpenTag(reader))
     return false;
-
+      
   std::vector<std::wstring> label_vector;
-  std::string label_buffer;
-  while (kLabelXmlTag == reader->NodeName() &&
-         false != reader->ReadElementContent(&label_buffer)) {
+  while (kLabelXmlTag == reader->NodeName()) {
+    std::string label_buffer;
+    if (!reader->ReadElementContent(&label_buffer)) {
+      label_buffer = "";
+    }
     label_vector.push_back(UTF8ToWide(label_buffer));
+    LocateNextOpenTag(reader);
   }
 
-  // if this is the first run then we place favorites with no labels
-  // in the title bar.  Else they are placed in the "Google Toolbar" folder.
-  if (first_run() && 0 == label_vector.size()) {
-    entry->in_toolbar = true;
-  } else {
-    entry->in_toolbar = false;
-    entry->path.push_back(l10n_util::GetString(
-                                      IDS_BOOKMARK_GROUP_FROM_GOOGLE_TOOLBAR));
-  }
-
-  // If there is only one label and it is in the form "xxx:yyy:zzz" this
-  // was created from a Firefox folder.  We undo the label creation and
-  // recreate the correct folder.
-  if (1 == label_vector.size()) {
-    std::vector< std::wstring > folder_names;
-    SplitString(label_vector[0], L':', &folder_names);
-    entry->path.insert(entry->path.end(),
-                       folder_names.begin(), folder_names.end());
-  } else if (0 != label_vector.size()) {
-    std::wstring folder_name = label_vector[0];
-    entry->path.push_back(folder_name);
+  if (0 == label_vector.size()) {
+    if (!FirstRun::IsChromeFirstRun()) {
+      bookmark_folders->resize(1);
+      (*bookmark_folders)[0].push_back(
+          l10n_util::GetString(IDS_BOOKMARK_GROUP_FROM_GOOGLE_TOOLBAR));
+    }    
+    return true;
   }
 
-  return true;
-}
+  // We will be making one bookmark folder for each label.
+  bookmark_folders->resize(label_vector.size());
 
-bool Toolbar5Importer::ExtractFaviconFromXmlReader(
-    XmlReader* reader,
-    ProfileWriter::BookmarkEntry* bookmark_entry,
-    history::ImportedFavIconUsage* favicon_entry) {
-  DCHECK(reader);
-  DCHECK(bookmark_entry);
-  DCHECK(favicon_entry);
-
-  if (!LocateNextTagByName(reader, kAttributesXmlTag))
-    return false;
-  if (!LocateNextTagByName(reader, kAttributeXmlTag))
-    return false;
-  if (!LocateNextTagByName(reader, kNameXmlTag))
-    return false;
-
-  // Attributes are <name>...</name><value>...</value> pairs.  The first
-  // attribute should be the favicon name tage, and the value is the url.
-  std::string buffer;
-  if (!ExtractNamedValueFromXmlReader(reader, kNameXmlTag, &buffer))
-    return false;
-  if (kFaviconAttributeXmlName != buffer)
-    return false;
-  if (!ExtractNamedValueFromXmlReader(reader, kValueXmlTag, &buffer))
-    return false;
-
-  // Validate the url
-  GURL favicon = GURL(buffer);
-  if (!favicon.is_valid())
-    return false;
-
-  favicon_entry->favicon_url = favicon;
-  favicon_entry->urls.insert(bookmark_entry->url);
+  for (size_t index = 0; index < label_vector.size(); ++index) {
+    // If this is the first run then we place favorites with no labels
+    // in the title bar.  Else they are placed in the "Google Toolbar" folder.    
+    if (!FirstRun::IsChromeFirstRun() || !label_vector[index].empty()) {
+      (*bookmark_folders)[index].push_back(
+          l10n_util::GetString(IDS_BOOKMARK_GROUP_FROM_GOOGLE_TOOLBAR));
+    }
+    
+    // If the label and is in the form "xxx:yyy:zzz" this was created from an 
+    // IE or Firefox folder.  We undo the label creation and recreate the correct
+    // folder.
+    std::vector<std::wstring> folder_names;
+    SplitString(label_vector[index], L':', &folder_names);
+    (*bookmark_folders)[index].insert((*bookmark_folders)[index].end(),
+        folder_names.begin(), folder_names.end());
+  }
 
   return true;
 }
 
 // Bookmark creation
 void  Toolbar5Importer::AddBookMarksToChrome(
-    const std::vector< ProfileWriter::BookmarkEntry >& bookmarks,
-    const std::vector< history::ImportedFavIconUsage >& favicons) {
+    const std::vector<ProfileWriter::BookmarkEntry>& bookmarks) {
   if (!bookmarks.empty() && !cancelled()) {
       main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
-          &ProfileWriter::AddBookmarkEntry, bookmarks));
-  }
-
-  if (!favicons.empty()) {
-    main_loop_->PostTask(FROM_HERE, NewRunnableMethod(writer_,
-        &ProfileWriter::AddFavicons, favicons));
+          &ProfileWriter::AddBookmarkEntry, bookmarks, true));
   }
 }
diff --git a/chrome/browser/importer/toolbar_importer.h b/chrome/browser/importer/toolbar_importer.h
index 729e73e..1b82655 100644
--- a/chrome/browser/importer/toolbar_importer.h
+++ b/chrome/browser/importer/toolbar_importer.h
@@ -2,6 +2,17 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// The functionality provided here allows the user to import their bookmarks 
+// (favorites) from Google Toolbar.  
+//
+// Currently the only configuration information we need is to check whether or 
+// not the user currently has their GAIA cookie.  This is done by the functions
+// exposed through the ToolbarImportUtils namespace.
+//
+// Toolbar5Importer is a class which exposes the functionality needed to
+// communicate with the Google Toolbar v5 front-end, negotiate the download of
+// Toolbar bookmarks, parse them, and install them on the client.
+
 #ifndef CHROME_BROWSER_IMPORTER_TOOLBAR_IMPROTER_H__
 #define CHROME_BROWSER_IMPORTER_TOOLBAR_IMPROTER_H__
 
@@ -13,30 +24,10 @@
 
 class XmlReader;
 
-enum TOOLBAR_VERSION {
-  NO_VERSION = -1,
-  DEPRECATED,
-  VERSION_4,
-  VERSION_5
-};
-
-class ToolbarImporterUtils {
- public:
-
-  static bool IsToolbarInstalled();
-  static bool IsGoogleGAIACookieInstalled();
-  static TOOLBAR_VERSION GetToolbarVersion();
-
- private:
-  static const HKEY kToolbarInstallRegistryRoots[2];
-  static const TCHAR* kToolbarRootRegistryFolder;
-  static const TCHAR* kToolbarVersionRegistryFolder;
-  static const TCHAR* kToolbarVersionRegistryKey;
+namespace ToolbarImporterUtils {
 
-  ToolbarImporterUtils() {}
-  ~ToolbarImporterUtils() {}
+bool IsGoogleGAIACookieInstalled();
 
-  DISALLOW_COPY_AND_ASSIGN(ToolbarImporterUtils);
 };
 
 class Toolbar5Importer : public URLFetcher::Delegate,
@@ -49,9 +40,15 @@ class Toolbar5Importer : public URLFetcher::Delegate,
   virtual void StartImport(ProfileInfo profile_info,
                            uint16 items,
                            ProfileWriter* writer,
+                           MessageLoop* delegate_loop,
                            ImporterHost* host);
 
-  // URLFetcher::Delegate method
+  // Importer view call this method when the user clicks the cancel button
+  // in the ImporterView UI.  We need to post a message to our loop
+  // to cancel network retrieval.
+  virtual void Cancel();
+
+  // URLFetcher::Delegate method called back from the URLFetcher object.
   void OnURLFetchComplete(const URLFetcher* source,
                           const GURL& url,
                           const URLRequestStatus& status,
@@ -60,19 +57,23 @@ class Toolbar5Importer : public URLFetcher::Delegate,
                           const std::string& data);
 
  private:
+  FRIEND_TEST(Toolbar5ImporterTest, BookmarkParse);
+ 
   // Internal state
   enum INTERNAL_STATE {
     NOT_USED = -1,
     INITIALIZED,
     GET_AUTHORIZATION_TOKEN,
     GET_BOOKMARKS,
+    PARSE_BOOKMARKS,
     DONE
   };
 
+  typedef std::vector<std::wstring> BOOKMARK_FOLDER;
+
   // URLs for connecting to the toolbar front end
   static const std::string kT5AuthorizationTokenUrl;
   static const std::string kT5FrontEndUrlTemplate;
-  static const std::string kT4FrontEndUrlTemplate;
 
   // Token replacement tags
   static const std::string kRandomNumberToken;
@@ -90,12 +91,9 @@ class Toolbar5Importer : public URLFetcher::Delegate,
   static const std::string kUrlXmlTag;
   static const std::string kTimestampXmlTag;
   static const std::string kLabelsXmlTag;
+  static const std::string kLabelsXmlCloseTag;
   static const std::string kLabelXmlTag;
   static const std::string kAttributesXmlTag;
-  static const std::string kAttributeXmlTag;
-  static const std::string kNameXmlTag;
-  static const std::string kValueXmlTag;
-  static const std::string kFaviconAttributeXmlName;
 
   // Flow control
   void ContinueImport();
@@ -111,39 +109,38 @@ class Toolbar5Importer : public URLFetcher::Delegate,
   // XML Parsing
   bool ParseAuthenticationTokenResponse(const std::string& response,
                                         std::string* token);
-  void ConstructFEConnectionString(const std::string& token,
-                                   std::string* conn_string);
 
-  bool ParseBookmarksFromReader(
+  static bool ParseBookmarksFromReader(
       XmlReader* reader,
-      std::vector< ProfileWriter::BookmarkEntry >* bookmarks,
-      std::vector< history::ImportedFavIconUsage >* favicons);
-
-  bool LocateNextTagByName(XmlReader* reader, const std::string& tag);
-
-  bool ExtractBookmarkInformation(XmlReader* reader,
-                                  ProfileWriter::BookmarkEntry* bookmark_entry,
-                                  history::ImportedFavIconUsage* favicon_entry);
-  bool ExtractNamedValueFromXmlReader(XmlReader* reader,
-                                      const std::string& name,
-                                      std::string* buffer);
-  bool ExtractTitleFromXmlReader(XmlReader* reader,
-                                 ProfileWriter::BookmarkEntry* entry);
-  bool ExtractUrlFromXmlReader(XmlReader* reader,
-                               ProfileWriter::BookmarkEntry* entry);
-  bool ExtractTimeFromXmlReader(XmlReader* reader,
-                                ProfileWriter::BookmarkEntry* entry);
-  bool ExtractFolderFromXmlReader(XmlReader* reader,
-                                  ProfileWriter::BookmarkEntry* entry);
-  bool ExtractFaviconFromXmlReader(
+      std::vector< ProfileWriter::BookmarkEntry >* bookmarks);
+
+  static bool LocateNextOpenTag(XmlReader* reader);
+  static bool LocateNextTagByName(XmlReader* reader, const std::string& tag);
+  static bool LocateNextTagWithStopByName(
+      XmlReader* reader, 
+      const std::string& tag, 
+      const std::string& stop);
+
+  static bool ExtractBookmarkInformation(
       XmlReader* reader,
       ProfileWriter::BookmarkEntry* bookmark_entry,
-      history::ImportedFavIconUsage* favicon_entry);
+      std::vector<BOOKMARK_FOLDER>* bookmark_folders);
+  static bool ExtractNamedValueFromXmlReader(XmlReader* reader,
+                                             const std::string& name,
+                                             std::string* buffer);
+  static bool ExtractTitleFromXmlReader(XmlReader* reader,
+                                        ProfileWriter::BookmarkEntry* entry);
+  static bool ExtractUrlFromXmlReader(XmlReader* reader,
+                                      ProfileWriter::BookmarkEntry* entry);
+  static bool ExtractTimeFromXmlReader(XmlReader* reader,
+                                       ProfileWriter::BookmarkEntry* entry);
+  static bool ExtractFoldersFromXmlReader(
+      XmlReader* reader,
+      std::vector<BOOKMARK_FOLDER>* bookmark_folders);
 
   // Bookmark creation
   void AddBookMarksToChrome(
-    const std::vector< ProfileWriter::BookmarkEntry >& bookmarks,
-    const std::vector< history::ImportedFavIconUsage >& favicons);
+      const std::vector<ProfileWriter::BookmarkEntry>& bookmarks);
 
   // Hosts the writer used in this importer.
   ProfileWriter* writer_;
diff --git a/chrome/browser/importer/toolbar_importer_unittest.cc b/chrome/browser/importer/toolbar_importer_unittest.cc
new file mode 100644
index 0000000..b2b2e3b
--- /dev/null
+++ b/chrome/browser/importer/toolbar_importer_unittest.cc
@@ -0,0 +1,471 @@
+// Copyright (c) 2006-2008 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 "testing/gtest/include/gtest/gtest.h"
+
+#include <string>
+#include <vector>
+
+#include "chrome/browser/importer/importer.h"
+#include "chrome/browser/importer/toolbar_importer.h"
+#include "chrome/common/libxml_utils.h"
+
+
+namespace toolbar_importer_unittest {
+static const std::wstring kTitle = L"MyTitle";
+static const std::wstring kUrl = L"http://www.google.com/";
+static const std::wstring kTimestamp = L"1153328691085181";
+static const std::wstring kFolder = L"Google";
+static const std::wstring kFolder2 = L"Homepage";
+static const std::wstring kFolderArray[3] = {L"Google", L"Search", L"Page"};
+
+static const std::wstring kOtherTitle = L"MyOtherTitle";
+static const std::wstring kOtherUrl = L"http://www.google.com/mail";
+static const std::wstring kOtherFolder = L"Mail";
+
+static const std::string kGoodBookMark =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kGoodBookMarkNoLabel =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kGoodBookMarkTwoLabels =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> <label>Homepage</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kGoodBookMarkFolderLabel =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google:Search:Page</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kGoodBookMarkNoFavicon =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kGoodBookMark2Items =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark>"
+    " <bookmark> "
+    "<title>MyOtherTitle</title> "
+    "<url>http://www.google.com/mail</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Mail</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name>"
+    "<value>http://www.google.com/mail/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1253328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark>"
+    "</bookmarks>";
+static const std::string kEmptyString = "";
+static const std::string kBadBookMarkNoBookmarks =
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kBadBookMarkNoBookmark =
+    " <bookmarks>"
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kBadBookMarkNoTitle =
+    " <bookmarks>"
+    " <bookmark> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kBadBookMarkNoUrl =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kBadBookMarkNoTimestamp =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<labels> <label>Google</label> </labels> "
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+static const std::string kBadBookMarkNoLabels =
+    " <bookmarks>"
+    " <bookmark> "
+    "<title>MyTitle</title> "
+    "<url>http://www.google.com/</url> "
+    "<timestamp>1153328691085181</timestamp> "
+    "<id>N123nasdf239</id> <notebook_id>Bxxxxxxx</notebook_id> "
+    "<section_id>Sxxxxxx</section_id> <has_highlight>0</has_highlight>"
+    "<attributes> "
+    "<attribute> "
+    "<name>favicon_url</name> <value>http://www.google.com/favicon.ico</value> "
+    "</attribute> "
+    "<attribute> "
+    "<name>favicon_timestamp</name> <value>1153328653</value> "
+    "</attribute> "
+    "<attribute> <name>notebook_name</name> <value>My notebook 0</value> "
+    "</attribute> "
+    "<attribute> <name>section_name</name> <value>My section 0 "
+    "</value> </attribute> </attributes> "
+    "</bookmark> </bookmarks>";
+}
+
+// Since the above is very dense to read I enumerate the test cases here.
+// 1. Correct bookmark structure with one label
+// 2. "" with no labels
+// 3. "" with two labels
+// 4. "" with a folder->label translation done by toolbar
+// 5. "" with no favicon
+// 6. Two correct bookmarks.
+// The following are error cases by removing sections:
+// 7. Empty string
+// 8. No <bookmarks>
+// 9. No <bookmark>
+// 10. No <title>
+// 11. No <url>
+// 12. No <timestamp>
+// 13. No <labels>
+TEST(Toolbar5ImporterTest, BookmarkParse) {
+//  bool ParseBookmarksFromReader(
+//    XmlReader* reader,
+//    std::vector<ProfileWriter::BookmarkEntry>* bookmarks,
+//    std::vector< history::ImportedFavIconUsage >* favicons);
+
+  XmlReader reader;
+  std::vector<ProfileWriter::BookmarkEntry> bookmarks;
+
+  const GURL url(toolbar_importer_unittest::kUrl);
+  const GURL other_url(toolbar_importer_unittest::kOtherUrl);
+
+  // Test case 1 - Basic bookmark, single lables
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMark));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(bookmarks.size() == 1);
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(url == bookmarks[0].url);
+  EXPECT_TRUE(2 == bookmarks[0].path.size());
+  EXPECT_TRUE(toolbar_importer_unittest::kFolder == bookmarks[0].path[1]);
+
+  // Test case 2 - No labels
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMarkNoLabel));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(bookmarks.size() == 1);
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(bookmarks[0].url == url);
+  EXPECT_TRUE(1 == bookmarks[0].path.size());
+
+  // Test case 3 - Two labels
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMarkTwoLabels));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(2 == bookmarks.size());
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_FALSE(bookmarks[1].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[1].title);
+  EXPECT_TRUE(bookmarks[0].url == url);
+  EXPECT_TRUE(bookmarks[1].url == url);
+  EXPECT_TRUE(toolbar_importer_unittest::kFolder == bookmarks[0].path[1]);
+  EXPECT_TRUE(toolbar_importer_unittest::kFolder2 == bookmarks[1].path[1]);
+
+  // Test case 4 - Label with a colon (file name translation).
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMarkFolderLabel));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(bookmarks.size() == 1);
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(bookmarks[0].url == url);
+  EXPECT_TRUE(4 == bookmarks[0].path.size());
+  EXPECT_TRUE(toolbar_importer_unittest::kFolderArray[0] ==
+      bookmarks[0].path[1]);
+  EXPECT_TRUE(toolbar_importer_unittest::kFolderArray[1] ==
+      bookmarks[0].path[2]);
+  EXPECT_TRUE(toolbar_importer_unittest::kFolderArray[2] ==
+      bookmarks[0].path[3]);
+
+  // Test case 5 - No favicon
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMarkNoFavicon));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(1 == bookmarks.size());
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(bookmarks[0].url == url);
+  EXPECT_TRUE(2 == bookmarks[0].path.size());
+  EXPECT_TRUE(toolbar_importer_unittest::kFolder == bookmarks[0].path[1]);
+
+  // Test case 6 - Two bookmarks
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kGoodBookMark2Items));
+  EXPECT_TRUE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // verificaiton
+  EXPECT_TRUE(2 == bookmarks.size());
+  EXPECT_FALSE(bookmarks[0].in_toolbar);
+  EXPECT_FALSE(bookmarks[1].in_toolbar);
+  EXPECT_TRUE(toolbar_importer_unittest::kTitle == bookmarks[0].title);
+  EXPECT_TRUE(toolbar_importer_unittest::kOtherTitle == bookmarks[1].title);
+  EXPECT_TRUE(bookmarks[0].url == url);
+  EXPECT_TRUE(bookmarks[1].url == other_url);
+  EXPECT_TRUE(2 == bookmarks[0].path.size());
+  EXPECT_TRUE(toolbar_importer_unittest::kFolder == bookmarks[0].path[1]);
+  EXPECT_TRUE(2 == bookmarks[0].path.size());
+  EXPECT_TRUE(toolbar_importer_unittest::kOtherFolder == bookmarks[1].path[1]);
+
+  // Test case 7 - Empty string
+  bookmarks.clear();
+
+  EXPECT_FALSE(reader.Load(toolbar_importer_unittest::kEmptyString));
+
+  // Test case 8 - No <bookmarks> section.
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoBookmarks));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // Test case 9 - No <bookmark> section
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoBookmark));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+
+  // Test case 10 - No title.
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoTitle));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+
+  // Test case 11 - No URL.
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoUrl));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // Test case 12 - No timestamp.
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoTimestamp));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+
+  // Test case 13
+  bookmarks.clear();
+  EXPECT_TRUE(reader.Load(toolbar_importer_unittest::kBadBookMarkNoLabels));
+  EXPECT_FALSE(Toolbar5Importer::ParseBookmarksFromReader(
+    &reader,
+    &bookmarks));
+}