Fix sandboxed crashReporter for windows.

- Use `path` module from browser process in sandboxed renderer. This is required
  because the return value of `path.join` is platform-specific, and this is an
  assumtion of crash-reporter.js which is shared between sandboxed and
  non-sandboxed renderers.
- Set `process.platform` and `process.execPath` in sandboxed renderer
  environment. This is required to spawn the windows crash service from
  sandboxed renderer.
- Use a single temporary directory for all crashReporter tests. This is required
  to make tests more deterministic across platforms(since mac's crashpad doesn't
  support changing the crash dump directory). Also make a few improvements/fixes
  to the `uploadToServer` test.

electron.gyp

@@ -447,6 +447,8 @@
           '-r',
           './lib/sandboxed_renderer/api/exports/os.js:os',
           '-r',
+          './lib/sandboxed_renderer/api/exports/path.js:path',
+          '-r',
           './lib/sandboxed_renderer/api/exports/child_process.js:child_process'
         ],
         'isolated_args': [

lib/sandboxed_renderer/api/exports/path.js

@@ -0,0 +1 @@
+module.exports = require('electron').remote.require('path')

lib/sandboxed_renderer/init.js

@@ -27,6 +27,7 @@ const preloadModules = new Map([
   ['electron', electron],
   ['fs', fs],
   ['os', require('os')],
+  ['path', require('path')],
   ['url', require('url')],
   ['timers', require('timers')]
 ])
@@ -36,8 +37,9 @@ const preloadSrc = fs.readFileSync(preloadPath).toString()
 // Pass different process object to the preload script(which should not have
 // access to things like `process.atomBinding`).
 const preloadProcess = new events.EventEmitter()
-preloadProcess.platform = electron.remote.process.platform
 preloadProcess.crash = () => binding.crash()
+process.platform = preloadProcess.platform = electron.remote.process.platform
+process.execPath = preloadProcess.execPath = electron.remote.process.execPath
 process.on('exit', () => preloadProcess.emit('exit'))
 
 // This is the `require` function that will be visible to the preload script

spec/api-crash-reporter-spec.js

@@ -15,24 +15,32 @@ describe('crashReporter module', function () {
   if (process.mas) {
     return
   }
+
+  var originalTempDirectory = null
+  var tempDirectory = null
+
+  before(function () {
+    tempDirectory = temp.mkdirSync('electronCrashReporterSpec-')
+    originalTempDirectory = app.getPath('temp')
+    app.setPath('temp', tempDirectory)
+  })
+
+  after(function () {
+    app.setPath('temp', originalTempDirectory)
+  })
+
   var fixtures = path.resolve(__dirname, 'fixtures')
   const generateSpecs = (description, browserWindowOpts) => {
     describe(description, function () {
       var w = null
-      var originalTempDirectory = null
-      var tempDirectory = null
 
       beforeEach(function () {
         w = new BrowserWindow(Object.assign({
           show: false
         }, browserWindowOpts))
-        tempDirectory = temp.mkdirSync('electronCrashReporterSpec-')
-        originalTempDirectory = app.getPath('temp')
-        app.setPath('temp', tempDirectory)
       })
 
       afterEach(function () {
-        app.setPath('temp', originalTempDirectory)
         return closeWindow(w).then(function () { w = null })
       })
 
@@ -77,13 +85,15 @@ describe('crashReporter module', function () {
       it('should not send minidump if uploadToServer is false', function (done) {
         this.timeout(120000)
 
-        if (process.platform === 'darwin') {
-          crashReporter.setUploadToServer(false)
-        }
-
         let server
         let dumpFile
-        let crashesDir
+        let crashesDir = crashReporter.getCrashesDirectory()
+        const existingDumpFiles = new Set()
+        if (process.platform === 'darwin') {
+          // crashpad puts the dump files in the "completed" subdirectory
+          crashesDir = path.join(crashesDir, 'completed')
+          crashReporter.setUploadToServer(false)
+        }
         const testDone = (uploaded) => {
           if (uploaded) {
             return done(new Error('fail'))
@@ -93,7 +103,6 @@ describe('crashReporter module', function () {
             crashReporter.setUploadToServer(true)
           }
           assert(fs.existsSync(dumpFile))
-          fs.unlinkSync(dumpFile)
           done()
         }
 
@@ -103,7 +112,7 @@ describe('crashReporter module', function () {
             if (err) {
               return
             }
-            const dumps = files.filter((file) => /\.dmp$/.test(file))
+            const dumps = files.filter((file) => /\.dmp$/.test(file) && !existingDumpFiles.has(file))
             if (!dumps.length) {
               return
             }
@@ -111,34 +120,17 @@ describe('crashReporter module', function () {
             dumpFile = path.join(crashesDir, dumps[0])
             clearInterval(pollInterval)
             // dump file should not be deleted when not uploading, so we wait
-            // 500 ms and assert it still exists in `testDone`
+            // 1s and assert it still exists in `testDone`
-            setTimeout(testDone, 500)
+            setTimeout(testDone, 1000)
           })
         }
 
-        remote.ipcMain.once('set-crash-directory', (event, dir) => {
+        remote.ipcMain.once('list-existing-dumps', (event) => {
-          if (process.platform === 'linux') {
-            crashesDir = dir
-          } else {
-            crashesDir = crashReporter.getCrashesDirectory()
-            if (process.platform === 'darwin') {
-              // crashpad uses an extra subdirectory
-              crashesDir = path.join(crashesDir, 'completed')
-            }
-          }
-
-          // Before starting, remove all dump files in the crash directory.
-          // This is required because:
-          // - mac crashpad not seem to allow changing the crash directory after
-          //   the first "start" call.
-          // - Other tests in this suite may leave dumps there.
-          // - We want to verify in `testDone` that a dump file is created and
-          //   not deleted.
           fs.readdir(crashesDir, (err, files) => {
             if (!err) {
               for (const file of files) {
                 if (/\.dmp$/.test(file)) {
-                  fs.unlinkSync(path.join(crashesDir, file))
+                  existingDumpFiles.add(file)
                 }
               }
             }

spec/fixtures/api/crash.html

@@ -17,7 +17,7 @@ crashReporter.start({
   }
 });
 if (!uploadToServer) {
-  ipcRenderer.sendSync('set-crash-directory', crashReporter.getCrashesDirectory())
+  ipcRenderer.sendSync('list-existing-dumps')
 }
 setImmediate(function() { process.crash(); });
 </script>