diff --git a/filenames.gni b/filenames.gni index 961c82fef563b..74513f4366ce0 100644 --- a/filenames.gni +++ b/filenames.gni @@ -389,6 +389,8 @@ filenames = { "shell/browser/ui/views/submenu_button.h", "shell/browser/ui/views/win_frame_view.cc", "shell/browser/ui/views/win_frame_view.h", + "shell/browser/ui/win/dialog_thread.cc", + "shell/browser/ui/win/dialog_thread.h", "shell/browser/ui/win/electron_desktop_native_widget_aura.cc", "shell/browser/ui/win/electron_desktop_native_widget_aura.h", "shell/browser/ui/win/electron_desktop_window_tree_host_win.cc", diff --git a/shell/browser/ui/file_dialog_win.cc b/shell/browser/ui/file_dialog_win.cc index 36428c97ba411..c2aec1c8a65a2 100644 --- a/shell/browser/ui/file_dialog_win.cc +++ b/shell/browser/ui/file_dialog_win.cc @@ -16,10 +16,9 @@ #include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" -#include "base/threading/thread.h" -#include "base/threading/thread_task_runner_handle.h" #include "base/win/registry.h" #include "shell/browser/native_window_views.h" +#include "shell/browser/ui/win/dialog_thread.h" #include "shell/browser/unresponsive_suppressor.h" #include "shell/common/gin_converters/file_path_converter.h" @@ -63,67 +62,6 @@ void ConvertFilters(const Filters& filters, } } -struct RunState { - base::Thread* dialog_thread; - scoped_refptr ui_task_runner; -}; - -bool CreateDialogThread(RunState* run_state) { - auto thread = - std::make_unique(ELECTRON_PRODUCT_NAME "FileDialogThread"); - thread->init_com_with_mta(false); - if (!thread->Start()) - return false; - - run_state->dialog_thread = thread.release(); - run_state->ui_task_runner = base::ThreadTaskRunnerHandle::Get(); - return true; -} - -void OnDialogOpened(electron::util::Promise promise, - bool canceled, - std::vector paths) { - gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(promise.isolate()); - dict.Set("canceled", canceled); - dict.Set("filePaths", paths); - promise.ResolveWithGin(dict); -} - -void RunOpenDialogInNewThread( - const RunState& run_state, - const DialogSettings& settings, - electron::util::Promise promise) { - std::vector paths; - bool result = ShowOpenDialogSync(settings, &paths); - run_state.ui_task_runner->PostTask( - FROM_HERE, - base::BindOnce(&OnDialogOpened, std::move(promise), !result, paths)); - run_state.ui_task_runner->DeleteSoon(FROM_HERE, run_state.dialog_thread); -} - -void OnSaveDialogDone(electron::util::Promise promise, - bool canceled, - const base::FilePath path) { - gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(promise.isolate()); - dict.Set("canceled", canceled); - dict.Set("filePath", path); - promise.ResolveWithGin(dict); -} - -void RunSaveDialogInNewThread( - const RunState& run_state, - const DialogSettings& settings, - electron::util::Promise promise) { - base::FilePath path; - bool result = ShowSaveDialogSync(settings, &path); - run_state.ui_task_runner->PostTask( - FROM_HERE, - base::BindOnce(&OnSaveDialogDone, std::move(promise), !result, path)); - run_state.ui_task_runner->DeleteSoon(FROM_HERE, run_state.dialog_thread); -} - -} // namespace - static HRESULT GetFileNameFromShellItem(IShellItem* pShellItem, SIGDN type, LPWSTR lpstr, @@ -218,6 +156,8 @@ static void ApplySettings(IFileDialog* dialog, const DialogSettings& settings) { } } +} // namespace + bool ShowOpenDialogSync(const DialogSettings& settings, std::vector* paths) { ATL::CComPtr file_open_dialog; @@ -276,17 +216,17 @@ bool ShowOpenDialogSync(const DialogSettings& settings, void ShowOpenDialog(const DialogSettings& settings, electron::util::Promise promise) { - gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(promise.isolate()); - RunState run_state; - if (!CreateDialogThread(&run_state)) { - dict.Set("canceled", true); - dict.Set("filePaths", std::vector()); + auto done = [](electron::util::Promise promise, + bool success, std::vector result) { + v8::Locker locker(promise.isolate()); + v8::HandleScope handle_scope(promise.isolate()); + gin::Dictionary dict = gin::Dictionary::CreateEmpty(promise.isolate()); + dict.Set("canceled", !success); + dict.Set("filePaths", result); promise.ResolveWithGin(dict); - } else { - run_state.dialog_thread->task_runner()->PostTask( - FROM_HERE, base::BindOnce(&RunOpenDialogInNewThread, run_state, - settings, std::move(promise))); - } + }; + dialog_thread::Run(base::BindOnce(ShowOpenDialogSync, settings), + base::BindOnce(done, std::move(promise))); } bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) { @@ -326,18 +266,17 @@ bool ShowSaveDialogSync(const DialogSettings& settings, base::FilePath* path) { void ShowSaveDialog(const DialogSettings& settings, electron::util::Promise promise) { - RunState run_state; - if (!CreateDialogThread(&run_state)) { - gin_helper::Dictionary dict = - gin::Dictionary::CreateEmpty(promise.isolate()); - dict.Set("canceled", true); - dict.Set("filePath", base::FilePath()); + auto done = [](electron::util::Promise promise, + bool success, base::FilePath result) { + v8::Locker locker(promise.isolate()); + v8::HandleScope handle_scope(promise.isolate()); + gin::Dictionary dict = gin::Dictionary::CreateEmpty(promise.isolate()); + dict.Set("canceled", !success); + dict.Set("filePath", result); promise.ResolveWithGin(dict); - } else { - run_state.dialog_thread->task_runner()->PostTask( - FROM_HERE, base::BindOnce(&RunSaveDialogInNewThread, run_state, - settings, std::move(promise))); - } + }; + dialog_thread::Run(base::BindOnce(ShowSaveDialogSync, settings), + base::BindOnce(done, std::move(promise))); } } // namespace file_dialog diff --git a/shell/browser/ui/message_box_win.cc b/shell/browser/ui/message_box_win.cc index 1e4dcd7653683..975f092381a41 100644 --- a/shell/browser/ui/message_box_win.cc +++ b/shell/browser/ui/message_box_win.cc @@ -14,12 +14,10 @@ #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/task/post_task.h" -#include "base/threading/thread.h" #include "base/win/scoped_gdi_object.h" -#include "content/public/browser/browser_task_traits.h" -#include "content/public/browser/browser_thread.h" #include "shell/browser/browser.h" #include "shell/browser/native_window_views.h" +#include "shell/browser/ui/win/dialog_thread.h" #include "shell/browser/unresponsive_suppressor.h" #include "ui/gfx/icon_util.h" #include "ui/gfx/image/image_skia.h" @@ -203,17 +201,6 @@ DialogResult ShowTaskDialogUTF8(const MessageBoxSettings& settings) { checkbox_label_16, settings.checkbox_checked, settings.icon); } -void RunMessageBoxInNewThread(base::Thread* thread, - const MessageBoxSettings& settings, - MessageBoxCallback callback) { - DialogResult result = ShowTaskDialogUTF8(settings); - base::PostTask( - FROM_HERE, {content::BrowserThread::UI}, - base::BindOnce(std::move(callback), result.first, result.second)); - content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE, - thread); -} - } // namespace int ShowMessageBoxSync(const MessageBoxSettings& settings) { @@ -224,19 +211,12 @@ int ShowMessageBoxSync(const MessageBoxSettings& settings) { void ShowMessageBox(const MessageBoxSettings& settings, MessageBoxCallback callback) { - auto thread = - std::make_unique(ELECTRON_PRODUCT_NAME "MessageBoxThread"); - thread->init_com_with_mta(false); - if (!thread->Start()) { - std::move(callback).Run(settings.cancel_id, settings.checkbox_checked); - return; - } - - base::Thread* unretained = thread.release(); - unretained->task_runner()->PostTask( - FROM_HERE, - base::BindOnce(&RunMessageBoxInNewThread, base::Unretained(unretained), - settings, std::move(callback))); + dialog_thread::Run(base::BindOnce(&ShowTaskDialogUTF8, settings), + base::BindOnce( + [](MessageBoxCallback callback, DialogResult result) { + std::move(callback).Run(result.first, result.second); + }, + std::move(callback))); } void ShowErrorBox(const base::string16& title, const base::string16& content) { diff --git a/shell/browser/ui/win/dialog_thread.cc b/shell/browser/ui/win/dialog_thread.cc new file mode 100644 index 0000000000000..00ff9ae321c6f --- /dev/null +++ b/shell/browser/ui/win/dialog_thread.cc @@ -0,0 +1,23 @@ +// Copyright (c) 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "shell/browser/ui/win/dialog_thread.h" + +#include "base/win/scoped_com_initializer.h" + +namespace dialog_thread { + +// Creates a SingleThreadTaskRunner to run a shell dialog on. Each dialog +// requires its own dedicated single-threaded sequence otherwise in some +// situations where a singleton owns a single instance of this object we can +// have a situation where a modal dialog in one window blocks the appearance +// of a modal dialog in another. +TaskRunner CreateDialogTaskRunner() { + return CreateCOMSTATaskRunner( + {base::ThreadPool(), base::TaskPriority::USER_BLOCKING, + base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN, base::MayBlock()}, + base::SingleThreadTaskRunnerThreadMode::DEDICATED); +} + +} // namespace dialog_thread diff --git a/shell/browser/ui/win/dialog_thread.h b/shell/browser/ui/win/dialog_thread.h new file mode 100644 index 0000000000000..accb8d91bf3b7 --- /dev/null +++ b/shell/browser/ui/win/dialog_thread.h @@ -0,0 +1,81 @@ +// Copyright (c) 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SHELL_BROWSER_UI_WIN_DIALOG_THREAD_H_ +#define SHELL_BROWSER_UI_WIN_DIALOG_THREAD_H_ + +#include +#include + +#include "base/memory/scoped_refptr.h" +#include "base/task/post_task.h" +#include "content/public/browser/browser_thread.h" + +namespace dialog_thread { + +// Returns the dedicated single-threaded sequence that the dialog will be on. +using TaskRunner = scoped_refptr; +TaskRunner CreateDialogTaskRunner(); + +// Runs the |execute| in dialog thread and pass result to |done| in UI thread. +template +void Run(base::OnceCallback execute, base::OnceCallback done) { + // dialogThread.postTask(() => { + // r = execute() + // uiThread.postTask(() => { + // done(r) + // } + // }) + TaskRunner task_runner = CreateDialogTaskRunner(); + task_runner->PostTask( + FROM_HERE, + base::BindOnce( + [](TaskRunner task_runner, base::OnceCallback execute, + base::OnceCallback done) { + R r = std::move(execute).Run(); + base::PostTask( + FROM_HERE, {content::BrowserThread::UI}, + base::BindOnce( + [](TaskRunner task_runner, base::OnceCallback done, + R r) { + std::move(done).Run(std::move(r)); + // Task runner will destroyed automatically after the + // scope ends. + }, + std::move(task_runner), std::move(done), std::move(r))); + }, + std::move(task_runner), std::move(execute), std::move(done))); +} + +// Adaptor to handle the |execute| that returns bool. +template +void Run(base::OnceCallback execute, + base::OnceCallback done) { + // run(() => { + // result = execute(&value) + // return {result, value} + // }, ({result, value}) => { + // done(result, value) + // }) + struct Result { + bool result; + R value; + }; + Run(base::BindOnce( + [](base::OnceCallback execute) { + Result r; + r.result = std::move(execute).Run(&r.value); + return r; + }, + std::move(execute)), + base::BindOnce( + [](base::OnceCallback done, Result r) { + std::move(done).Run(r.result, std::move(r.value)); + }, + std::move(done))); +} + +} // namespace dialog_thread + +#endif // SHELL_BROWSER_UI_WIN_DIALOG_THREAD_H_