Modernize SelectFileDialogImplKDE.

chrome/browser/ui/libgtk2ui/select_file_dialog_impl_kde.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <X11/Xlib.h>
 
+#include <memory>
 #include <set>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/nix/mime_util_xdg.h"
 #include "base/nix/xdg_util.h"
 #include "base/process/launch.h"
@@ skipping 49 matching lines @@
                       const FileTypeInfo* file_types,
                       int file_type_index,
                       const base::FilePath::StringType& default_extension,
                       gfx::NativeWindow owning_window,
                       void* params) override;
 
  private:
   bool HasMultipleFileTypeChoicesImpl() override;
 
   struct KDialogParams {
-    KDialogParams(const std::string& type, const std::string& title,
-                  const base::FilePath& default_path, XID parent,
-                  bool file_operation, bool multiple_selection,
-                  void* kdialog_params,
-                  void (SelectFileDialogImplKDE::*callback)(XID,
-                                                            const std::string&,
-                                                            int, void*))
-        : type(type), title(title), default_path(default_path), parent(parent),
+    KDialogParams(const std::string& type,
+                  const std::string& title,
+                  const base::FilePath& default_path,
+                  XID parent,
+                  bool file_operation,
+                  bool multiple_selection)
+        : type(type),
+          title(title),
+          default_path(default_path),
+          parent(parent),
           file_operation(file_operation),
-          multiple_selection(multiple_selection),
-          kdialog_params(kdialog_params), callback(callback) {
-    }
+          multiple_selection(multiple_selection) {}
 
     std::string type;
     std::string title;
     base::FilePath default_path;
     XID parent;
     bool file_operation;
     bool multiple_selection;
-    void* kdialog_params;
-    void (SelectFileDialogImplKDE::*callback)(XID, const std::string&,
-                                              int, void*);
+  };
+
+  struct KDialogOutputParams {
+    std::string output;
+    int exit_code;
   };
 
   // Get the filters from |file_types_| and concatenate them into
   // |filter_string|.
   std::string GetMimeTypeFilterString();
 
   // Get KDialog command line representing the Argv array for KDialog.
   void GetKDialogCommandLine(const std::string& type,
                              const std::string& title,
                              const base::FilePath& default_path,
                              XID parent,
                              bool file_operation,
                              bool multiple_selection,
                              base::CommandLine* command_line);
 
-  // Call KDialog on the FILE thread and post results back to the UI thread.
-  void CallKDialogOutput(const KDialogParams& params);
+  // Call KDialog on the FILE thread and return the results.
+  std::unique_ptr<KDialogOutputParams> CallKDialogOutput(
+      const KDialogParams& params);
 
   // Notifies the listener that a single file was chosen.
   void FileSelected(const base::FilePath& path, void* params);
 
   // Notifies the listener that multiple files were chosen.
   void MultiFilesSelected(const std::vector<base::FilePath>& files,
                           void* params);
 
   // Notifies the listener that no file was chosen (the action was canceled).
   // Dialog is passed so we can find that |params| pointer that was passed to
@@ skipping 12 matching lines @@
   void CreateMultiFileOpenDialog(const std::string& title,
                                  const base::FilePath& default_path,
                                  XID parent, void* params);
 
   void CreateSaveAsDialog(const std::string& title,
                           const base::FilePath& default_path,
                           XID parent, void* params);
 
   // Common function for OnSelectSingleFileDialogResponse and
   // OnSelectSingleFolderDialogResponse.
-  void SelectSingleFileHelper(const std::string& output, int exit_code,
-                              void* params, bool allow_folder);
+  void SelectSingleFileHelper(void* params,
+                              bool allow_folder,
+                              std::unique_ptr<KDialogOutputParams> results);
 
-  void OnSelectSingleFileDialogResponse(XID parent,
-                                        const std::string& output,
-                                        int exit_code, void* params);
-  void OnSelectMultiFileDialogResponse(XID parent,
-                                       const std::string& output,
-                                       int exit_code, void* params);
-  void OnSelectSingleFolderDialogResponse(XID parent,
-                                          const std::string& output,
-                                          int exit_code, void* params);
+  void OnSelectSingleFileDialogResponse(
+      XID parent,
+      void* params,
+      std::unique_ptr<KDialogOutputParams> results);
+  void OnSelectMultiFileDialogResponse(
+      XID parent,
+      void* params,
+      std::unique_ptr<KDialogOutputParams> results);
+  void OnSelectSingleFolderDialogResponse(
+      XID parent,
+      void* params,
+      std::unique_ptr<KDialogOutputParams> results);
 
   // Should be either DESKTOP_ENVIRONMENT_KDE3, KDE4, or KDE5.
   base::nix::DesktopEnvironment desktop_;
 
   // The set of all parent windows for which we are currently running
   // dialogs. This should only be accessed on the UI thread.
   std::set<XID> parents_;
 
   DISALLOW_COPY_AND_ASSIGN(SelectFileDialogImplKDE);
 };
