From 6c1e51af346f2ec01590fc45ea5b51eade1d2a66 Mon Sep 17 00:00:00 2001
From: Marc Treib <treib@chromium.org>
Date: Tue, 04 Aug 2026 07:37:27 -0700
Subject: [PATCH] ProfileImporter: Use mojom::BookmarkHtmlParser

ParseBookmarksUnsafe() is documented as needing to run inside the
BookmarkHtmlParser service, but the desktop ProfileImport utility still
called it directly from BookmarksFileImporter and from FirefoxImporter's
default-bookmarks loader.

After this CL, ExternalProcessImporterClient launches a
BookmarkHtmlParser service alongside the ProfileImport service itself,
and passes a remote to it through StartImport(). The Importer base class
now has a SetBookmarkHtmlParser() method to facilitate parsing over
Mojo, and both relevant importer implementations are updated to use the
new bookmarks parser and wait for the reply from the async Mojo
callback.

Fixed: 513757918
Change-Id: Ic2268611ccb446f75751d487e6ab59a26a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7864321
Reviewed-by: Filipa Senra <fsenra@google.com>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1673326}
---

diff --git a/chrome/browser/importer/DEPS b/chrome/browser/importer/DEPS
new file mode 100644
index 0000000..6f0957d4
--- /dev/null
+++ b/chrome/browser/importer/DEPS
@@ -0,0 +1,3 @@
+include_rules = [
+  "+components/user_data_importer/mojom",
+]
diff --git a/chrome/browser/importer/external_process_importer_client.cc b/chrome/browser/importer/external_process_importer_client.cc
index 412452d7..cf31971 100644
--- a/chrome/browser/importer/external_process_importer_client.cc
+++ b/chrome/browser/importer/external_process_importer_client.cc
@@ -16,6 +16,7 @@
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/user_data_importer/common/imported_bookmark_entry.h"
+#include "components/user_data_importer/mojom/bookmark_html_parser.mojom.h"
 #include "content/public/browser/service_process_host.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -69,10 +70,22 @@
       IDS_BOOKMARK_BAR_FOLDER_NAME,
       l10n_util::GetStringUTF8(IDS_BOOKMARK_BAR_FOLDER_NAME));
 
+  mojo::PendingRemote<user_data_importer::mojom::BookmarkHtmlParser>
+      bookmark_html_parser;
+  // Note: `FAVORITES` corresponds to bookmarks.
+  if (items_ & user_data_importer::FAVORITES) {
+    content::ServiceProcessHost::Launch(
+        bookmark_html_parser.InitWithNewPipeAndPassReceiver(),
+        content::ServiceProcessHost::Options()
+            .WithDisplayName(IDS_CONTENT_BOOKMARK_PARSER_SERVICE_DISPLAY_NAME)
+            .Pass());
+  }
+
   // If the utility process hasn't started yet the message will queue until it
   // does.
   profile_import_->StartImport(source_profile_, items_, localized_strings,
-                               receiver_.BindNewPipeAndPassRemote());
+                               receiver_.BindNewPipeAndPassRemote(),
+                               std::move(bookmark_html_parser));
 }
 
 void ExternalProcessImporterClient::Cancel() {
diff --git a/chrome/common/importer/BUILD.gn b/chrome/common/importer/BUILD.gn
index c719a21..bc73d6fe 100644
--- a/chrome/common/importer/BUILD.gn
+++ b/chrome/common/importer/BUILD.gn
@@ -10,6 +10,7 @@
   sources = [ "profile_import.mojom" ]
 
   public_deps = [
+    "//components/user_data_importer/mojom",
     "//mojo/public/mojom/base",
     "//sandbox/policy/mojom",
     "//url/mojom:url_mojom_gurl",
diff --git a/chrome/common/importer/profile_import.mojom b/chrome/common/importer/profile_import.mojom
index b28b732..815b911 100644
--- a/chrome/common/importer/profile_import.mojom
+++ b/chrome/common/importer/profile_import.mojom
@@ -4,6 +4,7 @@
 
 module chrome.mojom;
 
+import "components/user_data_importer/mojom/bookmark_html_parser.mojom";
 import "mojo/public/mojom/base/string16.mojom";
 import "sandbox/policy/mojom/sandbox.mojom";
 import "url/mojom/url.mojom";
@@ -93,7 +94,9 @@
       SourceProfile source_profile,
       uint16 items,
       map<uint32, string> localized_strings,
-      pending_remote<ProfileImportObserver> observer);
+      pending_remote<ProfileImportObserver> observer,
+      pending_remote<user_data_importer.mojom.BookmarkHtmlParser>?
+          bookmark_html_parser);
 
   // Stop the importer.
   CancelImport();
