brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.88k stars 2.34k forks source link

Safari import crashes #26365

Closed nullhook closed 1 year ago

nullhook commented 2 years ago

While building the new onboarding UI, I noticed that the Safari importer crashes. The bookmarks do get imported, even though the crash occurs.

This happens from both brave://welcome and brave://settings/importData. Here is the stack trace:

[37921:17411:1028/084901.895880:ERROR:database.cc(1543)] SQL compilation error: no such table: PageURL. Statement: SELECT iconID, url FROM PageURL;
[37921:17411:1028/084901.913721:FATAL:statement.cc(64)] Cannot call mutating statements on an invalid statement.
0   libbase.dylib                       0x0000000100ff738c base::debug::CollectStackTrace(void**, unsigned long) + 28
1   libbase.dylib                       0x0000000100efc860 base::debug::StackTrace::StackTrace() + 24
2   libbase.dylib                       0x0000000100f19c10 logging::LogMessage::~LogMessage() + 156
3   libsql.dylib                        0x00000001096b9c0c sql::Statement::CheckValid() const + 320
4   libsql.dylib                        0x00000001096b9c44 sql::Statement::StepInternal() + 52
5   libsql.dylib                        0x00000001096b9fb0 sql::Statement::Step() + 124
6   libchrome_dll.dylib                 0x000000010cea8b84 SafariImporter::ImportFaviconURLs(sql::Database*, std::Cr::map<long long, std::Cr::set<GURL, std::Cr::less<GURL>, std::Cr::allocator<GURL>>, std::Cr::less<long long>, std::Cr::allocator<std::Cr::pair<long long const, std::Cr::set<GURL, std::Cr::less<GURL>, std::Cr::allocator<GURL>>>>>*) + 128
7   libchrome_dll.dylib                 0x000000010cea86a4 SafariImporter::ImportBookmarks() + 436
8   libchrome_dll.dylib                 0x000000010cea8430 SafariImporter::StartImport(importer::SourceProfile const&, unsigned short, ImporterBridge*) + 308
9   libchrome_dll.dylib                 0x000000010ce9cfb8 base::internal::Invoker<base::internal::BindState<void (Importer::*)(importer::SourceProfile const&, unsigned short, ImporterBridge*), scoped_refptr<Importer>, importer::SourceProfile, unsigned short, base::internal::RetainedRefWrapper<BraveExternalProcessImporterBridge>>, void ()>::RunOnce(base::internal::BindStateBase*) + 72
10  libbase.dylib                       0x0000000100f8334c base::TaskAnnotator::RunTaskImpl(base::PendingTask&) + 328
11  libbase.dylib                       0x0000000100fa97fc base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) + 1288
12  libbase.dylib                       0x0000000100fa8de8 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() + 108
13  libbase.dylib                       0x0000000100f26710 base::MessagePumpDefault::Run(base::MessagePump::Delegate*) + 160
14  libbase.dylib                       0x0000000100faa434 base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) + 628
15  libbase.dylib                       0x0000000100f59e78 base::RunLoop::Run(base::Location const&) + 668
16  libbase.dylib                       0x0000000100fd9a74 base::Thread::Run(base::RunLoop*) + 240
17  libbase.dylib                       0x0000000100fd9dc0 base::Thread::ThreadMain() + 784
18  libbase.dylib                       0x0000000101007424 base::(anonymous namespace)::ThreadFunc(void*) + 132
19  libsystem_pthread.dylib             0x000000018d0a426c _pthread_start + 148
20  libsystem_pthread.dylib             0x000000018d09f08c thread_start + 8
Task trace:
0   libchrome_dll.dylib                 0x000000010cea7e40 ProfileImportImpl::StartImport(importer::SourceProfile const&, unsigned short, base::flat_map<unsigned int, std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>>, std::Cr::less<void>, std::Cr::vector<std::Cr::pair<unsigned int, std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>>>, std::Cr::allocator<std::Cr::pair<unsigned int, std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char>>>>>> const&, mojo::PendingRemote<chrome::mojom::ProfileImportObserver>) + 544
1   libmojo_public_system_cpp.dylib     0x0000000100813210 mojo::SimpleWatcher::ArmOrNotify() + 332
2   libcontent.dylib                    0x0000000105c00960 content::(anonymous namespace)::ServiceBinderImpl::BindServiceInterface(mojo::GenericPendingReceiver*) + 716
3   libbindings.dylib                   0x00000001015ad4c4 mojo::Connector::PostDispatchNextMessageFromPipe() + 120
4   libmojo_public_system_cpp.dylib     0x0000000100813210 mojo::SimpleWatcher::ArmOrNotify() + 332
Task trace buffer limit hit, update PendingTask::kTaskBacktraceLength to increase.
Crash keys:
  "service-name" = "chrome.mojom.ProfileImport"
  "switch-15" = "--enable-features=ui-debug-tools,SpeedreaderPanelV2"
  "switch-14" = "--variations-insecure-server-url"
  "switch-13" = "--variations-server-url"
  "switch-12" = "--lso-url=https://no-thanks.invalid"
  "switch-11" = "--sync-url=https://sync-v2.brave.software/v2"
  "switch-10" = "--origin-trial-public-key=bYUKPJoPnCxeNvu72j4EmPuK7tr1PAC7SHh8ld"
  "switch-9" = "--component-updater=url-source=https://go-updater.brave.com/exte"
  "switch-8" = "--enable-dom-distiller"
  "switch-7" = "--disable-domain-reliability"
  "switch-6" = "--enable-features=SpeedreaderPanelV2,ui-debug-tools"
  "switch-5" = "--field-trial-handle=1718379636,r,7428187872369533611,1752546452"
  "switch-4" = "--shared-files"
  "switch-3" = "--service-sandbox-type=none"
  "switch-2" = "--lang=en-US"
  "switch-1" = "--utility-sub-type=chrome.mojom.ProfileImport"
  "num-switches" = "20"
  "osarch" = "arm64"
  "pid" = "37921"
  "ptype" = "utility"

