From c65b0fdbc8c66218098ee6f697104dbc5bb7ee9f Mon Sep 17 00:00:00 2001 From: Mario Sanchez Prada Date: Wed, 16 Sep 2020 22:06:07 +0200 Subject: [PATCH] Revert "Import History from Safari" Importing History from Safari is no longer possible as macOS 10.14 is much stricter now, and Chromium's support is no longer available so let's drop this patch and keep only the bookmarks import feature. This reverts commit c9050d2f8ea9d04a934066133d332d60fd1c7b3a. Chromium change: https://chromium.googlesource.com/chromium/src/+/b2a8ac9a97c656e2e5d18f0b1948667f96117b48 commit b2a8ac9a97c656e2e5d18f0b1948667f96117b48 Author: Avi Drissman Date: Fri Sep 11 18:41:32 2020 +0000 Clean up Safari importing Starting in macOS 10.14, access to the Safari bookmarks file was restricted. Chromium didn't notice, and would proceed anyway, because it assumed that if the file was there, it could be read. That's not true in the general case anyway, so fix it. History import from Safari broke many years ago when they moved to a database for their storage from using a plist. No one realized that it broke, and since it's access restricted from 10.14 on, there's no point in fixing it, so this CL removes it. Bug: 850225 --- app/brave_generated_resources.grd | 2 +- .../settings/brave_import_data_handler_mac.mm | 34 +-------- .../common/importer/safari_importer_utils.mm | 20 ------ .../utility/importer/importer_creator.cc | 10 --- .../chrome/utility/importer/safari_importer.h | 19 ----- ...e-utility-importer-safari_importer.h.patch | 12 ---- utility/BUILD.gn | 13 ---- utility/importer/brave_safari_importer.h | 25 ------- utility/importer/brave_safari_importer.mm | 71 ------------------- 9 files changed, 4 insertions(+), 202 deletions(-) delete mode 100644 chromium_src/chrome/common/importer/safari_importer_utils.mm delete mode 100644 chromium_src/chrome/utility/importer/importer_creator.cc delete mode 100644 chromium_src/chrome/utility/importer/safari_importer.h delete mode 100644 patches/chrome-utility-importer-safari_importer.h.patch delete mode 100644 utility/importer/brave_safari_importer.h delete mode 100644 utility/importer/brave_safari_importer.mm diff --git a/app/brave_generated_resources.grd b/app/brave_generated_resources.grd index 52e32720cfdc..bf9054084fb5 100644 --- a/app/brave_generated_resources.grd +++ b/app/brave_generated_resources.grd @@ -938,7 +938,7 @@ By installing this extension, you are agreeing to the Google Widevine Terms of U Full Disk Access required - Brave needs Full Disk Access to import your Bookmarks and History from Safari. + Brave needs Full Disk Access to import your Bookmarks from Safari. Learn how to grant Full Disk Access from your System Preferences. diff --git a/browser/ui/webui/settings/brave_import_data_handler_mac.mm b/browser/ui/webui/settings/brave_import_data_handler_mac.mm index 9a44093a94d2..15bf7f367b40 100644 --- a/browser/ui/webui/settings/brave_import_data_handler_mac.mm +++ b/browser/ui/webui/settings/brave_import_data_handler_mac.mm @@ -32,26 +32,6 @@ using content::BrowserThread; -class ScopedOpenFile { - public: - explicit ScopedOpenFile(const base::FilePath& file_path) { - file_ = base::OpenFile(file_path, "r"); - } - - ~ScopedOpenFile() { - if (file_) - base::CloseFile(file_); - } - - bool CanRead() const { return !!file_; } - - ScopedOpenFile(const ScopedOpenFile&) = delete; - ScopedOpenFile& operator=(const ScopedOpenFile&) = delete; - - private: - FILE* file_ = nullptr; -}; - class FullDiskAccessConfirmDialogDelegate : public TabModalConfirmDialogDelegate { public: @@ -124,8 +104,7 @@ bool HasProperDiskAccessPermission(uint16_t imported_items) { if (imported_items & importer::FAVORITES) { const base::FilePath bookmarks_path = safari_dir.Append("Bookmarks.plist"); - ScopedOpenFile open_file(bookmarks_path); - if(!open_file.CanRead()) { + if(!PathIsWritable(bookmarks_path)) { LOG(ERROR) << __func__ << " " << bookmarks_path << " is not accessible." << " Please check full disk access permission."; return false; @@ -133,15 +112,8 @@ bool HasProperDiskAccessPermission(uint16_t imported_items) { } if (imported_items & importer::HISTORY) { - // HISTORY is set if plist or db exists. - base::FilePath history_path = safari_dir.Append("History.plist"); - if (!base::PathExists(history_path)) { - history_path = safari_dir.Append("History.db"); - DCHECK(base::PathExists(history_path)); - } - - ScopedOpenFile open_file(history_path); - if(!open_file.CanRead()) { + const base::FilePath history_path = safari_dir.Append("History.plist"); + if(!PathIsWritable(history_path)) { LOG(ERROR) << __func__ << " " << history_path << " is not accessible." << " Please check full disk access permission."; return false; diff --git a/chromium_src/chrome/common/importer/safari_importer_utils.mm b/chromium_src/chrome/common/importer/safari_importer_utils.mm deleted file mode 100644 index cb7abf98e67e..000000000000 --- a/chromium_src/chrome/common/importer/safari_importer_utils.mm +++ /dev/null @@ -1,20 +0,0 @@ -/* Copyright (c) 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#define SafariImporterCanImport SafariImporterCanImport_ChromiumImpl - -#include "../../../../../chrome/common/importer/safari_importer_utils.mm" - -#undef SafariImporterCanImport - -bool SafariImporterCanImport(const base::FilePath& library_dir, - uint16_t* services_supported) { - SafariImporterCanImport_ChromiumImpl(library_dir, services_supported); - - if (base::PathExists(library_dir.Append("Safari").Append("History.db"))) - *services_supported |= importer::HISTORY; - - return *services_supported != importer::NONE; -} diff --git a/chromium_src/chrome/utility/importer/importer_creator.cc b/chromium_src/chrome/utility/importer/importer_creator.cc deleted file mode 100644 index e75af9eefb45..000000000000 --- a/chromium_src/chrome/utility/importer/importer_creator.cc +++ /dev/null @@ -1,10 +0,0 @@ -/* Copyright 2019 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "brave/utility/importer/brave_safari_importer.h" - -#define SafariImporter BraveSafariImporter -#include "../../../../../chrome/utility/importer/importer_creator.cc" -#undef SafariImporter diff --git a/chromium_src/chrome/utility/importer/safari_importer.h b/chromium_src/chrome/utility/importer/safari_importer.h deleted file mode 100644 index b7f3d11d75ff..000000000000 --- a/chromium_src/chrome/utility/importer/safari_importer.h +++ /dev/null @@ -1,19 +0,0 @@ -/* Copyright 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_CHROMIUM_SRC_CHROME_UTILITY_IMPORTER_SAFARI_IMPORTER_H_ -#define BRAVE_CHROMIUM_SRC_CHROME_UTILITY_IMPORTER_SAFARI_IMPORTER_H_ - -#define BRAVE_SAFARI_IMPORTER_H \ - friend class BraveSafariImporter; - -#define ImportHistory virtual ImportHistory - -#include "../../../../../chrome/utility/importer/safari_importer.h" - -#undef BRAVE_SAFARI_IMPORTER_H -#undef ImportHistory - -#endif // BRAVE_CHROMIUM_SRC_CHROME_UTILITY_IMPORTER_SAFARI_IMPORTER_H_ diff --git a/patches/chrome-utility-importer-safari_importer.h.patch b/patches/chrome-utility-importer-safari_importer.h.patch deleted file mode 100644 index 0a1f82fd3010..000000000000 --- a/patches/chrome-utility-importer-safari_importer.h.patch +++ /dev/null @@ -1,12 +0,0 @@ -diff --git a/chrome/utility/importer/safari_importer.h b/chrome/utility/importer/safari_importer.h -index 1891fa3eff16f4e7b8f0821bc9f793c5e00514f7..89bce81bbc3d2f682adb3aacd888ac29f6d06e74 100644 ---- a/chrome/utility/importer/safari_importer.h -+++ b/chrome/utility/importer/safari_importer.h -@@ -46,6 +46,7 @@ class SafariImporter : public Importer { - uint16_t items, - ImporterBridge* bridge) override; - -+ BRAVE_SAFARI_IMPORTER_H - private: - FRIEND_TEST_ALL_PREFIXES(SafariImporterTest, BookmarkImport); - FRIEND_TEST_ALL_PREFIXES(SafariImporterTest, diff --git a/utility/BUILD.gn b/utility/BUILD.gn index c06a8c38cdac..c72c9ed75952 100644 --- a/utility/BUILD.gn +++ b/utility/BUILD.gn @@ -43,19 +43,6 @@ source_set("utility") { ] } - if (is_mac) { - sources += [ - "importer/brave_safari_importer.mm", - "importer/brave_safari_importer.h", - ] - - deps += [ - "//base", - "//sql", - "//url", - ] - } - if (enable_tor) { deps += [ "//brave/components/services/tor" ] } diff --git a/utility/importer/brave_safari_importer.h b/utility/importer/brave_safari_importer.h deleted file mode 100644 index 9b74c6bbbd55..000000000000 --- a/utility/importer/brave_safari_importer.h +++ /dev/null @@ -1,25 +0,0 @@ -/* Copyright 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_UTILITY_IMPORTER_BRAVE_SAFARI_IMPORTER_H_ -#define BRAVE_UTILITY_IMPORTER_BRAVE_SAFARI_IMPORTER_H_ - -#include "chrome/utility/importer/safari_importer.h" - -class BraveSafariImporter : public SafariImporter { - public: - using SafariImporter::SafariImporter; - - BraveSafariImporter(const BraveSafariImporter&) = delete; - BraveSafariImporter& operator=(const BraveSafariImporter&) = delete; - - private: - // SafariImporter overrides: - void ImportHistory() override; - - ~BraveSafariImporter() override; -}; - -#endif // BRAVE_UTILITY_IMPORTER_BRAVE_SAFARI_IMPORTER_H_ diff --git a/utility/importer/brave_safari_importer.mm b/utility/importer/brave_safari_importer.mm deleted file mode 100644 index a4ac51a6fe34..000000000000 --- a/utility/importer/brave_safari_importer.mm +++ /dev/null @@ -1,71 +0,0 @@ -/* Copyright 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include - -#include "brave/utility/importer/brave_safari_importer.h" - -#include -#include - -#include "base/files/file_path.h" -#include "base/files/file_util.h" -#include "base/strings/utf_string_conversions.h" -#include "base/time/time.h" -#include "chrome/common/importer/importer_bridge.h" -#include "chrome/common/importer/importer_url_row.h" -#include "sql/statement.h" -#include "url/gurl.h" - -BraveSafariImporter::~BraveSafariImporter() = default; - -void BraveSafariImporter::ImportHistory() { - // For importing history from History.plist. - SafariImporter::ImportHistory(); - - // From now, try to import history from History.db. - NSString* library_dir = [NSString - stringWithUTF8String:library_dir_.value().c_str()]; - NSString* safari_dir = [library_dir - stringByAppendingPathComponent:@"Safari"]; - NSString* history_db = [safari_dir - stringByAppendingPathComponent:@"History.db"]; - - // Import favicons. - sql::Database db; - const char* db_path = [history_db fileSystemRepresentation]; - if (!db.Open(base::FilePath(db_path))) - return; - - std::vector rows; - const char query[] = "SELECT hi.url, hi.visit_count, hv.visit_time, hv.title " - "FROM history_items as hi " - "JOIN history_visits as hv ON hi.id == hv.history_item"; - sql::Statement s(db.GetUniqueStatement(query)); - while (s.Step() && !cancelled()) { - const GURL url = GURL(s.ColumnString(0)); - if (!url.is_valid()) - continue; - - ImporterURLRow row(url); - row.visit_count = s.ColumnInt(1); - double visit_time = s.ColumnDouble(2); - if (!visit_time) - continue; - row.last_visit = - base::Time::FromDoubleT(visit_time + kCFAbsoluteTimeIntervalSince1970); - std::string title = s.ColumnString(3); - if (title.empty()) - title = url.spec(); - row.title = base::UTF8ToUTF16(title); - row.hidden = 0; - row.typed_count = 0; - rows.push_back(row); - } - - if (!rows.empty() && !cancelled()) { - bridge_->SetHistoryItems(rows, importer::VISIT_SOURCE_SAFARI_IMPORTED); - } -}