diff --git a/chrome/utility/BUILD.gn b/chrome/utility/BUILD.gn
index 48138252..500cbc7e 100644
--- a/chrome/utility/BUILD.gn
+++ b/chrome/utility/BUILD.gn
@@ -292,6 +292,7 @@
       "//chrome/common/importer:test_support",
       "//components/favicon_base",
       "//components/user_data_importer/common",
+      "//components/user_data_importer/content:test_support",
       "//components/user_data_importer/utility:safari_data_importer",
       "//content/test:test_support",
       "//sql",
diff --git a/chrome/utility/importer/bookmarks_file_importer.cc b/chrome/utility/importer/bookmarks_file_importer.cc
index 6603970..18840df6 100644
--- a/chrome/utility/importer/bookmarks_file_importer.cc
+++ b/chrome/utility/importer/bookmarks_file_importer.cc
@@ -21,8 +21,10 @@
 #include "components/user_data_importer/common/imported_bookmark_entry.h"
 #include "components/user_data_importer/common/importer_data_types.h"
 #include "components/user_data_importer/content/content_bookmark_parser_utils.h"
+#include "components/user_data_importer/mojom/bookmark_html_parser.mojom.h"
 #include "components/user_data_importer/utility/bookmark_parser.h"
 #include "content/public/common/url_constants.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 namespace internal {
 
@@ -82,6 +84,11 @@
 
 BookmarksFileImporter::~BookmarksFileImporter() = default;
 
