Address review feedback

electron · Jul 29, 2020 · c0a3138 · c0a3138
1 parent eeee943
commit c0a3138
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 14 deletions.
diff --git a/shell/common/api/electron_api_v8_util.cc b/shell/common/api/electron_api_v8_util.cc
@@ -103,16 +103,6 @@ void RequestGarbageCollectionForTesting(v8::Isolate* isolate) {
       v8::Isolate::GarbageCollectionType::kFullGarbageCollection);
 }
 
-// This causes a fatal error by creating a circular extension dependency.
-void TriggerFatalErrorForTesting(v8::Isolate* isolate) {
-  static const char* aDeps[] = {"B"};
-  v8::RegisterExtension(std::make_unique<v8::Extension>("A", "", 1, aDeps));
-  static const char* bDeps[] = {"A"};
-  v8::RegisterExtension(std::make_unique<v8::Extension>("B", "", 1, bDeps));
-  v8::ExtensionConfiguration config(1, bDeps);
-  v8::Context::New(isolate, &config);
-}
-
 bool IsSameOrigin(const GURL& l, const GURL& r) {
   return url::Origin::Create(l).IsSameOriginWith(url::Origin::Create(r));
 }
@@ -138,6 +128,16 @@ std::vector<v8::Local<v8::Value>> GetWeaklyTrackedValues(v8::Isolate* isolate) {
   }
   return locals;
 }
+
+// This causes a fatal error by creating a circular extension dependency.
+void TriggerFatalErrorForTesting(v8::Isolate* isolate) {
+  static const char* aDeps[] = {"B"};
+  v8::RegisterExtension(std::make_unique<v8::Extension>("A", "", 1, aDeps));
+  static const char* bDeps[] = {"A"};
+  v8::RegisterExtension(std::make_unique<v8::Extension>("B", "", 1, bDeps));
+  v8::ExtensionConfiguration config(1, bDeps);
+  v8::Context::New(isolate, &config);
+}
 #endif
 
 void Initialize(v8::Local<v8::Object> exports,
@@ -153,8 +153,8 @@ void Initialize(v8::Local<v8::Object> exports,
   dict.SetMethod("requestGarbageCollectionForTesting",
                  &RequestGarbageCollectionForTesting);
   dict.SetMethod("isSameOrigin", &IsSameOrigin);
-  dict.SetMethod("triggerFatalErrorForTesting", &TriggerFatalErrorForTesting);
 #ifdef DCHECK_IS_ON
+  dict.SetMethod("triggerFatalErrorForTesting", &TriggerFatalErrorForTesting);
   dict.SetMethod("getWeaklyTrackedValues", &GetWeaklyTrackedValues);
   dict.SetMethod("clearWeaklyTrackedValues", &ClearWeaklyTrackedValues);
   dict.SetMethod("weaklyTrackValue", &WeaklyTrackValue);

diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc
@@ -137,13 +137,16 @@ bool IsPackagedApp() {
 #endif
 }
 
-void FatalErrorCallback(const char* location, const char* message) {
+void V8FatalErrorCallback(const char* location, const char* message) {
   LOG(ERROR) << "Fatal error in V8: " << location << " " << message;
 
+#if !defined(MAS_BUILD)
   electron::crash_keys::SetCrashKey("electron.v8-fatal.message", message);
   electron::crash_keys::SetCrashKey("electron.v8-fatal.location", location);
+#endif
 
-  electron::ElectronBindings::Crash();
+  volatile int* zero = nullptr;
+  *zero = 0;
 }
 
 // Initialize Node.js cli options to pass to Node.js
@@ -413,7 +416,7 @@ node::Environment* NodeBindings::CreateEnvironment(
 
   // Use a custom fatal error callback to allow us to add
   // crash message and location to CrashReports.
-  is.fatal_error_callback = FatalErrorCallback;
+  is.fatal_error_callback = V8FatalErrorCallback;
 
   if (browser_env_ == BrowserEnvironment::BROWSER) {
     // Node.js requires that microtask checkpoints be explicitly invoked.