0

Tag files manually

The Mac file picker has an option to apply tags to newly-saved files.
There is a mechanism to automatically apply the tags to the files, but
that mechanism is not reliable. Therefore, plumb tag information
throughout Chromium and do it ourselves.

Fixed: 1510399
Change-Id: Ib3aae475f019937498d167519e10f5a522cf65b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5217354
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1250445}
This commit is contained in:
Avi Drissman
2024-01-22 21:37:25 +00:00
committed by Chromium LUCI CQ
parent 8e6e1c1c52
commit fc29fbdff5
20 changed files with 210 additions and 60 deletions

@ -11,6 +11,7 @@
#include <string>
#include <string_view>
#include <vector>
#include "base/base_export.h"
@ -54,6 +55,10 @@ BASE_EXPORT bool WasLaunchedAsHiddenLoginItem();
// an error, or true otherwise.
BASE_EXPORT bool RemoveQuarantineAttribute(const FilePath& file_path);
// Sets the tags on a given file or folder.
BASE_EXPORT void SetFileTags(const FilePath& file_path,
const std::vector<std::string>& file_tags);
// The following two functions return the version of the macOS currently
// running. MacOSVersion() returns the full trio of version numbers, packed into
// one int (e.g. macOS 12.6.5 returns 12'06'05), and MacOSMajorVersion() returns