+void BookmarksFileImporter::SetBookmarkHtmlParser(
+    mojo::PendingRemote<user_data_importer::mojom::BookmarkHtmlParser> parser) {
+  html_parser_remote_ = std::move(parser);
+}
+
 void BookmarksFileImporter::StartImport(
     const user_data_importer::SourceProfile& source_profile,
     uint16_t items,
@@ -99,9 +106,24 @@
   // ReadFileToString can return false, but still populate something into
   // `raw_html`. In that case, try to recover as much data as possible.
   base::ReadFileToString(source_profile.source_path, &raw_html);
-  user_data_importer::BookmarkParser::ParsedBookmarks parsed_bookmarks =
-      user_data_importer::ParseBookmarksUnsafe(raw_html);
 
+  CHECK(html_parser_remote_.is_valid());
+  auto html_parser = std::make_unique<
+      mojo::Remote<user_data_importer::mojom::BookmarkHtmlParser>>(
+      std::move(html_parser_remote_));
+
+  auto* raw_parser = html_parser.get();
+  (*raw_parser)
+      ->Parse(
+          raw_html,
+          base::BindOnce(&BookmarksFileImporter::OnBookmarksParsed,
+                         base::WrapRefCounted(this), std::move(html_parser)));
+}
+
+void BookmarksFileImporter::OnBookmarksParsed(
+    std::unique_ptr<mojo::Remote<user_data_importer::mojom::BookmarkHtmlParser>>
+        html_parser,
+    user_data_importer::BookmarkParser::ParsedBookmarks parsed_bookmarks) {
   if (!parsed_bookmarks.bookmarks.empty()) {
     std::u16string first_folder_name =
         bridge_->GetLocalizedString(IDS_BOOKMARK_GROUP);
diff --git a/chrome/utility/importer/bookmarks_file_importer.h b/chrome/utility/importer/bookmarks_file_importer.h
index 1b0adbe..3371999 100644
--- a/chrome/utility/importer/bookmarks_file_importer.h
+++ b/chrome/utility/importer/bookmarks_file_importer.h
@@ -8,6 +8,10 @@
 #include <stdint.h>
 
 #include "chrome/utility/importer/importer.h"
+#include "components/user_data_importer/mojom/bookmark_html_parser.mojom-forward.h"
+#include "components/user_data_importer/utility/bookmark_parser.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 // Importer for bookmarks files.
 class BookmarksFileImporter : public Importer {
@@ -21,8 +25,20 @@
                    uint16_t items,
                    ImporterBridge* bridge) override;
 
+  void SetBookmarkHtmlParser(
+      mojo::PendingRemote<user_data_importer::mojom::BookmarkHtmlParser> parser)
+      override;
+
  private:
   ~BookmarksFileImporter() override;
+
+  void OnBookmarksParsed(

diff --git a/chrome/utility/importer/bookmarks_file_importer_unittest.cc b/chrome/utility/importer/bookmarks_file_importer_unittest.cc
index 253407b..cb786262 100644
--- a/chrome/utility/importer/bookmarks_file_importer_unittest.cc
+++ b/chrome/utility/importer/bookmarks_file_importer_unittest.cc
@@ -11,12 +11,16 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ref_counted.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "chrome/common/importer/importer_autofill_form_data_entry.h"
 #include "chrome/common/importer/importer_bridge.h"
 #include "components/user_data_importer/common/imported_bookmark_entry.h"
 #include "components/user_data_importer/common/importer_data_types.h"
+#include "components/user_data_importer/content/fake_bookmark_html_parser.h"
+#include "content/public/test/browser_task_environment.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -87,8 +91,15 @@
   ~MockImporterBridge() override = default;
 };
 
+class BookmarksFileImporterTest : public testing::Test {
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  user_data_importer::FakeBookmarkHtmlParser fake_parser_;
+  mojo::Receiver<user_data_importer::mojom::BookmarkHtmlParser> receiver_{
+      &fake_parser_};
+};
 
-TEST(BookmarksFileImporterTest, CanImportURL) {
+TEST_F(BookmarksFileImporterTest, CanImportURL) {
   struct TestCase {
     const std::string url;
     const bool can_be_imported;
@@ -117,7 +128,7 @@
   }
 }
 
-TEST(BookmarksFileImporterTest, ImportBookmarks) {
+TEST_F(BookmarksFileImporterTest, ImportBookmarks) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath file_path = temp_dir.GetPath().AppendASCII("bookmarks.html");
@@ -148,15 +159,19 @@
   bookmark.creation_time = base::Time::UnixEpoch() + base::Seconds(456);
   expected_bookmarks.push_back(bookmark);
 
+  base::RunLoop run_loop;
   EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   EXPECT_CALL(*bridge, AddBookmarks(expected_bookmarks, _));
 
+  importer->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
   importer->StartImport(source_profile, user_data_importer::FAVORITES,
                         bridge.get());
+  run_loop.Run();
 }
 
-TEST(BookmarksFileImporterTest, ImportEmptyFile) {
+TEST_F(BookmarksFileImporterTest, ImportEmptyFile) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath file_path = temp_dir.GetPath().AppendASCII("bookmarks.html");
@@ -168,17 +183,21 @@
   user_data_importer::SourceProfile source_profile;
   source_profile.source_path = file_path;
 
+  base::RunLoop run_loop;
   EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   EXPECT_CALL(*bridge, AddBookmarks(_, _)).Times(0);
   EXPECT_CALL(*bridge, SetKeywords(_, _)).Times(0);
   EXPECT_CALL(*bridge, SetFavicons(_)).Times(0);
 
+  importer->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
   importer->StartImport(source_profile, user_data_importer::FAVORITES,
                         bridge.get());
+  run_loop.Run();
 }
 
-TEST(BookmarksFileImporterTest, ImportWithInvalidBookmarks) {
+TEST_F(BookmarksFileImporterTest, ImportWithInvalidBookmarks) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath file_path = temp_dir.GetPath().AppendASCII("bookmarks.html");
@@ -204,15 +223,19 @@
   bookmark.creation_time = base::Time::UnixEpoch() + base::Seconds(123);
   expected_bookmarks.push_back(bookmark);
 
+  base::RunLoop run_loop;
   EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   EXPECT_CALL(*bridge, AddBookmarks(expected_bookmarks, _));
 
+  importer->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
   importer->StartImport(source_profile, user_data_importer::FAVORITES,
                         bridge.get());
+  run_loop.Run();
 }
 
-TEST(BookmarksFileImporterTest, ImportSearchEngine) {
+TEST_F(BookmarksFileImporterTest, ImportSearchEngine) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath file_path = temp_dir.GetPath().AppendASCII("bookmarks.html");
@@ -235,16 +258,20 @@
   search_engine.display_name = u"Google Search";
   expected_search_engines.push_back(search_engine);
 
+  base::RunLoop run_loop;
   EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   EXPECT_CALL(*bridge, AddBookmarks(_, _)).Times(0);
   EXPECT_CALL(*bridge, SetKeywords(expected_search_engines, false));
 
+  importer->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
   importer->StartImport(source_profile, user_data_importer::FAVORITES,
                         bridge.get());
+  run_loop.Run();
 }
 
-TEST(BookmarksFileImporterTest, ImportWithFavicon) {
+TEST_F(BookmarksFileImporterTest, ImportWithFavicon) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   base::FilePath file_path = temp_dir.GetPath().AppendASCII("bookmarks.html");
@@ -261,8 +288,10 @@
   user_data_importer::SourceProfile source_profile;
   source_profile.source_path = file_path;
 
+  base::RunLoop run_loop;
   EXPECT_CALL(*bridge, NotifyStarted());
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
   EXPECT_CALL(*bridge,
               AddBookmarks(_, _));  // Expecting a bookmark to be added.
   EXPECT_CALL(
@@ -274,7 +303,9 @@
                 Contains(GURL("http://www.google.com/"))),
           Field(&favicon_base::FaviconUsageData::png_data, Not(IsEmpty()))))));
 
