refactor: use PathProvider for user-data-dir and others (#29649)

* refactor: use PathProvider for user-data-dir and others

* consolidate logic for DIR_RECENT and DIR_APP_LOGS into path provider

* fix bad include

* remove debugging code

* fix build on mac

* fix build on win

* create app logs dir on both mac and non-mac

default_app/main.ts

@@ -109,7 +109,7 @@ function loadApplicationPackage (packagePath: string) {
 
     try {
       const filePath = Module._resolveFilename(packagePath, module, true);
-      app._setDefaultAppPaths(appPath || path.dirname(filePath));
+      app.setAppPath(appPath || path.dirname(filePath));
     } catch (e) {
       showErrorMessage(`Unable to find Electron app at ${packagePath}\n\n${e.message}`);
       return;

lib/browser/api/app.ts

@@ -1,5 +1,4 @@
 import * as fs from 'fs';
-import * as path from 'path';
 
 import { Menu } from 'electron/main';
 
@@ -58,19 +57,6 @@ Object.defineProperty(app, 'applicationMenu', {
 // The native implementation is not provided on non-windows platforms
 app.setAppUserModelId = app.setAppUserModelId || (() => {});
 
-app._setDefaultAppPaths = (packagePath) => {
-  // Set the user path according to application's name.
-  app.setPath('userData', path.join(app.getPath('appData'), app.name!));
-  app.setPath('userCache', path.join(app.getPath('cache'), app.name!));
-  app.setAppPath(packagePath);
-
-  // Add support for --user-data-dir=
-  if (app.commandLine.hasSwitch('user-data-dir')) {
-    const userDataDir = app.commandLine.getSwitchValue('user-data-dir');
-    if (path.isAbsolute(userDataDir)) app.setPath('userData', userDataDir);
-  }
-};
-
 if (process.platform === 'darwin') {
   const setDockMenu = app.dock!.setMenu;
   app.dock!.setMenu = (menu) => {

lib/browser/init.ts

@@ -129,7 +129,7 @@ if (packageJson.v8Flags != null) {
   require('v8').setFlagsFromString(packageJson.v8Flags);
 }
 
-app._setDefaultAppPaths(packagePath);
+app.setAppPath(packagePath);
 
 // Load the chrome devtools support.
 require('@electron/internal/browser/devtools');

shell/app/electron_crash_reporter_client.cc

@@ -165,7 +165,7 @@ bool ElectronCrashReporterClient::GetCrashDumpLocation(
 #if defined(OS_MAC) || defined(OS_LINUX)
 bool ElectronCrashReporterClient::GetCrashMetricsLocation(
     base::FilePath* metrics_dir) {
-  return base::PathService::Get(electron::DIR_USER_DATA, metrics_dir);
+  return base::PathService::Get(chrome::DIR_USER_DATA, metrics_dir);
 }
 #endif  // OS_MAC || OS_LINUX
 

shell/app/electron_main_delegate.cc

@@ -8,8 +8,8 @@
 #include <memory>
 #include <string>
 
+#include "base/base_switches.h"
 #include "base/command_line.h"
-#include "base/debug/stack_trace.h"
 #include "base/environment.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
@@ -17,6 +17,7 @@
 #include "base/path_service.h"
 #include "base/strings/string_split.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
 #include "content/public/common/content_switches.h"
 #include "electron/buildflags/buildflags.h"
@@ -29,8 +30,10 @@
 #include "shell/browser/electron_gpu_client.h"
 #include "shell/browser/feature_list.h"
 #include "shell/browser/relauncher.h"
+#include "shell/common/application_info.h"
 #include "shell/common/electron_paths.h"
 #include "shell/common/options_switches.h"
+#include "shell/common/platform_util.h"
 #include "shell/renderer/electron_renderer_client.h"
 #include "shell/renderer/electron_sandboxed_renderer_client.h"
 #include "shell/utility/electron_content_utility_client.h"
@@ -47,6 +50,7 @@
 #endif
 
 #if defined(OS_LINUX)
+#include "base/nix/xdg_util.h"
 #include "components/crash/core/app/breakpad_linux.h"
 #include "v8/include/v8-wasm-trap-handler-posix.h"
 #include "v8/include/v8.h"
@@ -107,28 +111,89 @@ void InvalidParameterHandler(const wchar_t*,
 
 // TODO(nornagon): move path provider overriding to its own file in
 // shell/common
-bool GetDefaultCrashDumpsPath(base::FilePath* path) {
+bool ElectronPathProvider(int key, base::FilePath* result) {
+  bool create_dir = false;
   base::FilePath cur;
-  if (!base::PathService::Get(DIR_USER_DATA, &cur))
+  switch (key) {
-    return false;
+    case chrome::DIR_USER_DATA:
+      if (!base::PathService::Get(DIR_APP_DATA, &cur))
+        return false;
+      cur = cur.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
+      create_dir = true;
+      break;
+    case DIR_CRASH_DUMPS:
+      if (!base::PathService::Get(chrome::DIR_USER_DATA, &cur))
+        return false;
 #if defined(OS_MAC) || defined(OS_WIN)
-  cur = cur.Append(FILE_PATH_LITERAL("Crashpad"));
+      cur = cur.Append(FILE_PATH_LITERAL("Crashpad"));
 #else
-  cur = cur.Append(FILE_PATH_LITERAL("Crash Reports"));
+      cur = cur.Append(FILE_PATH_LITERAL("Crash Reports"));
 #endif
+      create_dir = true;
+      break;
+    case chrome::DIR_APP_DICTIONARIES:
+      // TODO(nornagon): can we just default to using Chrome's logic here?
+      if (!base::PathService::Get(chrome::DIR_USER_DATA, &cur))
+        return false;
+      cur = cur.Append(base::FilePath::FromUTF8Unsafe("Dictionaries"));
+      create_dir = true;
+      break;
+    case DIR_USER_CACHE: {
+#if defined(OS_POSIX)
+      int parent_key = base::DIR_CACHE;
+#else
+      // On Windows, there's no OS-level centralized location for caches, so
+      // store the cache in the app data directory.
+      int parent_key = base::DIR_APP_DATA;
+#endif
+      if (!base::PathService::Get(parent_key, &cur))
+        return false;
+      cur = cur.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
+      create_dir = true;
+      break;
+    }
+#if defined(OS_LINUX)
+    case DIR_APP_DATA: {
+      auto env = base::Environment::Create();
+      cur = base::nix::GetXDGDirectory(
+          env.get(), base::nix::kXdgConfigHomeEnvVar, base::nix::kDotConfigDir);
+      break;
+    }
+#endif
+#if defined(OS_WIN)
+    case DIR_RECENT:
+      if (!platform_util::GetFolderPath(DIR_RECENT, &cur))
+        return false;
+      create_dir = true;
+      break;
+#endif
+    case DIR_APP_LOGS:
+#if defined(OS_MAC)
+      if (!base::PathService::Get(base::DIR_HOME, &cur))
+        return false;
+      cur = cur.Append(FILE_PATH_LITERAL("Library"));
+      cur = cur.Append(FILE_PATH_LITERAL("Logs"));
+      cur = cur.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
+#else
+      if (!base::PathService::Get(chrome::DIR_USER_DATA, &cur))
+        return false;
+      cur = cur.Append(base::FilePath::FromUTF8Unsafe("logs"));
+#endif
+      create_dir = true;
+      break;
+    default:
+      return false;
+  }
+
   // TODO(bauerb): http://crbug.com/259796
   base::ThreadRestrictions::ScopedAllowIO allow_io;
-  if (!base::PathExists(cur) && !base::CreateDirectory(cur))
+  if (create_dir && !base::PathExists(cur) && !base::CreateDirectory(cur)) {
     return false;
-  *path = cur;
-  return true;
-}
-
-bool ElectronPathProvider(int key, base::FilePath* path) {
-  if (key == DIR_CRASH_DUMPS) {
-    return GetDefaultCrashDumpsPath(path);
   }
-  return false;
+
+  *result = cur;
+
+  return true;
 }
 
 void RegisterPathProvider() {
@@ -281,6 +346,13 @@ void ElectronMainDelegate::PreSandboxStartup() {
   std::string process_type =
       command_line->GetSwitchValueASCII(::switches::kProcessType);
 
+  base::FilePath user_data_dir =
+      command_line->GetSwitchValuePath(::switches::kUserDataDir);
+  if (!user_data_dir.empty()) {
+    base::PathService::OverrideAndCreateIfNeeded(chrome::DIR_USER_DATA,
+                                                 user_data_dir, false, true);
+  }
+
 #if !defined(MAS_BUILD)
   crash_reporter::InitializeCrashKeys();
 #endif

shell/browser/api/electron_api_app.cc

@@ -441,9 +441,13 @@ int GetPathConstant(const std::string& name) {
   if (name == "appData")
     return DIR_APP_DATA;
   else if (name == "userData")
-    return DIR_USER_DATA;
+    return chrome::DIR_USER_DATA;
   else if (name == "cache")
-    return DIR_CACHE;
+#if defined(OS_POSIX)
+    return base::DIR_CACHE;
+#else
+    return base::DIR_APP_DATA;
+#endif
   else if (name == "userCache")
     return DIR_USER_CACHE;
   else if (name == "logs")
@@ -930,8 +934,7 @@ void App::SetAppLogsPath(gin_helper::ErrorThrower thrower,
     }
   } else {
     base::FilePath path;
-    if (base::PathService::Get(DIR_USER_DATA, &path)) {
+    if (base::PathService::Get(chrome::DIR_USER_DATA, &path)) {
-      path = path.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
       path = path.Append(base::FilePath::FromUTF8Unsafe("logs"));
       {
         base::ThreadRestrictions::ScopedAllowIO allow_io;
@@ -962,30 +965,10 @@ bool App::IsPackaged() {
 
 base::FilePath App::GetPath(gin_helper::ErrorThrower thrower,
                             const std::string& name) {
-  bool succeed = false;
   base::FilePath path;
 
   int key = GetPathConstant(name);
-  if (key >= 0) {
+  if (key < 0 || !base::PathService::Get(key, &path))
-    succeed = base::PathService::Get(key, &path);
-    // 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") {
-      SetAppLogsPath(thrower, absl::optional<base::FilePath>());
-      succeed = base::PathService::Get(key, &path);
-    }
-
-#if defined(OS_WIN)
-    // If we get the "recent" path before setting it, set it
-    if (!succeed && name == "recent" &&
-        platform_util::GetFolderPath(DIR_RECENT, &path)) {
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
-      succeed = base::PathService::Override(DIR_RECENT, path);
-    }
-#endif
-  }
-
-  if (!succeed)
     thrower.ThrowError("Failed to get '" + name + "' path");
 
   return path;
@@ -999,20 +982,9 @@ void App::SetPath(gin_helper::ErrorThrower thrower,
     return;
   }
 
-  bool succeed = false;
   int key = GetPathConstant(name);
-  if (key >= 0) {
+  if (key < 0 || !base::PathService::OverrideAndCreateIfNeeded(
-    succeed =
+                     key, path, /* is_absolute = */ true, /* create = */ false))
-        base::PathService::OverrideAndCreateIfNeeded(key, path, true, false);
-    if (key == DIR_USER_DATA) {
-      succeed |= base::PathService::OverrideAndCreateIfNeeded(
-          chrome::DIR_USER_DATA, path, true, false);
-      succeed |= base::PathService::Override(
-          chrome::DIR_APP_DICTIONARIES,
-          path.Append(base::FilePath::FromUTF8Unsafe("Dictionaries")));
-    }
-  }
-  if (!succeed)
     thrower.ThrowError("Failed to set path");
 }
 
@@ -1082,7 +1054,7 @@ bool App::RequestSingleInstanceLock() {
     return true;
 
   base::FilePath user_dir;
-  base::PathService::Get(DIR_USER_DATA, &user_dir);
+  base::PathService::Get(chrome::DIR_USER_DATA, &user_dir);
 
   auto cb = base::BindRepeating(&App::OnSecondInstance, base::Unretained(this));
 

shell/browser/api/electron_api_crash_reporter.cc

@@ -172,7 +172,7 @@ void Start(const std::string& submit_url,
   for (const auto& pair : extra)
     electron::crash_keys::SetCrashKey(pair.first, pair.second);
   base::FilePath user_data_dir;
-  base::PathService::Get(DIR_USER_DATA, &user_data_dir);
+  base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   ::crash_reporter::InitializeCrashpadWithEmbeddedHandler(
       process_type.empty(), process_type,
       base::WideToUTF8(user_data_dir.value()), base::FilePath());

shell/browser/browser.cc

@@ -14,6 +14,7 @@
 #include "base/run_loop.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "chrome/common/chrome_paths.h"
 #include "shell/browser/browser_observer.h"
 #include "shell/browser/electron_browser_main_parts.h"
 #include "shell/browser/login_handler.h"
@@ -187,7 +188,7 @@ void Browser::DidFinishLaunching(base::DictionaryValue launch_info) {
   // Make sure the userData directory is created.
   base::ThreadRestrictions::ScopedAllowIO allow_io;
   base::FilePath user_data;
-  if (base::PathService::Get(DIR_USER_DATA, &user_data))
+  if (base::PathService::Get(chrome::DIR_USER_DATA, &user_data))
     base::CreateDirectoryAndGetError(user_data, nullptr);
 
   is_ready_ = true;

shell/browser/browser_process_impl.cc

@@ -11,6 +11,7 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/path_service.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/os_crypt/os_crypt.h"
 #include "components/prefs/in_memory_pref_store.h"
@@ -104,7 +105,7 @@ void BrowserProcessImpl::PostEarlyInitialization() {
   // is the only key that needs it
   if (electron::fuses::IsCookieEncryptionEnabled()) {
     base::FilePath prefs_path;
-    CHECK(base::PathService::Get(electron::DIR_USER_DATA, &prefs_path));
+    CHECK(base::PathService::Get(chrome::DIR_USER_DATA, &prefs_path));
     prefs_path = prefs_path.Append(FILE_PATH_LITERAL("Local State"));
     base::ThreadRestrictions::ScopedAllowIO allow_io;
     scoped_refptr<JsonPrefStore> user_pref_store =

shell/browser/electron_browser_client.cc

@@ -1023,7 +1023,7 @@ void ElectronBrowserClient::OnNetworkServiceCreated(
 std::vector<base::FilePath>
 ElectronBrowserClient::GetNetworkContextsParentDirectory() {
   base::FilePath user_data_dir;
-  base::PathService::Get(DIR_USER_DATA, &user_data_dir);
+  base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   DCHECK(!user_data_dir.empty());
 
   return {user_data_dir};
@@ -1416,7 +1416,7 @@ std::string ElectronBrowserClient::GetApplicationLocale() {
 
 base::FilePath ElectronBrowserClient::GetFontLookupTableCacheDir() {
   base::FilePath user_data_dir;
-  base::PathService::Get(DIR_USER_DATA, &user_data_dir);
+  base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
   DCHECK(!user_data_dir.empty());
   return user_data_dir.Append(FILE_PATH_LITERAL("FontLookupTableCache"));
 }

shell/browser/electron_browser_context.cc

@@ -117,14 +117,10 @@ ElectronBrowserContext::ElectronBrowserContext(const std::string& partition,
   base::StringToInt(command_line->GetSwitchValueASCII(switches::kDiskCacheSize),
                     &max_cache_size_);
 
-  if (!base::PathService::Get(DIR_USER_DATA, &path_)) {
+  if (!base::PathService::Get(chrome::DIR_USER_DATA, &path_)) {
     base::PathService::Get(DIR_APP_DATA, &path_);
     path_ = path_.Append(base::FilePath::FromUTF8Unsafe(GetApplicationName()));
-    base::PathService::Override(DIR_USER_DATA, path_);
     base::PathService::Override(chrome::DIR_USER_DATA, path_);
-    base::PathService::Override(
-        chrome::DIR_APP_DICTIONARIES,
-        path_.Append(base::FilePath::FromUTF8Unsafe("Dictionaries")));
   }
 
   if (!in_memory && !partition.empty())

shell/browser/electron_browser_main_parts.cc

@@ -61,7 +61,6 @@
 
 #if defined(OS_LINUX)
 #include "base/environment.h"
-#include "base/nix/xdg_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gtk/gtk_ui_factory.h"
 #include "ui/gtk/gtk_util.h"
@@ -155,16 +154,6 @@ std::u16string MediaStringProvider(media::MessageId id) {
 }
 
 #if defined(OS_LINUX)
-void OverrideLinuxAppDataPath() {
-  base::FilePath path;
-  if (base::PathService::Get(DIR_APP_DATA, &path))
-    return;
-  auto env = base::Environment::Create();
-  path = base::nix::GetXDGDirectory(env.get(), base::nix::kXdgConfigHomeEnvVar,
-                                    base::nix::kDotConfigDir);
-  base::PathService::Override(DIR_APP_DATA, path);
-}
-
 // GTK does not provide a way to check if current theme is dark, so we compare
 // the text and background luminosity to get a result.
 // This trick comes from FireFox.
@@ -232,10 +221,6 @@ int ElectronBrowserMainParts::GetExitCode() const {
 
 int ElectronBrowserMainParts::PreEarlyInitialization() {
   field_trial_list_ = std::make_unique<base::FieldTrialList>(nullptr);
-#if defined(OS_LINUX)
-  OverrideLinuxAppDataPath();
-#endif
-
 #if defined(OS_POSIX)
   HandleSIGCHLD();
 #endif

shell/browser/net/system_network_context_manager.cc

@@ -12,6 +12,7 @@
 #include "base/path_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/chrome_mojo_proxy_resolver_factory.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/os_crypt/os_crypt.h"
 #include "content/public/browser/browser_thread.h"
@@ -257,7 +258,7 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
           command_line.GetSwitchValueASCII(::switches::kPasswordStore);
       config->should_use_preference =
           command_line.HasSwitch(::switches::kEnableEncryptionSelection);
-      base::PathService::Get(electron::DIR_USER_DATA, &config->user_data_path);
+      base::PathService::Get(chrome::DIR_USER_DATA, &config->user_data_path);
       network_service->SetCryptConfig(std::move(config));
 #else
       network_service->SetEncryptionKey(OSCrypt::GetRawEncryptionKey());

shell/browser/ui/devtools_manager_delegate.cc

@@ -14,6 +14,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/common/chrome_paths.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_frontend_host.h"
 #include "content/public/browser/devtools_socket_factory.h"
@@ -89,7 +90,7 @@ const char kBrowserCloseMethod[] = "Browser.close";
 // static
 void DevToolsManagerDelegate::StartHttpHandler() {
   base::FilePath user_dir;
-  base::PathService::Get(DIR_USER_DATA, &user_dir);
+  base::PathService::Get(chrome::DIR_USER_DATA, &user_dir);
   content::DevToolsAgentHost::StartRemoteDebuggingServer(
       CreateSocketFactory(), user_dir, base::FilePath());
 }

shell/common/electron_paths.h

@@ -22,9 +22,8 @@ namespace electron {
 enum {
   PATH_START = 11000,
 
-  DIR_USER_DATA = PATH_START,  // Directory where user data can be written.
+  DIR_USER_CACHE = PATH_START,  // Directory where user cache can be written.
-  DIR_USER_CACHE,              // Directory where user cache can be written.
+  DIR_APP_LOGS,                 // Directory where app logs live
-  DIR_APP_LOGS,                // Directory where app logs live
 
 #if defined(OS_WIN)
   DIR_RECENT,  // Directory where recent files live
@@ -41,12 +40,6 @@ enum {
 #if !defined(OS_LINUX)
   DIR_APP_DATA = base::DIR_APP_DATA,
 #endif
-
-#if defined(OS_POSIX)
-  DIR_CACHE = base::DIR_CACHE  // Directory where to put cache data.
-#else
-  DIR_CACHE = base::DIR_APP_DATA
-#endif
 };
 
 static_assert(PATH_START < PATH_END, "invalid PATH boundaries");

typings/internal-electron.d.ts

@@ -13,7 +13,6 @@ declare namespace Electron {
   }
 
   interface App {
-    _setDefaultAppPaths(packagePath: string | null): void;
     setVersion(version: string): void;
     setDesktopName(name: string): void;
     setAppPath(path: string | null): void;