8000 refactor: have ShowSaveDialogSync() return a std::optional<base::File… · electron/electron@573c8aa · GitHub
[go: up one dir, main page]
Skip to content

Commit 573c8aa

Browse files
trop[bot]ckerr
trop[bot]
and
ckerr
authored
refactor: have ShowSaveDialogSync() return a std::optional<base::FilePath> (#47452)
* refactor: have ShowSaveDialogSync() return a std::optional<base::FilePath> Co-authored-by: Charles Kerr <charles@charleskerr.com> * fixup! refactor: have ShowSaveDialogSync() return a std::optional<base::FilePath> Co-authored-by: Charles Kerr <charles@charleskerr.com> --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Charles Kerr <charles@charleskerr.com>
1 parent ddee51e commit 573c8aa

File tree

6 files changed

+56
-45
lines changed

6 files changed

+56
-45
li 8000 nes changed

‎shell/browser/api/electron_api_dialog.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,8 @@ v8::Local<v8::Promise> ShowOpenDialog(
7171

7272
void ShowSaveDialogSync(const file_dialog::DialogSettings& settings,
7373
gin::Arguments* args) {
74-
base::FilePath path;
75-
if (file_dialog::ShowSaveDialogSync(settings, &path))
76-
args->Return(path);
74+
if (const auto path = file_dialog::ShowSaveDialogSync(settings))
75+
args->Return(*path);
7776
}
7877

7978
v8::Local<v8::Promise> ShowSaveDialog(

‎shell/browser/api/electron_api_web_contents.cc

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "base/containers/fixed_flat_map.h"
1919
#include "base/containers/flat_set.h"
2020
#include "base/containers/id_map.h"
21+
#include "base/containers/map_util.h"
2122
#include "base/files/file_util.h"
2223
#include "base/json/json_reader.h"
2324
#include "base/no_destructor.h"
@@ -4024,30 +4025,35 @@ void WebContents::DevToolsSaveToFile(const std::string& url,
40244025
const std::string& content,
40254026
bool save_as,
40264027
bool is_base64) {
4027-
base::FilePath path;
4028-
auto it = saved_files_.find(url);
4029-
if (it != saved_files_.end() && !save_as) {
4030-
path = it->second;
4031-
} else {
4028+
const base::FilePath* path = nullptr;
4029+
4030+
if (!save_as)
4031+
base::FindOrNull(saved_files_, url);
4032+
4033+
if (path == nullptr) {
40324034
file_dialog::DialogSettings settings;
40334035
settings.parent_window = owner_window();
40344036
settings.force_detached = offscreen_;
40354037
settings.title = url;
40364038
settings.default_path = base::FilePath::FromUTF8Unsafe(url);
4037-
if (!file_dialog::ShowSaveDialogSync(settings, &path)) {
4038-
inspectable_web_contents_->CallClientFunction(
4039-
"DevToolsAPI", "canceledSaveURL", base::Value(url));
4040-
return;
4039+
if (auto new_path = file_dialog::ShowSaveDialogSync(settings)) {
4040+
auto [iter, _] = saved_files_.try_emplace(url, std::move(*new_path));
4041+
path = &iter->second;
40414042
}
40424043
}
40434044

4044-
saved_files_[url] = path;
4045+
if (path == nullptr) {
4046+
inspectable_web_contents_->CallClientFunction(
4047+
"DevToolsAPI", "canceledSaveURL", base::Value{url});
4048+
return;
4049+
}
4050+
40454051
// Notify DevTools.
40464052
inspectable_web_contents_->CallClientFunction(
4047-
"DevToolsAPI", "savedURL", base::Value(url),
4048-
base::Value(path.AsUTF8Unsafe()));
4053+
"DevToolsAPI", "savedURL", base::Value{url},
4054+
base::Value{path->AsUTF8Unsafe()});
40494055
file_task_runner_->PostTask(
4050-
FROM_HERE, base::BindOnce(&WriteToFile, path, content, is_base64));
4056+
FROM_HERE, base::BindOnce(&WriteToFile, *path, content, is_base64));
40514057
}
40524058

40534059
void WebContents::DevToolsAppendToFile(const std::string& url,

‎shell/browser/ui/file_dialog.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef ELECTRON_SHELL_BROWSER_UI_FILE_DIALOG_H_
66
#define ELECTRON_SHELL_BROWSER_UI_FILE_DIALOG_H_
77

8+
#include <optional>
89
#include <string>
910
#include <utility>
1011
#include <vector>
@@ -72,7 +73,8 @@ bool ShowOpenDialogSync(const DialogSettings& settings,
7273
void ShowOpenDialog(const DialogSettings& settings,
7374
gin_helper::Promise<gin_helper::Dictionary> promise);
7475

75-
bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path);
76+
std::optional<base::FilePath> ShowSaveDialogSync(
77+
const DialogSettings& settings);
7678

7779
void ShowSaveDialog(const DialogSettings& settings,
7880
gin_helper::Promise<gin_helper::Dictionary> promise);

‎shell/browser/ui/file_dialog_linux.cc

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -233,20 +233,25 @@ void ShowOpenDialog(const DialogSettings& settings,
233233
dialog->RunOpenDialog(std::move(promise), settings);
234234
}
235235

236-
bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) {
237-
base::RunLoop run_loop(base::RunLoop::Type::kNestableTasksAllowed);
238-
auto cb = base::BindOnce(
239-
[](base::RepeatingClosure cb, base::FilePath* file_path,
240-
gin_helper::Dictionary result) {
241-
result.Get("filePath", file_path);
242-
std::move(cb).Run();
236+
std::optional<base::FilePath> ShowSaveDialogSync(
237+
const DialogSettings& settings) {
238+
std::optional<base::FilePath> path;
239+
240+
base::RunLoop run_loop{base::RunLoop::Type::kNestableTasksAllowed};
241+
auto on_chooser_dialog_done = base::BindOnce(
242+
[](base::RepeatingClosure run_loop_closure,
243+
std::optional<base::FilePath>* path, gin_helper::Dictionary result) {
244+
if (base::FilePath val; result.Get("filePath", &val))
245+
*path = std::move(val);
246+
std::move(run_loop_closure).Run();
243247
},
244-
run_loop.QuitClosure(), path);
248+
run_loop.QuitClosure(), &path);
245249

246-
FileChooserDialog* dialog = new FileChooserDialog();
247-
dialog->RunSaveDialog(std::move(cb), settings);
250+
auto* const dialog = new FileChooserDialog{};
251+
dialog->RunSaveDialog(std::move(on_chooser_dialog_done), settings);
248252
run_loop.Run();
249-
return !path->empty();
253+
254+
return path;
250255
}
251256

252257
void ShowSaveDialog(const DialogSettings& settings,

‎shell/browser/ui/file_dialog_mac.mm

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -436,19 +436,18 @@ void ShowOpenDialog(const DialogSettings& settings,
436436
}
437437
}
438438

439-
bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) {
440-
DCHECK(path);
439+
std::optional<base::FilePath> ShowSaveDialogSync(
440+
const DialogSettings& settings) {
441441
NSSavePanel* dialog = [NSSavePanel savePanel];
442442

443443
SetupDialog(dialog, settings);
444444
SetupSaveDialogForProperties(dialog, settings.properties);
445445

446-
int chosen = RunModalDialog(dialog, settings);
446+
const int chosen = RunModalDialog(dialog, settings);
447447
if (chosen == NSModalResponseCancel || ![[dialog URL] isFileURL])
448-
return false;
448+
return {};
449449

450-
*path = base::FilePath(base::SysNSStringToUTF8([[dialog URL] path]));
451-
return true;
450+
return base::FilePath{base::SysNSStringToUTF8([[dialog URL] path])};
452451
}
453452

454453
void SaveDialogCompletion(int chosen,

‎shell/browser/ui/file_dialog_win.cc

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -233,11 +233,12 @@ void ShowOpenDialog(const DialogSettings& settings,
233233
base::BindOnce(done, std::move(promise)));
234234
}
235235

236-
bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) {
236+
std::optional<base::FilePath> ShowSaveDialogSync(
237+
const DialogSettings& settings) {
237238
ATL::CComPtr<IFileSaveDialog> file_save_dialog;
238239
HRESULT hr = file_save_dialog.CoCreateInstance(CLSID_FileSaveDialog);
239240
if (FAILED(hr))
240-
return false;
241+
return {};
241242

242243
DWORD options = FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_OVERWRITEPROMPT;
243244
if (settings.properties & SAVE_DIALOG_SHOW_HIDDEN_FILES)
@@ -250,32 +251,31 @@ bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) {
250251
hr = ShowFileDialog(file_save_dialog, settings);
251252

252253
if (FAILED(hr))
253-
return false;
254+
return {};
254255

255256
CComPtr<IShellItem> pItem;
256257
hr = file_save_dialog->GetResult(&pItem);
257258
if (FAILED(hr))
258-
return false;
259+
return {};
259260

260261
PWSTR result_path = nullptr;
261262
hr = pItem->GetDisplayName(SIGDN_FILESYSPATH, &result_path);
262263
if (!SUCCEEDED(hr))
263-
return false;
264+
return {};
264265

265-
*path = base::FilePath(result_path);
266+
auto path = base::FilePath{result_path};
266267
CoTaskMemFree(result_path);
267-
268-
return true;
268+
return path;
269269
}
270270

271271
void ShowSaveDialog(const DialogSettings& settings,
272272
gin_helper::Promise<gin_helper::Dictionary> promise) {
273273
auto done = [](gin_helper::Promise<gin_helper::Dictionary> promise,
274-
bool success, base::FilePath result) {
274+
std::optional<base::FilePath> result) {
275275
v8::HandleScope handle_scope(promise.isolate());
276276
auto dict = gin::Dictionary::CreateEmpty(promise.isolate());
277-
dict.Set("canceled", !success);
278-
dict.Set("filePath", result);
277+
dict.Set("canceled", !result.has_value());
278+
dict.Set("filePath", result.value_or(base::FilePath{}));
279279
promise.Resolve(dict);
280280
};
281281
dialog_thread::Run(base::BindOnce(ShowSaveDialogSync, settings),

0 commit comments

Comments
 (0)
0