+  importer->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
   importer->StartImport(source_profile, user_data_importer::FAVORITES,
                         bridge.get());
+  run_loop.Run();
 }
 }  // namespace
diff --git a/chrome/utility/importer/firefox_importer_unittest.cc b/chrome/utility/importer/firefox_importer_unittest.cc
index 6e202ee..d5ae875 100644
--- a/chrome/utility/importer/firefox_importer_unittest.cc
+++ b/chrome/utility/importer/firefox_importer_unittest.cc
@@ -12,6 +12,7 @@
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
+#include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/common/chrome_paths.h"
@@ -20,7 +21,9 @@
 #include "components/user_data_importer/common/imported_bookmark_entry.h"
 #include "components/user_data_importer/common/importer_data_types.h"
 #include "components/user_data_importer/common/importer_url_row.h"
+#include "components/user_data_importer/content/fake_bookmark_html_parser.h"
 #include "content/public/test/browser_task_environment.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "sql/database.h"
 #include "sql/test/test_helpers.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -45,6 +48,7 @@
     user_data_importer::SourceProfile profile;
     profile.source_path = places_path;
 
+    base::RunLoop run_loop;
     EXPECT_CALL(*bridge_, NotifyStarted());
     EXPECT_CALL(*bridge_, NotifyItemStarted(user_data_importer::FAVORITES));
     EXPECT_CALL(*bridge_, AddBookmarks(_, _))
@@ -52,17 +56,24 @@
     EXPECT_CALL(*bridge_, SetFavicons(_))
         .WillOnce(::testing::SaveArg<0>(favicons));
     EXPECT_CALL(*bridge_, NotifyItemEnded(user_data_importer::FAVORITES));
-    EXPECT_CALL(*bridge_, NotifyEnded());
+    EXPECT_CALL(*bridge_, NotifyEnded())
+        .WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
+
+    importer_->SetBookmarkHtmlParser(receiver_.BindNewPipeAndPassRemote());
     importer_->StartImport(profile, user_data_importer::FAVORITES,
                            bridge_.get());
+    run_loop.Run();
   }
 
- private:
+ protected:
   content::BrowserTaskEnvironment task_environment_;
   scoped_refptr<MockImporterBridge> bridge_ =
       base::MakeRefCounted<MockImporterBridge>();
   scoped_refptr<FirefoxImporter> importer_ =
       base::MakeRefCounted<FirefoxImporter>();
+  user_data_importer::FakeBookmarkHtmlParser fake_parser_;
+  mojo::Receiver<user_data_importer::mojom::BookmarkHtmlParser> receiver_{
+      &fake_parser_};
 };
 
 TEST_F(FirefoxImporterTest, ImportBookmarks_Firefox48) {
@@ -155,14 +166,23 @@
   profile.source_path = source_temp_dir.GetPath();
   scoped_refptr<MockImporterBridge> bridge =
       base::MakeRefCounted<MockImporterBridge>();
+  base::RunLoop run_loop1;
   EXPECT_CALL(*bridge, NotifyStarted());
   EXPECT_CALL(*bridge, NotifyItemStarted(user_data_importer::FAVORITES));
   EXPECT_CALL(*bridge, AddBookmarks(_, _)).Times(0);
   EXPECT_CALL(*bridge, SetFavicons(_)).Times(0);
   EXPECT_CALL(*bridge, NotifyItemEnded(user_data_importer::FAVORITES));
-  EXPECT_CALL(*bridge, NotifyEnded());
+  EXPECT_CALL(*bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop1, &base::RunLoop::Quit));
+
+  user_data_importer::FakeBookmarkHtmlParser fake_parser1;
+  mojo::Receiver<user_data_importer::mojom::BookmarkHtmlParser> receiver1{
+      &fake_parser1};
+  first_importer->SetBookmarkHtmlParser(receiver1.BindNewPipeAndPassRemote());
+
   first_importer->StartImport(profile, user_data_importer::FAVORITES,
                               bridge.get());
+  run_loop1.Run();
 
   // Part 2: Test GetWholeBookmarkFolder validation
   base::ScopedTempDir second_source_dir;
@@ -201,14 +221,23 @@
   second_profile.source_path = second_source_dir.GetPath();
   scoped_refptr<MockImporterBridge> second_bridge =
       base::MakeRefCounted<MockImporterBridge>();
+  base::RunLoop run_loop2;
   EXPECT_CALL(*second_bridge, NotifyStarted());
   EXPECT_CALL(*second_bridge, NotifyItemStarted(user_data_importer::FAVORITES));
   EXPECT_CALL(*second_bridge, AddBookmarks(_, _)).Times(0);
   EXPECT_CALL(*second_bridge, SetFavicons(_)).Times(0);
   EXPECT_CALL(*second_bridge, NotifyItemEnded(user_data_importer::FAVORITES));
-  EXPECT_CALL(*second_bridge, NotifyEnded());
+  EXPECT_CALL(*second_bridge, NotifyEnded())
+      .WillOnce(testing::InvokeWithoutArgs(&run_loop2, &base::RunLoop::Quit));
+
+  user_data_importer::FakeBookmarkHtmlParser fake_parser2;
+  mojo::Receiver<user_data_importer::mojom::BookmarkHtmlParser> receiver2{
+      &fake_parser2};
+  second_importer->SetBookmarkHtmlParser(receiver2.BindNewPipeAndPassRemote());
+
   second_importer->StartImport(second_profile, user_data_importer::FAVORITES,
                                second_bridge.get());
+  run_loop2.Run();
 }
 
 TEST_F(FirefoxImporterTest, ImportHistorySchema) {