@ -273,6 +273,21 @@ bool RemoveQuarantineAttribute(const FilePath& file_path) {
return status == 0 || errno == ENOATTR;
}
void SetFileTags(const FilePath& file_path,
const std::vector<std::string>& file_tags) {
if (file_tags.empty()) {
return;
}
NSMutableArray* tag_array = [NSMutableArray array];
for (const auto& tag : file_tags) {
[tag_array addObject:SysUTF8ToNSString(tag)];
}
NSURL* file_url = apple::FilePathToNSURL(file_path);
[file_url setResourceValue:tag_array forKey:NSURLTagNamesKey error:nil];
}
namespace {
int ParseOSProductVersion(const std::string_view& version) {

@ -644,6 +644,9 @@ void DownloadTargetDeterminer::RequestConfirmationDone(
confirmation_reason_ = DownloadConfirmationReason::NONE;
virtual_path_ = virtual_path;
#if BUILDFLAG(IS_MAC)
file_tags_ = selected_file_info.file_tags;
#endif
#if BUILDFLAG(IS_ANDROID)
if (result == DownloadConfirmationResult::CONFIRMED_WITH_DIALOG) {
@ -1103,7 +1106,7 @@ DownloadTargetDeterminer::Result
}
void DownloadTargetDeterminer::ScheduleCallbackAndDeleteSelf(
download::DownloadInterruptReason result) {
download::DownloadInterruptReason interrupt_reason) {
DCHECK(download_);
DVLOG(20) << "Scheduling callback. Virtual:" << virtual_path_.AsUTF8Unsafe()
<< " Local:" << local_path_.AsUTF8Unsafe()
@ -1111,12 +1114,21 @@ void DownloadTargetDeterminer::ScheduleCallbackAndDeleteSelf(
<< " Confirmation reason:" << static_cast<int>(confirmation_reason_)
<< " Danger type:" << danger_type_
<< " Danger level:" << danger_level_
<< " Result:" << static_cast<int>(result);
<< " Interrupt reason:" << static_cast<int>(interrupt_reason);
download::DownloadTargetInfo target_info;
target_info.target_path = local_path_;
target_info.intermediate_path = intermediate_path_;
#if BUILDFLAG(IS_ANDROID)
// If |virtual_path_| is content URI, there is no need to prompt the user.
if (local_path_.IsContentUri() && !virtual_path_.IsContentUri()) {
target_info.display_name = virtual_path_.BaseName();
}
#endif
target_info.mime_type = mime_type_;
#if BUILDFLAG(IS_MAC)
target_info.file_tags = file_tags_;
#endif
target_info.is_filetype_handled_safely = is_filetype_handled_safely_;
target_info.target_disposition =
(HasPromptedForPath() ||
@ -1124,14 +1136,8 @@ void DownloadTargetDeterminer::ScheduleCallbackAndDeleteSelf(
? DownloadItem::TARGET_DISPOSITION_PROMPT
: DownloadItem::TARGET_DISPOSITION_OVERWRITE);
target_info.danger_type = danger_type_;
target_info.interrupt_reason = result;
target_info.interrupt_reason = interrupt_reason;
target_info.insecure_download_status = insecure_download_status_;
#if BUILDFLAG(IS_ANDROID)
// If |virtual_path_| is content URI, there is no need to prompt the user.
if (local_path_.IsContentUri() && !virtual_path_.IsContentUri()) {
target_info.display_name = virtual_path_.BaseName();
}
#endif
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(std::move(completion_callback_),

@ -349,7 +349,8 @@ class DownloadTargetDeterminer : public download::DownloadItem::Observer {
// this object. The determined target info will be passed into the callback
// if |interrupt_reason| is NONE. Otherwise, only the interrupt reason will be
// passed on.
void ScheduleCallbackAndDeleteSelf(download::DownloadInterruptReason result);
void ScheduleCallbackAndDeleteSelf(
download::DownloadInterruptReason interrupt_reason);
Profile* GetProfile() const;
@ -413,6 +414,11 @@ class DownloadTargetDeterminer : public download::DownloadItem::Observer {
#if BUILDFLAG(IS_ANDROID)
bool is_checking_dialog_confirmed_path_;
#endif
#if BUILDFLAG(IS_MAC)
// A list of tags specified by the user to be set on the file upon the
// completion of it being written to disk.
std::vector<std::string> file_tags_;
#endif
raw_ptr<download::DownloadItem> download_;
const bool is_resumption_;

@ -274,7 +274,13 @@ void SavePackageFilePicker::FileSelected(const ui::SelectedFileInfo& file,
download_prefs_->SetSaveFilePath(path.DirName());
std::move(callback_).Run(path, save_type,
content::SavePackagePathPickedParams params;
params.file_path = path;
params.save_type = save_type;
#if BUILDFLAG(IS_MAC)
params.file_tags = file.file_tags;
#endif
std::move(callback_).Run(std::move(params),
base::BindOnce(&OnSavePackageDownloadCreated));
}

@ -852,28 +852,27 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest,
base::FilePath full_file_name = download_dir.AppendASCII("test_page");
download_prefs->SetSaveFileType(content::SAVE_PAGE_TYPE_AS_MHTML);
base::FilePath received_path;
content::SavePageType received_type;
content::SavePackagePathPickedParams received_params;
content::SavePackagePathPickedCallback callback = base::BindOnce(
[](base::FilePath* received_path, content::SavePageType* received_type,
const base::FilePath& path, content::SavePageType type,
[](content::SavePackagePathPickedParams* received_params,
content::SavePackagePathPickedParams params,
content::SavePackageDownloadCreatedCallback cb) {
*received_path = path;
*received_type = type;
*received_params = params;
},
&received_path, &received_type);
&received_params);
// Deletes itself.
new SavePackageFilePicker(
/* web_contents */ GetCurrentTab(browser()),
/* suggested_path */ full_file_name,
/* default_extension */ FILE_PATH_LITERAL(".html"),
/* can_save_as_complete */ true,
/* download_prefs */ download_prefs,
/* callback */ std::move(callback));
/*web_contents=*/GetCurrentTab(browser()),
/*suggested_path=*/full_file_name,
/*default_extension=*/FILE_PATH_LITERAL(".html"),
/*can_save_as_complete=*/true,
/*download_prefs=*/download_prefs,
/*callback=*/std::move(callback));
EXPECT_TRUE(received_path.MatchesExtension(FILE_PATH_LITERAL(".mhtml")));
EXPECT_EQ(received_type, content::SAVE_PAGE_TYPE_AS_MHTML);
EXPECT_TRUE(
received_params.file_path.MatchesExtension(FILE_PATH_LITERAL(".mhtml")));
EXPECT_EQ(received_params.save_type, content::SAVE_PAGE_TYPE_AS_MHTML);
}
// Flaky on Windows: https://crbug.com/1247404.

@ -69,6 +69,10 @@
#include "components/download/internal/common/android/download_collection_bridge.h"
#endif // BUILDFLAG(IS_ANDROID)
#if BUILDFLAG(IS_MAC)
#include "base/mac/mac_util.h"
#endif // BUILDFLAG(IS_MAC)
namespace download {
namespace {
@ -1759,6 +1763,9 @@ void DownloadItemImpl::OnDownloadTargetDetermined(
if (!target_info.mime_type.empty()) {
mime_type_ = target_info.mime_type;
}
#if BUILDFLAG(IS_MAC)
file_tags_ = target_info.file_tags;
#endif
// This was an interrupted download that was looking for a filename. Resolve
// early without performing the intermediate rename. If there is a
@ -1969,6 +1976,10 @@ void DownloadItemImpl::OnDownloadRenamedToFinalName(
SetFullPath(full_path);
}
#if BUILDFLAG(IS_MAC)
base::mac::SetFileTags(full_path, file_tags_);
#endif
// Complete the download and release the DownloadFile.
DCHECK(download_file_);
ReleaseDownloadFile(false);

@ -743,6 +743,12 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadItemImpl
// which may look at the file extension and first few bytes of the file.
std::string original_mime_type_;
#if BUILDFLAG(IS_MAC)
// A list of tags specified by the user to be set on the file upon the
// completion of it being written to disk.
std::vector<std::string> file_tags_;
#endif
// Total bytes expected.
int64_t total_bytes_ = 0;

@ -7,6 +7,7 @@
#include <optional>
#include <string>
#include <vector>
#include "base/files/file_path.h"
#include "components/download/public/common/download_danger_type.h"
@ -46,6 +47,12 @@ struct COMPONENTS_DOWNLOAD_EXPORT DownloadTargetInfo {
// MIME type will be kept.
std::string mime_type;
#if BUILDFLAG(IS_MAC)
// A list of tags specified by the user to be set on the file upon the
// completion of it being written to disk.
std::vector<std::string> file_tags;
#endif
// Whether the |target_path| would be handled safely by the browser if it were
// to be opened with a file:// URL. This can be used later to decide how file
// opens should be handled. The file is considered to be handled safely if the

@ -24,12 +24,6 @@ namespace remote_cocoa {
class REMOTE_COCOA_APP_SHIM_EXPORT SelectFileDialogBridge
: public mojom::SelectFileDialog {
public:
// Callback made from the NSSavePanel's completion block.
using PanelEndedCallback =
base::OnceCallback<void(bool was_cancelled,
const std::vector<base::FilePath>& files,
int index)>;
explicit SelectFileDialogBridge(NSWindow* owning_window);
SelectFileDialogBridge(const SelectFileDialogBridge&) = delete;

@ -294,7 +294,7 @@ void SelectFileDialogBridge::Show(
SelectFileTypeInfoPtr file_types,
int file_type_index,
const base::FilePath::StringType& default_extension,
PanelEndedCallback initialize_callback) {
ShowCallback callback) {
// Never consider the current WatchHangsInScope as hung. There was most likely
// one created in ThreadControllerWithMessagePumpImpl::DoWork(). The current
// hang watching deadline is not valid since the user can take unbounded time
@ -303,7 +303,7 @@ void SelectFileDialogBridge::Show(
// reactivate it. You can see the function comments for more details.
base::HangWatcher::InvalidateActiveExpectations();
show_callback_ = std::move(initialize_callback);
show_callback_ = std::move(callback);
type_ = type;
// Note: we need to retain the dialog as |owning_window_| can be null.
// (See https://crbug.com/29213 .)
@ -361,6 +361,13 @@ void SelectFileDialogBridge::Show(
panel_.extensionHidden = YES;
panel_.canSelectHiddenExtension = YES;
}
// The tag autosetter in macOS is not reliable (see
// https://crbug.com/1510399). Explicitly set the `showsTagField` property
// as a signal to macOS that we will handle all the file tagging; a
// side-effect of setting the property to any value is that it turns off
// the tag autosetter.
panel_.showsTagField = YES;
} else {
// This does not use ObjCCast because the underlying object could be a
// non-exported AppKit type (https://crbug.com/995476).
@ -401,11 +408,11 @@ void SelectFileDialogBridge::Show(
panel_.nameFieldStringValue = default_filename;
// Ensure that |callback| (rather than |this|) be retained by the block.
auto callback = base::BindRepeating(&SelectFileDialogBridge::OnPanelEnded,
weak_factory_.GetWeakPtr());
auto ended_callback = base::BindRepeating(
&SelectFileDialogBridge::OnPanelEnded, weak_factory_.GetWeakPtr());
[panel_ beginSheetModalForWindow:owning_window_
completionHandler:^(NSInteger result) {
callback.Run(result != NSModalResponseOK);
ended_callback.Run(result != NSModalResponseOK);
}];
}
@ -556,23 +563,30 @@ void SelectFileDialogBridge::OnPanelEnded(bool did_cancel) {
int index = 0;
std::vector<base::FilePath> paths;
std::vector<std::string> file_tags;
if (!did_cancel) {
if (type_ == SelectFileDialogType::kSaveAsFile) {
NSURL* url = [panel_ URL];
if ([url isFileURL]) {
paths.push_back(base::apple::NSStringToFilePath([url path]));
NSURL* url = panel_.URL;
if (url.isFileURL) {
paths.push_back(base::apple::NSURLToFilePath(url));
}
NSView* accessoryView = [panel_ accessoryView];
NSView* accessoryView = panel_.accessoryView;
if (accessoryView) {
NSPopUpButton* popup = [accessoryView viewWithTag:kFileTypePopupTag];
if (popup) {
// File type indexes are 1-based.
index = [popup indexOfSelectedItem] + 1;
index = popup.indexOfSelectedItem + 1;
}
} else {
index = 1;
}
// The tag autosetter was turned off when `showsTagField` was set above.
// Retrieve the tags for assignment later.
for (NSString* tag in panel_.tagNames) {
file_tags.push_back(base::SysNSStringToUTF8(tag));
}
} else {
// This does not use ObjCCast because the underlying object could be a
// non-exported AppKit type (https://crbug.com/995476).
@ -603,7 +617,7 @@ void SelectFileDialogBridge::OnPanelEnded(bool did_cancel) {
}
}
std::move(show_callback_).Run(did_cancel, paths, index);
std::move(show_callback_).Run(did_cancel, paths, index, file_tags);
}
// static

@ -36,6 +36,24 @@ struct SelectFileTypeInfo {
// The interface to a file selection (Save As, Upload, etc) dialog.
interface SelectFileDialog {
// Show the dialog, and issue a callback when it has completed.
//
// Sent parameters:
// `type`: The type of dialog to use.
// `title`: The title presented at the top of the file selection dialog.
// `file_path`: The default path for use when opening the dialog.
// `file_types`: The file types allowed for saving a file.
// `file_type_index`: The index within the `file_types` array to use for the
// initial selection. NOTE: This is 1-based.
// `default_extension`: The default extension to use.
//
// Returned parameters:
// `was_cancelled`: Whether or not the user canceled the dialog.
// `files`: The files that the user selected. If this was not a
// multiple-selection dialog, this will have at most one item.
// `index`: The index within the `file_types` array that the user selected.
// NOTE: This is 1-based.
// `file_tags`: The file tags that the user selected to apply to the file
// once the file is saved.
Show(SelectFileDialogType type,
mojo_base.mojom.String16 title,
mojo_base.mojom.FilePath file_path,
@ -43,5 +61,6 @@ interface SelectFileDialog {
int32 file_type_index,
string default_extension) => (bool was_cancelled,
array<mojo_base.mojom.FilePath> files,
int32 index);
int32 index,
array<string> file_tags);
};

@ -68,6 +68,10 @@
#include "services/network/public/cpp/request_mode.h"
#include "url/url_constants.h"
#if BUILDFLAG(IS_MAC)
#include "base/mac/mac_util.h"
#endif
namespace content {
namespace {
@ -785,6 +789,15 @@ void SavePackage::Finish() {
wait_state_ = SUCCESSFUL;
finished_ = true;
#if BUILDFLAG(IS_MAC)
// Always set tags on the main HTML file, and if there is an associated
// "_files" directory, set the tags on it, too.
base::mac::SetFileTags(saved_main_file_path_, file_tags_);
if (save_type_ == SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
base::mac::SetFileTags(saved_main_directory_path_, file_tags_);
}
#endif // BUILDFLAG(IS_MAC)
if (download_) {
std::vector<download::DownloadSaveItemData::ItemInfo> files;
for (auto& item : saved_success_items_) {
@ -1477,18 +1490,17 @@ void SavePackage::ContinueGetSaveInfo(bool can_save_as_complete,
}
void SavePackage::OnPathPicked(
const base::FilePath& final_name,
SavePageType type,
SavePackagePathPickedParams params,
SavePackageDownloadCreatedCallback download_created_callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK((type == SAVE_PAGE_TYPE_AS_ONLY_HTML) ||
(type == SAVE_PAGE_TYPE_AS_MHTML) ||
(type == SAVE_PAGE_TYPE_AS_COMPLETE_HTML))
<< type;
DCHECK((params.save_type == SAVE_PAGE_TYPE_AS_ONLY_HTML) ||
(params.save_type == SAVE_PAGE_TYPE_AS_MHTML) ||
(params.save_type == SAVE_PAGE_TYPE_AS_COMPLETE_HTML))
<< params.save_type;
if (!page_)
return;
// Ensure the filename is safe.
saved_main_file_path_ = final_name;
saved_main_file_path_ = params.file_path;
// TODO(asanka): This call may block on IO and shouldn't be made
// from the UI thread. See http://crbug.com/61827.
std::string mime_type =
@ -1496,7 +1508,7 @@ void SavePackage::OnPathPicked(
net::GenerateSafeFileName(mime_type, false, &saved_main_file_path_);
saved_main_directory_path_ = saved_main_file_path_.DirName();
save_type_ = type;
save_type_ = params.save_type;
if (save_type_ == SAVE_PAGE_TYPE_AS_COMPLETE_HTML) {
// Make new directory for saving complete file.
saved_main_directory_path_ = saved_main_directory_path_.Append(
@ -1504,6 +1516,10 @@ void SavePackage::OnPathPicked(
FILE_PATH_LITERAL("_files"));
}
#if BUILDFLAG(IS_MAC)
file_tags_ = params.file_tags;
#endif
Init(std::move(download_created_callback));
}

@ -333,8 +333,7 @@ class CONTENT_EXPORT SavePackage
const base::FilePath& download_save_dir);
void ContinueGetSaveInfo(bool can_save_as_complete,
const base::FilePath& suggested_path);
void OnPathPicked(const base::FilePath& final_name,
SavePageType type,
void OnPathPicked(SavePackagePathPickedParams params,
SavePackageDownloadCreatedCallback cb);
// The number of in process SaveItems.
@ -442,6 +441,12 @@ class CONTENT_EXPORT SavePackage
// Type about saving page as only-html or complete-html.
SavePageType save_type_ = SAVE_PAGE_TYPE_UNKNOWN;
#if BUILDFLAG(IS_MAC)
// A list of tags specified by the user to be set on the file upon the
// completion of it being written to disk.
std::vector<std::string> file_tags_;
#endif
// Number of all need to be saved resources.
size_t all_save_items_count_ = 0;

@ -40,7 +40,10 @@ class TestShellDownloadManagerDelegate : public ShellDownloadManagerDelegate {
const base::FilePath::StringType& default_extension,
bool can_save_as_complete,
SavePackagePathPickedCallback callback) override {
std::move(callback).Run(suggested_path, save_page_type_,
content::SavePackagePathPickedParams params;
params.file_path = suggested_path;
params.save_type = save_page_type_;
std::move(callback).Run(std::move(params),
SavePackageDownloadCreatedCallback());
}

@ -12,6 +12,18 @@
namespace content {
SavePackagePathPickedParams::SavePackagePathPickedParams() = default;
SavePackagePathPickedParams::~SavePackagePathPickedParams() = default;
SavePackagePathPickedParams::SavePackagePathPickedParams(
const SavePackagePathPickedParams& other) = default;
SavePackagePathPickedParams& SavePackagePathPickedParams::operator=(
const SavePackagePathPickedParams& other) = default;
SavePackagePathPickedParams::SavePackagePathPickedParams(
SavePackagePathPickedParams&& other) = default;
SavePackagePathPickedParams& SavePackagePathPickedParams::operator=(
SavePackagePathPickedParams&& other) = default;
void DownloadManagerDelegate::GetNextId(DownloadIdCallback callback) {
std::move(callback).Run(download::DownloadItem::kInvalidId);
}

@ -37,9 +37,24 @@ using SavePackageDownloadCreatedCallback =
// operation. If the delegate wants notification of the download item created
// in response to this operation, the SavePackageDownloadCreatedCallback will be
// non-null.
struct CONTENT_EXPORT SavePackagePathPickedParams {
SavePackagePathPickedParams();
~SavePackagePathPickedParams();
SavePackagePathPickedParams(const SavePackagePathPickedParams& other);
SavePackagePathPickedParams& operator=(
const SavePackagePathPickedParams& other);
SavePackagePathPickedParams(SavePackagePathPickedParams&& other);
SavePackagePathPickedParams& operator=(SavePackagePathPickedParams&& other);
base::FilePath file_path;
SavePageType save_type;
#if BUILDFLAG(IS_MAC)
std::vector<std::string> file_tags;
#endif
};
using SavePackagePathPickedCallback =
base::OnceCallback<void(const base::FilePath&,
SavePageType,
base::OnceCallback<void(SavePackagePathPickedParams,
SavePackageDownloadCreatedCallback)>;
// Called when a download delayed by the delegate has completed.

@ -81,7 +81,8 @@ class SHELL_DIALOGS_EXPORT SelectFileDialogImpl : public ui::SelectFileDialog {
bool is_multi,
bool was_cancelled,
const std::vector<base::FilePath>& files,
int index);
int index,
const std::vector<std::string>& file_tags);
bool HasMultipleFileTypeChoicesImpl() override;

@ -48,7 +48,8 @@ void SelectFileDialogImpl::FileWasSelected(
bool is_multi,
bool was_cancelled,
const std::vector<base::FilePath>& files,
int index) {
int index,
const std::vector<std::string>& file_tags) {
auto it = base::ranges::find(dialog_data_list_, dialog_data,
[](const DialogData& d) { return &d; });
DCHECK(it != dialog_data_list_.end());
@ -68,7 +69,9 @@ void SelectFileDialogImpl::FileWasSelected(
listener_->MultiFilesSelected(FilePathListToSelectedFileInfoList(files),
params);
} else {
listener_->FileSelected(SelectedFileInfo(files[0]), index, params);
SelectedFileInfo file(files[0]);
file.file_tags = file_tags;
listener_->FileSelected(file, index, params);
}
}
}

@ -8,6 +8,7 @@
#include <vector>
#include "base/files/file_path.h"
#include "build/build_config.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/shell_dialogs/shell_dialogs_export.h"
#include "url/gurl.h"
@ -50,6 +51,12 @@ struct SHELL_DIALOGS_EXPORT SelectedFileInfo {
// preference over `local_path` and `url`.
absl::optional<base::FilePath> virtual_path;
#if BUILDFLAG(IS_MAC)
// A list of tags specified by the user to be set on the file upon the
// completion of it being written to disk.
std::vector<std::string> file_tags;
#endif
// Constructs an empty object.
SelectedFileInfo();