From a33ffd621f184916e76fce43100701264321e2c2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 22 Feb 2017 15:58:46 -0800 Subject: [PATCH 1/5] Use callback dialog methods in RunFileChooser --- atom/browser/web_dialog_helper.cc | 116 ++++++++++++++++++++++++------ 1 file changed, 94 insertions(+), 22 deletions(-) diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index af7cc27674..94ff44d551 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -16,7 +16,9 @@ #include "base/strings/utf_string_conversions.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" +#include "content/public/browser/browser_thread.h" #include "content/public/browser/render_frame_host.h" +#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "content/public/common/file_chooser_file_info.h" @@ -26,6 +28,94 @@ namespace { +class FileSelectHelper : public base::RefCountedThreadSafe< + FileSelectHelper, + content::BrowserThread::DeleteOnUIThread>, + public content::WebContentsObserver { + public: + FileSelectHelper(content::RenderFrameHost* render_frame_host, + const content::FileChooserParams::Mode& mode) + : render_frame_host_(render_frame_host), mode_(mode) { + auto web_contents = content::WebContents::FromRenderFrameHost( + render_frame_host); + content::WebContentsObserver::Observe(web_contents); + // Add ref that will be released when the dialog is completed + AddRef(); + } + + void ShowOpenDialog(const file_dialog::DialogSettings& settings) { + auto callback = base::Bind(&FileSelectHelper::OnOpenDialogDone, + base::Unretained(this)); + file_dialog::ShowOpenDialog(settings, callback); + } + + void ShowSaveDialog(const file_dialog::DialogSettings& settings) { + auto callback = base::Bind(&FileSelectHelper::OnSaveDialogDone, + base::Unretained(this)); + file_dialog::ShowSaveDialog(settings, callback); + } + + private: + void OnOpenDialogDone(bool result, const std::vector& paths) { + std::vector file_info; + if (result) { + for (auto& path : paths) { + content::FileChooserFileInfo info; + info.file_path = path; + info.display_name = path.BaseName().value(); + file_info.push_back(info); + } + + if (!paths.empty()) { + auto browser_context = static_cast( + render_frame_host_->GetProcess()->GetBrowserContext()); + browser_context->prefs()->SetFilePath(prefs::kSelectFileLastDirectory, + paths[0].DirName()); + } + } + OnFilesSelected(file_info); + } + + void OnSaveDialogDone(bool result, const base::FilePath& path) { + std::vector file_info; + if (result) { + content::FileChooserFileInfo info; + info.file_path = path; + info.display_name = path.BaseName().value(); + file_info.push_back(info); + } + OnFilesSelected(file_info); + } + + void OnFilesSelected( + const std::vector& file_info) { + if (render_frame_host_) + render_frame_host_->FilesSelectedInChooser(file_info, mode_); + Release(); + } + + // content::WebContentsObserver: + void RenderFrameHostChanged(content::RenderFrameHost* old_host, + content::RenderFrameHost* new_host) override { + if (old_host == render_frame_host_) + render_frame_host_ = nullptr; + } + + // content::WebContentsObserver: + void RenderFrameDeleted(content::RenderFrameHost* deleted_host) override { + if (deleted_host == render_frame_host_) + render_frame_host_ = nullptr; + } + + // content::WebContentsObserver: + void WebContentsDestroyed() override { + render_frame_host_ = nullptr; + } + + content::RenderFrameHost* render_frame_host_; + content::FileChooserParams::Mode mode_; +}; + file_dialog::Filters GetFileTypesFromAcceptType( const std::vector& accept_types) { file_dialog::Filters filters; @@ -87,15 +177,11 @@ void WebDialogHelper::RunFileChooser( settings.parent_window = window_; settings.title = base::UTF16ToUTF8(params.title); + scoped_refptr file_select_helper( + new FileSelectHelper(render_frame_host, params.mode)); if (params.mode == content::FileChooserParams::Save) { - base::FilePath path; settings.default_path = params.default_file_name; - if (file_dialog::ShowSaveDialog(settings, &path)) { - content::FileChooserFileInfo info; - info.file_path = path; - info.display_name = path.BaseName().value(); - result.push_back(info); - } + file_select_helper->ShowSaveDialog(settings); } else { int flags = file_dialog::FILE_DIALOG_CREATE_DIRECTORY; switch (params.mode) { @@ -111,27 +197,13 @@ void WebDialogHelper::RunFileChooser( NOTREACHED(); } - std::vector paths; AtomBrowserContext* browser_context = static_cast( window_->web_contents()->GetBrowserContext()); settings.default_path = browser_context->prefs()->GetFilePath( prefs::kSelectFileLastDirectory).Append(params.default_file_name); settings.properties = flags; - if (file_dialog::ShowOpenDialog(settings, &paths)) { - for (auto& path : paths) { - content::FileChooserFileInfo info; - info.file_path = path; - info.display_name = path.BaseName().value(); - result.push_back(info); - } - if (!paths.empty()) { - browser_context->prefs()->SetFilePath(prefs::kSelectFileLastDirectory, - paths[0].DirName()); - } - } + file_select_helper->ShowOpenDialog(settings); } - - render_frame_host->FilesSelectedInChooser(result, params.mode); } void WebDialogHelper::EnumerateDirectory(content::WebContents* web_contents, From 29f92bfb53a229a3c84aa6ff69f64d40c9699d9e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 23 Feb 2017 08:38:29 -0800 Subject: [PATCH 2/5] Extend RefCounted and add private destructor --- atom/browser/web_dialog_helper.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index 94ff44d551..8ea212504c 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -28,9 +28,7 @@ namespace { -class FileSelectHelper : public base::RefCountedThreadSafe< - FileSelectHelper, - content::BrowserThread::DeleteOnUIThread>, +class FileSelectHelper : public base::RefCounted, public content::WebContentsObserver { public: FileSelectHelper(content::RenderFrameHost* render_frame_host, @@ -44,18 +42,20 @@ class FileSelectHelper : public base::RefCountedThreadSafe< } void ShowOpenDialog(const file_dialog::DialogSettings& settings) { - auto callback = base::Bind(&FileSelectHelper::OnOpenDialogDone, - base::Unretained(this)); + auto callback = base::Bind(&FileSelectHelper::OnOpenDialogDone, this); file_dialog::ShowOpenDialog(settings, callback); } void ShowSaveDialog(const file_dialog::DialogSettings& settings) { - auto callback = base::Bind(&FileSelectHelper::OnSaveDialogDone, - base::Unretained(this)); + auto callback = base::Bind(&FileSelectHelper::OnSaveDialogDone, this); file_dialog::ShowSaveDialog(settings, callback); } private: + friend class base::RefCounted; + + ~FileSelectHelper() {} + void OnOpenDialogDone(bool result, const std::vector& paths) { std::vector file_info; if (result) { From a62c2f9e2e276cde2f989d815600480c71894cf7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 23 Feb 2017 08:52:48 -0800 Subject: [PATCH 3/5] Remove unused include --- atom/browser/web_dialog_helper.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index 8ea212504c..d9e747ad80 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -16,7 +16,6 @@ #include "base/strings/utf_string_conversions.h" #include "chrome/common/pref_names.h" #include "components/prefs/pref_service.h" -#include "content/public/browser/browser_thread.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_process_host.h" #include "content/public/browser/render_view_host.h" From 6837ec8576df4846f6fe5a46d7ce11506c9870a6 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 24 Feb 2017 07:49:26 -0800 Subject: [PATCH 4/5] Check render frame host before getting context --- atom/browser/web_dialog_helper.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index d9e747ad80..3ca5ba8f50 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -65,7 +65,7 @@ class FileSelectHelper : public base::RefCounted, file_info.push_back(info); } - if (!paths.empty()) { + if (render_frame_host_ && !paths.empty()) { auto browser_context = static_cast( render_frame_host_->GetProcess()->GetBrowserContext()); browser_context->prefs()->SetFilePath(prefs::kSelectFileLastDirectory, From ef085a1f15e5f1a8f0f1e19fe3f04c4f0e3247ab Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 24 Feb 2017 07:56:29 -0800 Subject: [PATCH 5/5] Remove unneeded AddRef/Release calls handled by base::Bind --- atom/browser/web_dialog_helper.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/atom/browser/web_dialog_helper.cc b/atom/browser/web_dialog_helper.cc index 3ca5ba8f50..448fccd31f 100644 --- a/atom/browser/web_dialog_helper.cc +++ b/atom/browser/web_dialog_helper.cc @@ -36,8 +36,6 @@ class FileSelectHelper : public base::RefCounted, auto web_contents = content::WebContents::FromRenderFrameHost( render_frame_host); content::WebContentsObserver::Observe(web_contents); - // Add ref that will be released when the dialog is completed - AddRef(); } void ShowOpenDialog(const file_dialog::DialogSettings& settings) { @@ -90,7 +88,6 @@ class FileSelectHelper : public base::RefCounted, const std::vector& file_info) { if (render_frame_host_) render_frame_host_->FilesSelectedInChooser(file_info, mode_); - Release(); } // content::WebContentsObserver: