fix: allow blocking IO for setAppLogsPath to avoid DCHECK (electron#23111)

loc · web-flow · commit cd0dda0125ee · 2020-04-21T12:28:01.000-07:00
* fix: let setAppLogsPath write to disk on UI thread

Otherwise, the DCHECK in thread_restrictions will fire.

* scope the io allowance more tightly

* oops, scope it tightly in the mac version too
diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc
@@ -851,13 +851,19 @@ void App::SetAppLogsPath(gin_helper::ErrorThrower thrower,
       thrower.ThrowError("Path must be absolute");
       return;
     }
-    base::PathService::Override(DIR_APP_LOGS, custom_path.value());
+    {
+      base::ThreadRestrictions::ScopedAllowIO allow_io;
+      base::PathService::Override(DIR_APP_LOGS, custom_path.value());
+    }
   } else {
     base::FilePath path;
     if (base::PathService::Get(DIR_USER_DATA, &path)) {
       path = path.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
       path = path.Append(base::FilePath::FromUTF8Unsafe("logs"));
-      base::PathService::Override(DIR_APP_LOGS, path);
+      {
+        base::ThreadRestrictions::ScopedAllowIO allow_io;
+        base::PathService::Override(DIR_APP_LOGS, path);
+      }
     }
   }
 }
@@ -874,7 +880,6 @@ base::FilePath App::GetPath(gin_helper::ErrorThrower thrower,
     // If users try to get the logs path before setting a logs path,
     // set the path to a sensible default and then try to get it again
     if (!succeed && name == "logs") {
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
       SetAppLogsPath(thrower, base::Optional<base::FilePath>());
       succeed = base::PathService::Get(key, &path);
     }
diff --git a/shell/browser/api/electron_api_app_mac.mm b/shell/browser/api/electron_api_app_mac.mm
@@ -21,16 +21,22 @@
       thrower.ThrowError("Path must be absolute");
       return;
     }
-    base::PathService::Override(DIR_APP_LOGS, custom_path.value());
+    {
+      base::ThreadRestrictions::ScopedAllowIO allow_io;
+      base::PathService::Override(DIR_APP_LOGS, custom_path.value());
+    }
   } else {
     NSString* bundle_name =
         [[[NSBundle mainBundle] infoDictionary] objectForKey:@"CFBundleName"];
     NSString* logs_path =
         [NSString stringWithFormat:@"Library/Logs/%@", bundle_name];
     NSString* library_path =
         [NSHomeDirectory() stringByAppendingPathComponent:logs_path];
-    base::PathService::Override(DIR_APP_LOGS,
-                                base::FilePath([library_path UTF8String]));
+    {
+      base::ThreadRestrictions::ScopedAllowIO allow_io;
+      base::PathService::Override(DIR_APP_LOGS,
+                                  base::FilePath([library_path UTF8String]));
+    }
   }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -851,13 +851,19 @@ void App::SetAppLogsPath(gin_helper::ErrorThrower thrower,`
`851`	`851`	`thrower.ThrowError("Path must be absolute");`
`852`	`852`	`return;`
`853`	`853`	`}`
`854`		`- base::PathService::Override(DIR_APP_LOGS, custom_path.value());`
	`854`	`+ {`
	`855`	`+ base::ThreadRestrictions::ScopedAllowIO allow_io;`
	`856`	`+ base::PathService::Override(DIR_APP_LOGS, custom_path.value());`
	`857`	`+ }`
`855`	`858`	`} else {`
`856`	`859`	`base::FilePath path;`
`857`	`860`	`if (base::PathService::Get(DIR_USER_DATA, &path)) {`
`858`	`861`	`path = path.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));`
`859`	`862`	`path = path.Append(base::FilePath::FromUTF8Unsafe("logs"));`
`860`		`- base::PathService::Override(DIR_APP_LOGS, path);`
	`863`	`+ {`
	`864`	`+ base::ThreadRestrictions::ScopedAllowIO allow_io;`
	`865`	`+ base::PathService::Override(DIR_APP_LOGS, path);`
	`866`	`+ }`
`861`	`867`	`}`
`862`	`868`	`}`
`863`	`869`	`}`
`@@ -874,7 +880,6 @@ base::FilePath App::GetPath(gin_helper::ErrorThrower thrower,`
`874`	`880`	`// If users try to get the logs path before setting a logs path,`
`875`	`881`	`// set the path to a sensible default and then try to get it again`
`876`	`882`	`if (!succeed && name == "logs") {`
`877`		`- base::ThreadRestrictions::ScopedAllowIO allow_io;`
`878`	`883`	`SetAppLogsPath(thrower, base::Optional<base::FilePath>());`
`879`	`884`	`succeed = base::PathService::Get(key, &path);`
`880`	`885`	`}`