Pass crashes directory instead of product name and temp dir

atom/common/api/atom_api_crash_reporter.cc

@@ -40,8 +40,6 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
                  base::Bind(&CrashReporter::Start, report));
   dict.SetMethod("_getUploadedReports",
                  base::Bind(&CrashReporter::GetUploadedReports, report));
-  dict.SetMethod("_getCrashesDirectory",
-                 base::Bind(&CrashReporter::GetCrashesDirectory, report));
 }
 
 }  // namespace

atom/common/crash_reporter/crash_reporter.cc

@@ -10,7 +10,6 @@
 #include "base/files/file_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
-#include "base/strings/utf_string_conversions.h"
 #include "content/public/common/content_switches.h"
 
 namespace crash_reporter {
@@ -26,24 +25,14 @@ CrashReporter::~CrashReporter() {
 void CrashReporter::Start(const std::string& product_name,
                           const std::string& company_name,
                           const std::string& submit_url,
-                          const base::FilePath& temp_path,
+                          const base::FilePath& crashes_dir,
                           bool auto_submit,
                           bool skip_system_crash_handler,
                           const StringMap& extra_parameters) {
   SetUploadParameters(extra_parameters);
 
   InitBreakpad(product_name, ATOM_VERSION_STRING, company_name, submit_url,
-               temp_path, auto_submit, skip_system_crash_handler);
+               crashes_dir, auto_submit, skip_system_crash_handler);
-}
-
-base::FilePath CrashReporter::GetCrashesDirectory(
-    const std::string& product_name, const base::FilePath& temp_path) {
-  std::string folder_name = product_name + " Crashes";
-#if defined(OS_WIN)
-  return temp_path.Append(base::UTF8ToUTF16(folder_name));
-#else
-  return temp_path.Append(folder_name);
-#endif
 }
 
 void CrashReporter::SetUploadParameters(const StringMap& parameters) {
@@ -55,14 +44,11 @@ void CrashReporter::SetUploadParameters(const StringMap& parameters) {
 }
 
 std::vector<CrashReporter::UploadReportResult>
-CrashReporter::GetUploadedReports(const std::string& product_name,
+CrashReporter::GetUploadedReports(const base::FilePath& crashes_dir) {
-                                  const base::FilePath& temp_path) {
-  std::vector<CrashReporter::UploadReportResult> result;
-
-  base::FilePath uploads_path =
-      GetCrashesDirectory(product_name, temp_path)
-          .Append(FILE_PATH_LITERAL("uploads.log"));
   std::string file_content;
+  std::vector<CrashReporter::UploadReportResult> result;
+  base::FilePath uploads_path =
+      crashes_dir.Append(FILE_PATH_LITERAL("uploads.log"));
   if (base::ReadFileToString(uploads_path, &file_content)) {
     std::vector<std::string> reports = base::SplitString(
         file_content, "\n", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
@@ -84,7 +70,7 @@ void CrashReporter::InitBreakpad(const std::string& product_name,
                                  const std::string& version,
                                  const std::string& company_name,
                                  const std::string& submit_url,
-                                 const base::FilePath& temp_path,
+                                 const base::FilePath& crashes_dir,
                                  bool auto_submit,
                                  bool skip_system_crash_handler) {
 }

atom/common/crash_reporter/crash_reporter.h

@@ -25,17 +25,13 @@ class CrashReporter {
   void Start(const std::string& product_name,
              const std::string& company_name,
              const std::string& submit_url,
-             const base::FilePath& temp_path,
+             const base::FilePath& crashes_dir,
              bool auto_submit,
              bool skip_system_crash_handler,
              const StringMap& extra_parameters);
 
   virtual std::vector<CrashReporter::UploadReportResult> GetUploadedReports(
-      const std::string& product_name,
+      const base::FilePath& crashes_dir);
-      const base::FilePath& temp_path);
-
-  base::FilePath GetCrashesDirectory(const std::string& product_name,
-                                     const base::FilePath& temp_path);
 
  protected:
   CrashReporter();
@@ -45,7 +41,7 @@ class CrashReporter {
                             const std::string& version,
                             const std::string& company_name,
                             const std::string& submit_url,
-                            const base::FilePath& temp_path,
+                            const base::FilePath& crashes_dir,
                             bool auto_submit,
                             bool skip_system_crash_handler);
   virtual void SetUploadParameters();

atom/common/crash_reporter/crash_reporter_linux.cc

@@ -17,7 +17,6 @@
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/process/memory.h"
-#include "base/strings/stringprintf.h"
 #include "vendor/breakpad/src/client/linux/handler/exception_handler.h"
 #include "vendor/breakpad/src/common/linux/linux_libc_support.h"
 
@@ -60,10 +59,10 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name,
                                       const std::string& version,
                                       const std::string& company_name,
                                       const std::string& submit_url,
-                                      const base::FilePath& temp_path,
+                                      const base::FilePath& crashes_dir,
                                       bool auto_submit,
                                       bool skip_system_crash_handler) {
-  EnableCrashDumping(product_name, temp_path);
+  EnableCrashDumping(crashes_dir);
 
   crash_keys_.SetKeyValue("prod", ATOM_PRODUCT_NAME);
   crash_keys_.SetKeyValue("ver", version.c_str());
@@ -78,16 +77,13 @@ void CrashReporterLinux::SetUploadParameters() {
   upload_parameters_["platform"] = "linux";
 }
 
-void CrashReporterLinux::EnableCrashDumping(const std::string& product_name,
+void CrashReporterLinux::EnableCrashDumping(const base::FilePath& crashes_dir) {
-                                            const base::FilePath& temp_path) {
+  base::CreateDirectory(crashes_dir);
-  base::FilePath dumps_path = GetCrashesDirectory(product_name, temp_path);
-  base::CreateDirectory(dumps_path);
 
-  std::string log_file = base::StringPrintf(
+  std::string log_file = crashes_dir.Append("uploads.log").value();
-      "%s/%s", dumps_path.value().c_str(), "uploads.log");
   strncpy(g_crash_log_path, log_file.c_str(), sizeof(g_crash_log_path));
 
-  MinidumpDescriptor minidump_descriptor(dumps_path.value());
+  MinidumpDescriptor minidump_descriptor(crashes_dir.value());
   minidump_descriptor.set_size_limit(kMaxMinidumpFileSize);
 
   breakpad_.reset(new ExceptionHandler(

atom/common/crash_reporter/crash_reporter_linux.h

@@ -31,7 +31,7 @@ class CrashReporterLinux : public CrashReporter {
                     const std::string& version,
                     const std::string& company_name,
                     const std::string& submit_url,
-                    const base::FilePath& temp_path,
+                    const base::FilePath& crashes_dir,
                     bool auto_submit,
                     bool skip_system_crash_handler) override;
   void SetUploadParameters() override;
@@ -42,8 +42,7 @@ class CrashReporterLinux : public CrashReporter {
   CrashReporterLinux();
   virtual ~CrashReporterLinux();
 
-  void EnableCrashDumping(const std::string& product_name,
+  void EnableCrashDumping(const base::FilePath& crashes_dir);
-                          const base::FilePath& temp_path);
 
   static bool CrashDone(const google_breakpad::MinidumpDescriptor& minidump,
                         void* context,

atom/common/crash_reporter/crash_reporter_mac.h

@@ -27,7 +27,7 @@ class CrashReporterMac : public CrashReporter {
                     const std::string& version,
                     const std::string& company_name,
                     const std::string& submit_url,
-                    const base::FilePath& temp_path,
+                    const base::FilePath& crashes_dir,
                     bool auto_submit,
                     bool skip_system_crash_handler) override;
   void SetUploadParameters() override;
@@ -43,7 +43,7 @@ class CrashReporterMac : public CrashReporter {
                         const base::StringPiece& value);
 
   std::vector<UploadReportResult> GetUploadedReports(
-      const std::string& path, const base::FilePath& temp_path) override;
+      const base::FilePath& crashes_dir) override;
 
   std::unique_ptr<crashpad::SimpleStringDictionary> simple_string_dictionary_;
 

atom/common/crash_reporter/crash_reporter_mac.mm

@@ -6,7 +6,6 @@
 
 #include <memory>
 
-#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/mac/bundle_locations.h"
 #include "base/mac/mac_util.h"
@@ -31,7 +30,7 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name,
                                     const std::string& version,
                                     const std::string& company_name,
                                     const std::string& submit_url,
-                                    const base::FilePath& temp_path,
+                                    const base::FilePath& crashes_dir,
                                     bool auto_submit,
                                     bool skip_system_crash_handler) {
   // check whether crashpad has been initialized.
@@ -39,7 +38,6 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name,
   if (simple_string_dictionary_)
     return;
 
-  base::FilePath database_path = GetCrashesDirectory(product_name, temp_path);
   if (is_browser_) {
     @autoreleasepool {
       base::FilePath framework_bundle_path = base::mac::FrameworkBundlePath();
@@ -47,7 +45,7 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name,
           framework_bundle_path.Append("Resources").Append("crashpad_handler");
 
       crashpad::CrashpadClient crashpad_client;
-      if (crashpad_client.StartHandler(handler_path, database_path,
+      if (crashpad_client.StartHandler(handler_path, crashes_dir,
                                        submit_url,
                                        StringMap(),
                                        std::vector<std::string>(),
@@ -76,7 +74,7 @@ void CrashReporterMac::InitBreakpad(const std::string& product_name,
   }
   if (is_browser_) {
     std::unique_ptr<crashpad::CrashReportDatabase> database =
-        crashpad::CrashReportDatabase::Initialize(database_path);
+        crashpad::CrashReportDatabase::Initialize(crashes_dir);
     if (database) {
       database->GetSettings()->SetUploadsEnabled(auto_submit);
     }
@@ -93,17 +91,15 @@ void CrashReporterMac::SetCrashKeyValue(const base::StringPiece& key,
 }
 
 std::vector<CrashReporter::UploadReportResult>
-CrashReporterMac::GetUploadedReports(const std::string& product_name,
+CrashReporterMac::GetUploadedReports(const base::FilePath& crashes_dir) {
-                                     const base::FilePath& temp_path) {
   std::vector<CrashReporter::UploadReportResult> uploaded_reports;
 
-  base::FilePath file_path = GetCrashesDirectory(product_name, temp_path);
+  if (!base::PathExists(crashes_dir)) {
-  if (!base::PathExists(file_path)) {
     return uploaded_reports;
   }
   // Load crashpad database.
   std::unique_ptr<crashpad::CrashReportDatabase> database =
-    crashpad::CrashReportDatabase::Initialize(file_path);
+    crashpad::CrashReportDatabase::Initialize(crashes_dir);
   DCHECK(database);
 
   std::vector<crashpad::CrashReportDatabase::Report> completed_reports;

atom/common/crash_reporter/crash_reporter_win.cc

@@ -149,7 +149,7 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name,
                                     const std::string& version,
                                     const std::string& company_name,
                                     const std::string& submit_url,
-                                    const base::FilePath& temp_path,
+                                    const base::FilePath& crashes_dir,
                                     bool auto_submit,
                                     bool skip_system_crash_handler) {
   skip_system_crash_handler_ = skip_system_crash_handler;
@@ -172,7 +172,7 @@ void CrashReporterWin::InitBreakpad(const std::string& product_name,
   breakpad_.reset();
 
   breakpad_.reset(new google_breakpad::ExceptionHandler(
-      temp_path.value(),
+      crashes_dir.DirName().value(),
       FilterCallback,
       MinidumpCallback,
       this,

atom/common/crash_reporter/crash_reporter_win.h

@@ -27,7 +27,7 @@ class CrashReporterWin : public CrashReporter {
                     const std::string& version,
                     const std::string& company_name,
                     const std::string& submit_url,
-                    const base::FilePath& temp_path,
+                    const base::FilePath& crashes_dir,
                     bool auto_submit,
                     bool skip_system_crash_handler) override;
   void SetUploadParameters() override;

lib/common/api/crash-reporter.js

@@ -2,6 +2,7 @@
 
 const {spawn} = require('child_process')
 const os = require('os')
+const path = require('path')
 const electron = require('electron')
 const {app} = process.type === 'browser' ? electron : electron.remote
 const binding = process.atomBinding('crash_reporter')
@@ -11,13 +12,9 @@ class CrashReporter {
     if (options == null) {
       options = {}
     }
-    this.productName = options.productName
+    this.productName = options.productName != null ? options.productName : app.getName()
     let {autoSubmit, companyName, extra, ignoreSystemCrashHandler, submitURL} = options
 
-    this.tempDirectory = getTempPath()
-    if (this.productName == null) {
-      this.productName = app.getName()
-    }
     if (autoSubmit == null) {
       autoSubmit = true
     }
@@ -28,7 +25,7 @@ class CrashReporter {
       extra = {}
     }
     if (extra._productName == null) {
-      extra._productName = this.productName
+      extra._productName = this.getProductName()
     }
     if (extra._companyName == null) {
       extra._companyName = companyName
@@ -46,8 +43,8 @@ class CrashReporter {
     if (process.platform === 'win32') {
       const args = [
         '--reporter-url=' + submitURL,
-        '--application-name=' + this.productName,
+        '--application-name=' + this.getProductName(),
-        '--crashes-directory=' + binding._getCrashesDirectory(this.productName, this.tempDirectory),
+        '--crashes-directory=' + this.getCrashesDirectory(),
         '--v=1'
       ]
       const env = {
@@ -59,7 +56,7 @@ class CrashReporter {
       })
     }
 
-    binding.start(this.productName, companyName, submitURL, this.tempDirectory, autoSubmit, ignoreSystemCrashHandler, extra)
+    binding.start(this.getProductName(), companyName, submitURL, this.getCrashesDirectory(), autoSubmit, ignoreSystemCrashHandler, extra)
   }
 
   getLastCrashReport () {
@@ -72,18 +69,31 @@ class CrashReporter {
   }
 
   getUploadedReports () {
-    const productName = this.productName != null ? this.productName : app.getName()
+    return binding._getUploadedReports(this.getCrashesDirectory())
-    const tempDirectory = this.tempDirectory != null ? this.tempDirectory : getTempPath()
-    return binding._getUploadedReports(productName, tempDirectory)
   }
-}
 
-const getTempPath = () => {
+  getCrashesDirectory () {
-  try {
+    const crashesDir = this.getProductName() + ' Crashes'
-    return app.getPath('temp')
+    return path.join(this.getTempDirectory(), crashesDir)
-  } catch (error) {
+  }
-    // app.getPath may throw so fallback to OS temp directory
+
-    return os.tmpdir()
+  getProductName () {
+    if (this.productName == null) {
+      this.productName = app.getName()
+    }
+    return this.productName
+  }
+
+  getTempDirectory () {
+    if (this.tempDirectory == null) {
+      try {
+        this.tempDirectory = app.getPath('temp')
+      } catch (error) {
+        // app.getPath may throw so fallback to OS temp directory
+        this.tempDirectory = os.tmpdir()
+      }
+    }
+    return this.tempDirectory
   }
 }