Read bookmarks from 'NETSCAPE-Bookmark-file's that don't have a beginning <DT> tag.

chrome/utility/importer/bookmark_html_reader.cc

 // Copyright 2013 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 "chrome/utility/importer/bookmark_html_reader.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/callback.h"
@@ skipping 73 matching lines @@
   // We only have one URL per favicon for Firefox 2 bookmarks.
   usage.urls.insert(link_url);
 
   favicons->push_back(usage);
 }
 
 }  // namespace
 
 namespace bookmark_html_reader {
 
+static void stripDt(std::string& line) {
+  // Remove "<DT>" if the line starts with "<DT>".  This may not occur if
+  // "<DT>" was on the previous line.  Liberally accept entries that do not
+  // have an opening "<DT>" at all.
+  static const char kDtTag[] = "<DT>";
+  if (base::StartsWith(line, kDtTag,
+                       base::CompareCase::INSENSITIVE_ASCII)) {
+    line.erase(0, arraysize(kDtTag) - 1);
+    base::TrimString(line, " ", &line);
+  }
+}
+
 void ImportBookmarksFile(
     const base::Callback<bool(void)>& cancellation_callback,
     const base::Callback<bool(const GURL&)>& valid_url_callback,
     const base::FilePath& file_path,
     std::vector<ImportedBookmarkEntry>* bookmarks,
     std::vector<importer::SearchEngineInfo>* search_engines,
     favicon_base::FaviconUsageDataList* favicons) {
   std::string content;
   base::ReadFileToString(file_path, &content);
   std::vector<std::string> lines = base::SplitString(
@@ skipping 187 matching lines @@
     if (begin == std::string::npos)
       return false;
     begin += std::string(kCharset).size();
     size_t end = line.find_first_of('\"', begin);
     *charset = line.substr(begin, end - begin);
     return true;
   }
   return false;
 }
 
-bool ParseFolderNameFromLine(const std::string& line,
+bool ParseFolderNameFromLine(std::string line,

[Ilya Sherman, 2016/04/14 02:22:53]

Even if you want to modify the variable in the method implementation, it's
preferred to pass the string by const-ref for the method signature, and then
make a copy inline, as needed.  This way, the interface does not change even if
the implementation changes to no longer need the copy; and moreover people
reading the interface don't spend any time wondering why one of the strings is
passed by const-ref, and the other by value.

[Tom (Use chromium acct), 2016/04/14 17:21:44]

Done.

                              const std::string& charset,
                              base::string16* folder_name,
                              bool* is_toolbar_folder,
                              base::Time* add_date) {
-  const char kFolderOpen[] = "<DT><H3";
+  const char kFolderOpen[] = "<H3";
   const char kFolderClose[] = "</H3>";
   const char kToolbarFolderAttribute[] = "PERSONAL_TOOLBAR_FOLDER";
   const char kAddDateAttribute[] = "ADD_DATE";
 
+  stripDt(line);

[Ilya Sherman, 2016/04/14 02:22:53]

Hmm, why do you need to also strip DT tags here, rather than just in the spot
where HRs are stripped?

[Tom (Use chromium acct), 2016/04/14 17:21:44]

I removed the code that strips the DT near where were strip the HR because some
unit tests were failing that expected the Parse*FromLine functions to work with
a leading <DT>.

+
   if (!base::StartsWith(line, kFolderOpen, base::CompareCase::SENSITIVE))
     return false;
 
   size_t end = line.find(kFolderClose);
   size_t tag_end = line.rfind('>', end) + 1;
   // If no end tag or start tag is broken, we skip to find the folder name.
   if (end == std::string::npos || tag_end < arraysize(kFolderOpen))
     return false;
 
   base::CodepageToUTF16(line.substr(tag_end, end - tag_end), charset.c_str(),
@@ skipping 15 matching lines @@
 
   if (GetAttribute(attribute_list, kToolbarFolderAttribute, &value) &&
       base::LowerCaseEqualsASCII(value, "true"))
     *is_toolbar_folder = true;
   else
     *is_toolbar_folder = false;
 
   return true;
 }
 
-bool ParseBookmarkFromLine(const std::string& line,
+bool ParseBookmarkFromLine(std::string line,
                            const std::string& charset,
                            base::string16* title,
                            GURL* url,
                            GURL* favicon,
                            base::string16* shortcut,
                            base::Time* add_date,
                            base::string16* post_data) {
-  const char kItemOpen[] = "<DT><A";
+  const char kItemOpen[] = "<A";
   const char kItemClose[] = "</A>";
   const char kFeedURLAttribute[] = "FEEDURL";
   const char kHrefAttribute[] = "HREF";
   const char kIconAttribute[] = "ICON";
   const char kShortcutURLAttribute[] = "SHORTCUTURL";
   const char kAddDateAttribute[] = "ADD_DATE";
   const char kPostDataAttribute[] = "POST_DATA";
 
+  stripDt(line);
   title->clear();
   *url = GURL();
   *favicon = GURL();
   shortcut->clear();
   post_data->clear();
   *add_date = base::Time();
 
   if (!base::StartsWith(line, kItemOpen, base::CompareCase::SENSITIVE))
     return false;
 
@@ skipping 50 matching lines @@
   // Post data.
   if (GetAttribute(attribute_list, kPostDataAttribute, &value)) {
     base::CodepageToUTF16(value, charset.c_str(),
                           base::OnStringConversionError::SKIP, post_data);
     *post_data = net::UnescapeForHTML(*post_data);
   }
 
   return true;
 }
 
-bool ParseMinimumBookmarkFromLine(const std::string& line,
+bool ParseMinimumBookmarkFromLine(std::string line,
                                   const std::string& charset,
                                   base::string16* title,
                                   GURL* url) {
-  const char kItemOpen[] = "<DT><A";
+  const char kItemOpen[] = "<A";
   const char kItemClose[] = "</";
   const char kHrefAttributeUpper[] = "HREF";
   const char kHrefAttributeLower[] = "href";
 
+  stripDt(line);
   title->clear();
   *url = GURL();
 
   // Case-insensitive check of open tag.
   if (!base::StartsWith(line, kItemOpen, base::CompareCase::INSENSITIVE_ASCII))
     return false;
 
   // Find any close tag.
   size_t end = line.find(kItemClose);
   size_t tag_end = line.rfind('>', end) + 1;
@@ skipping 23 matching lines @@
       *url = GURL(value);
     }
   }
 
   return true;
 }
 
 }  // namespace internal
 
 }  // namespace bookmark_html_reader