fix: reject contentTracing.stopRecording on failure (#26656)

Co-authored-by: Jeremy Rose <nornagon@nornagon.net>
electron · Nov 23, 2020 · 1cac335 · 1cac335
1 parent 57c69ab
commit 1cac335
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 6 deletions.
diff --git a/shell/browser/api/electron_api_content_tracing.cc b/shell/browser/api/electron_api_content_tracing.cc
@@ -66,15 +66,26 @@ base::Optional<base::FilePath> CreateTemporaryFileOnIO() {
 
 void StopTracing(gin_helper::Promise<base::FilePath> promise,
                  base::Optional<base::FilePath> file_path) {
+  auto resolve_or_reject = base::AdaptCallbackForRepeating(base::BindOnce(
+      [](gin_helper::Promise<base::FilePath> promise,
+         const base::FilePath& path, base::Optional<std::string> error) {
+        if (error) {
+          promise.RejectWithErrorMessage(error.value());
+        } else {
+          promise.Resolve(path);
+        }
+      },
+      std::move(promise), *file_path));
   if (file_path) {
     auto endpoint = TracingController::CreateFileEndpoint(
-        *file_path, base::AdaptCallbackForRepeating(base::BindOnce(
-                        &gin_helper::Promise<base::FilePath>::ResolvePromise,
-                        std::move(promise), *file_path)));
-    TracingController::GetInstance()->StopTracing(endpoint);
+        *file_path, base::BindRepeating(resolve_or_reject, base::nullopt));
+    if (!TracingController::GetInstance()->StopTracing(endpoint)) {
+      resolve_or_reject.Run(base::make_optional(
+          "Failed to stop tracing (was a trace in progress?)"));
+    }
   } else {
-    promise.RejectWithErrorMessage(
-        "Failed to create temporary file for trace data");
+    resolve_or_reject.Run(
+        base::make_optional("Failed to create temporary file for trace data"));
   }
 }
 

diff --git a/spec-main/api-content-tracing-spec.ts b/spec-main/api-content-tracing-spec.ts
@@ -119,6 +119,10 @@ ifdescribe(!(process.platform !== 'win32' && ['arm', 'arm64'].includes(process.a
       const resultFilePath = await record(/* options */ {}, /* outputFilePath */ undefined);
       expect(resultFilePath).to.be.a('string').that.is.not.empty('result path');
     });
+
+    it('rejects if no trace is happening', async () => {
+      await expect(contentTracing.stopRecording()).to.be.rejected();
+    });
   });
 
   describe('captured events', () => {