@@ skipping 119 matching lines @@
     filter_set.insert("application/octet-stream");
   // Create the final output string.
   filter_string.clear();
   for (std::set<std::string>::iterator it = filter_set.begin();
        it != filter_set.end(); ++it) {
     filter_string.append(*it + " ");
   }
   return filter_string;
 }
 
-void SelectFileDialogImplKDE::CallKDialogOutput(const KDialogParams& params) {
+std::unique_ptr<SelectFileDialogImplKDE::KDialogOutputParams>
+SelectFileDialogImplKDE::CallKDialogOutput(const KDialogParams& params) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   base::CommandLine::StringVector cmd_vector;
   cmd_vector.push_back(kKdialogBinary);
   base::CommandLine command_line(cmd_vector);
   GetKDialogCommandLine(params.type, params.title, params.default_path,
                         params.parent, params.file_operation,
                         params.multiple_selection, &command_line);
-  std::string output;
-  int exit_code;
+
+  auto results = base::MakeUnique<KDialogOutputParams>();
+  std::string& output = results->output;
+  int& exit_code = results->exit_code;
   // Get output from KDialog
   base::GetAppOutputWithExitCode(command_line, &output, &exit_code);

[Tom (Use chromium acct), 2016/08/26 18:46:23]

Nit: Can we just do &results->output, &results->exit_code and skip the
temporaries?

[Lei Zhang, 2016/08/26 19:18:08]

Done.

   if (!output.empty())
     output.erase(output.size() - 1);
-
-  // Now the dialog is no longer showing, but we can't erase its parent from the
-  // parent set yet because we're on the FILE thread.
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(params.callback, this, params.parent, output, exit_code,
-                 params.kdialog_params));
+  return results;
 }
 
 void SelectFileDialogImplKDE::GetKDialogCommandLine(
     const std::string& type,
     const std::string& title,
     const base::FilePath& path,
     XID parent,
     bool file_operation,
     bool multiple_selection,
     base::CommandLine* command_line) {
@@ skipping 55 matching lines @@
   if (listener_)
     listener_->FileSelectionCanceled(params);
 }
 
 void SelectFileDialogImplKDE::CreateSelectFolderDialog(
     Type type, const std::string& title, const base::FilePath& default_path,
     XID parent, void *params) {
   int title_message_id = (type == SELECT_UPLOAD_FOLDER)
       ? IDS_SELECT_UPLOAD_FOLDER_DIALOG_TITLE
       : IDS_SELECT_FOLDER_DIALOG_TITLE;
-  BrowserThread::PostTask(
+  BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
-          &SelectFileDialogImplKDE::CallKDialogOutput,
-          this,
+          &SelectFileDialogImplKDE::CallKDialogOutput, this,
           KDialogParams(
-              "--getexistingdirectory",
-              GetTitle(title, title_message_id),
-              default_path.empty() ? *last_opened_path_ : default_path,
-              parent, false, false, params,
-              &SelectFileDialogImplKDE::OnSelectSingleFolderDialogResponse)));
+              "--getexistingdirectory", GetTitle(title, title_message_id),
+              default_path.empty() ? *last_opened_path_ : default_path, parent,
+              false, false)),
+      base::Bind(&SelectFileDialogImplKDE::OnSelectSingleFolderDialogResponse,
+                 this, parent, params));
 }
 
 void SelectFileDialogImplKDE::CreateFileOpenDialog(
     const std::string& title, const base::FilePath& default_path,
     XID parent, void* params) {
-  BrowserThread::PostTask(
+  BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
-          &SelectFileDialogImplKDE::CallKDialogOutput,
-          this,
+          &SelectFileDialogImplKDE::CallKDialogOutput, this,
           KDialogParams(
-              "--getopenfilename",
-              GetTitle(title, IDS_OPEN_FILE_DIALOG_TITLE),
-              default_path.empty() ? *last_opened_path_ : default_path,
-              parent, true, false, params,
-              &SelectFileDialogImplKDE::OnSelectSingleFileDialogResponse)));
+              "--getopenfilename", GetTitle(title, IDS_OPEN_FILE_DIALOG_TITLE),
+              default_path.empty() ? *last_opened_path_ : default_path, parent,
+              true, false)),
+      base::Bind(&SelectFileDialogImplKDE::OnSelectSingleFileDialogResponse,
+                 this, parent, params));
 }
 
 void SelectFileDialogImplKDE::CreateMultiFileOpenDialog(
     const std::string& title, const base::FilePath& default_path,
     XID parent, void* params) {
-  BrowserThread::PostTask(
+  BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
-          &SelectFileDialogImplKDE::CallKDialogOutput,
-          this,
+          &SelectFileDialogImplKDE::CallKDialogOutput, this,
           KDialogParams(
-              "--getopenfilename",
-              GetTitle(title, IDS_OPEN_FILES_DIALOG_TITLE),
-              default_path.empty() ? *last_opened_path_ : default_path,
-              parent, true, true, params,
-              &SelectFileDialogImplKDE::OnSelectMultiFileDialogResponse)));
+              "--getopenfilename", GetTitle(title, IDS_OPEN_FILES_DIALOG_TITLE),
+              default_path.empty() ? *last_opened_path_ : default_path, parent,
+              true, true)),
+      base::Bind(&SelectFileDialogImplKDE::OnSelectMultiFileDialogResponse,
+                 this, parent, params));
 }
 
 void SelectFileDialogImplKDE::CreateSaveAsDialog(
     const std::string& title, const base::FilePath& default_path,
     XID parent, void* params) {
-  BrowserThread::PostTask(
+  BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(
-          &SelectFileDialogImplKDE::CallKDialogOutput,
-          this,
-          KDialogParams(
-              "--getsavefilename",
-              GetTitle(title, IDS_SAVE_AS_DIALOG_TITLE),
-              default_path.empty() ? *last_saved_path_ : default_path,
-              parent, true, false, params,
-              &SelectFileDialogImplKDE::OnSelectSingleFileDialogResponse)));
+          &SelectFileDialogImplKDE::CallKDialogOutput, this,
+          KDialogParams("--getsavefilename",
+                        GetTitle(title, IDS_SAVE_AS_DIALOG_TITLE),
+                        default_path.empty() ? *last_saved_path_ : default_path,
+                        parent, true, false)),
+      base::Bind(&SelectFileDialogImplKDE::OnSelectSingleFileDialogResponse,
+                 this, parent, params));
 }
 