[37894:259:1028/084902.186985:ERROR:external_process_importer_client.cc(96)] OnProcessCrashed
stephendonner commented 1 year ago

Verification PASSED using

Brave 1.48.119 Chromium: 109.0.5414.80 (Official Build) beta (x86_64)
Revision 0f69b168d36a06cace4365e9f029fa987afa5633-refs/branch-heads/5414@{#1178}
OS macOS Version 11.7.2 (Build 20G1020)

Prerequisite: populate Safari with a good set of bookmarks

Safari bookmarks from brave://welcome from brave://settings/importData
Screen Shot 2023-01-10 at 6 09 35 PM Screen Shot 2023-01-10 at 6 10 55 PM Screen Shot 2023-01-10 at 6 12 11 PM

brave://welcome - PASSED

1. new profile 2. launched Brave 3. clicked `Skip` on the 1st page of `brave://welcome` 4. selected the `Safari` icon and text on the next screen 5. clicked `Import` 6. clicked `Finish` 7. opened `brave://bookmarks` 8. compared the list with Safari's #### Confirmed all bookmarks from `Safari` were imported without crashing example | example | example | example ---------|----------|---------|--------- Screen Shot 2023-01-10 at 6 10 32 PM | Screen Shot 2023-01-10 at 6 10 35 PM | Screen Shot 2023-01-10 at 6 10 41 PM | Screen Shot 2023-01-10 at 6 10 55 PM

brave://settings/importData - PASSED

1. new profile 2. launched Brave 3. opened `brave://settings/importData` 4. noted `Safari` was preselected 5. clicked `Import` 6. clicked `Done` 7. opened `brave://bookmarks` 8. compared the list with Safari's #### Confirmed all bookmarks from `Safari` were imported without crashing example | example | example ---------|---------|---------- Screen Shot 2023-01-10 at 6 11 55 PM | Screen Shot 2023-01-10 at 6 11 58 PM | Screen Shot 2023-01-10 at 6 12 11 PM