-void SelectFileDialogImplKDE::SelectSingleFileHelper(const std::string& output,
-    int exit_code, void* params, bool allow_folder) {
-  VLOG(1) << "[kdialog] SingleFileResponse: " << output;
-  if (exit_code != 0 || output.empty()) {
+void SelectFileDialogImplKDE::SelectSingleFileHelper(
+    void* params,
+    bool allow_folder,
+    std::unique_ptr<KDialogOutputParams> results) {
+  VLOG(1) << "[kdialog] SingleFileResponse: " << results->output;
+  if (results->exit_code || results->output.empty()) {
     FileNotSelected(params);
     return;
   }
 
-  base::FilePath path(output);
+  base::FilePath path(results->output);
   if (allow_folder) {
     FileSelected(path, params);
     return;
   }
 
   if (CallDirectoryExistsOnUIThread(path))
     FileNotSelected(params);
   else
     FileSelected(path, params);
 }
 
 void SelectFileDialogImplKDE::OnSelectSingleFileDialogResponse(
-    XID parent, const std::string& output, int exit_code, void* params) {
+    XID parent,
+    void* params,
+    std::unique_ptr<KDialogOutputParams> results) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   parents_.erase(parent);
-  SelectSingleFileHelper(output, exit_code, params, false);
+  SelectSingleFileHelper(params, false, std::move(results));
 }
 
 void SelectFileDialogImplKDE::OnSelectSingleFolderDialogResponse(
-    XID parent, const std::string& output, int exit_code, void* params) {
+    XID parent,
+    void* params,
+    std::unique_ptr<KDialogOutputParams> results) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   parents_.erase(parent);
-  SelectSingleFileHelper(output, exit_code, params, true);
+  SelectSingleFileHelper(params, true, std::move(results));
 }
 
 void SelectFileDialogImplKDE::OnSelectMultiFileDialogResponse(
-    XID parent, const std::string& output, int exit_code, void* params) {
+    XID parent,
+    void* params,
+    std::unique_ptr<KDialogOutputParams> results) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  VLOG(1) << "[kdialog] MultiFileResponse: " << output;
+  VLOG(1) << "[kdialog] MultiFileResponse: " << results->output;
 
   parents_.erase(parent);
 
-  if (exit_code != 0 || output.empty()) {
+  if (results->exit_code || results->output.empty()) {
     FileNotSelected(params);
     return;
   }
 
   std::vector<base::FilePath> filenames_fp;
   for (const base::StringPiece& line :
-       base::SplitStringPiece(output, "\n", base::KEEP_WHITESPACE,
+       base::SplitStringPiece(results->output, "\n", base::KEEP_WHITESPACE,
                               base::SPLIT_WANT_NONEMPTY)) {
     base::FilePath path(line);
     if (CallDirectoryExistsOnUIThread(path))
       continue;
     filenames_fp.push_back(path);
   }
 
   if (filenames_fp.empty()) {
     FileNotSelected(params);
     return;
   }
   MultiFilesSelected(filenames_fp, params);
 }
 
 }  // namespace libgtk2ui