Bug 1962880 - Get the download directory correctly for the profiler POSIX signal profiling a=pascalc
We were using `NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR, ...)` before for getting the download directory for the profiler POSIX signal profiling. But this ignores the user-set download directory and always use the default downloads folder. And while doing so, it creates this default folder even if the user set it to another one. Now we are using `GetPreferredDownloadsDirectory` directly which looks for the user-set download directory first and uses it if it find one. We had to change the location we call profiler_lookup_async_signal_dump_directory because in that location user-defined prefs weren't fully initialized yet in NS_InitXPCOM. Moved it inside `XREMain::XRE_mainRun`. Note that there is a small behavior change here. Previously that function was being called in *every process*, and now it's being called only in the parent process. This is actually the desired behavior because we can't capture profiles by directly sending POSIX signals to child processes due to sandboxing restrictions. Original Revision: https://phabricator.services.mozilla.com/D254342 Differential Revision: https://phabricator.services.mozilla.com/D256960
This commit is contained in:
committed by
pchevrel@mozilla.com
parent
b42b497a1f
commit
6a3ed07194
@@ -1237,6 +1237,11 @@ int XRE_XPCShellMain(int argc, char** argv, char** envp,
|
||||
return 1;
|
||||
}
|
||||
|
||||
// Now that the profiler, directory services, and prefs have been
|
||||
// initialized we can find the download directory, where the profiler can
|
||||
// write profiles when user stops the profiler using POSIX signal handling.
|
||||
profiler_lookup_async_signal_dump_directory();
|
||||
|
||||
// xpc::ErrorReport::LogToConsoleWithStack needs this to print errors
|
||||
// to stderr.
|
||||
Preferences::SetBool("browser.dom.window.dump.enabled", true);
|
||||
|
||||
@@ -5620,6 +5620,18 @@ nsresult XREMain::XRE_mainRun() {
|
||||
// files can't override JS engine start-up prefs.
|
||||
mDirProvider.FinishInitializingUserPrefs();
|
||||
|
||||
// Now that the profiler, directory services, and prefs have been
|
||||
// initialized we can find the download directory, where the profiler can
|
||||
// write profiles when user stops the profiler using POSIX signal handling.
|
||||
//
|
||||
// It's possible to start and stop the profiler by sending POSIX signals to
|
||||
// the profiler binary when Firefox is frozen. At the time when stop signal
|
||||
// is handled, Firefox stops the profiler and dumps the profile to disk. But
|
||||
// getting the download directory is not really possible at that time
|
||||
// because main thread will be unresponsive. That's why we are getting this
|
||||
// information right now.
|
||||
profiler_lookup_async_signal_dump_directory();
|
||||
|
||||
// Do a canary load of a JS based module here. This will help us detect
|
||||
// missing resources during startup and make us react appropriate, that
|
||||
// is inform the user before exiting with a crash.
|
||||
|
||||
@@ -49,6 +49,7 @@
|
||||
#include "mozilla/Maybe.h"
|
||||
#include "mozilla/MozPromise.h"
|
||||
#include "mozilla/Perfetto.h"
|
||||
#include "nsCExternalHandlerService.h"
|
||||
#include "nsCOMPtr.h"
|
||||
#include "nsDebug.h"
|
||||
#include "nsISupports.h"
|
||||
@@ -7006,9 +7007,9 @@ void profiler_lookup_async_signal_dump_directory() {
|
||||
#if !defined(XP_WIN)
|
||||
LOG("profiler_lookup_async_signal_dump_directory");
|
||||
|
||||
MOZ_ASSERT(
|
||||
NS_IsMainThread(),
|
||||
"We can only get access to the directory service from the main thread");
|
||||
MOZ_ASSERT(XRE_IsParentProcess() && NS_IsMainThread(),
|
||||
"We can only get access to the directory service from the parent "
|
||||
"process main thread");
|
||||
|
||||
// Make sure the profiler is actually running~
|
||||
MOZ_RELEASE_ASSERT(CorePS::Exists());
|
||||
@@ -7037,17 +7038,34 @@ void profiler_lookup_async_signal_dump_directory() {
|
||||
|
||||
CorePS::SetAsyncSignalDumpDirectory(lock, Some(mozUploadDirFile));
|
||||
} else {
|
||||
# if defined(GP_OS_android)
|
||||
// Bug 1904639: GetPreferredDownloadsDirectory is not implemented for
|
||||
// Android.
|
||||
return;
|
||||
# endif
|
||||
|
||||
LOG("Defaulting to the user's Download directory for profile dumps");
|
||||
nsCOMPtr<nsIFile> tDownloadDir;
|
||||
rv = NS_GetSpecialDirectory(NS_OS_DEFAULT_DOWNLOAD_DIR,
|
||||
getter_AddRefs(tDownloadDir));
|
||||
nsresult rv;
|
||||
nsCOMPtr<nsIExternalHelperAppService> svc =
|
||||
do_GetService(NS_EXTERNALHELPERAPPSERVICE_CONTRACTID, &rv);
|
||||
|
||||
if (NS_FAILED(rv)) {
|
||||
LOG("Failed to get nsIExternalHelperAppService for the download "
|
||||
"directory. Profiler signal handling will not be able to save to "
|
||||
"disk. Error: %s",
|
||||
GetStaticErrorName(rv));
|
||||
return;
|
||||
}
|
||||
|
||||
rv = svc->GetPreferredDownloadsDirectory(getter_AddRefs(tDownloadDir));
|
||||
if (NS_FAILED(rv)) {
|
||||
LOG("Failed to find download directory. Profiler signal handling will "
|
||||
"not be able to save to disk. Error: %s",
|
||||
GetStaticErrorName(rv));
|
||||
} else {
|
||||
CorePS::SetAsyncSignalDumpDirectory(lock, Some(tDownloadDir));
|
||||
return;
|
||||
}
|
||||
CorePS::SetAsyncSignalDumpDirectory(lock, Some(tDownloadDir));
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -480,11 +480,6 @@ NS_InitXPCOM(nsIServiceManager** aResult, nsIFile* aBinDirectory,
|
||||
// to the directory service.
|
||||
nsDirectoryService::gService->RegisterCategoryProviders();
|
||||
|
||||
// Now that both the profiler and directory services have been started
|
||||
// we can find the download directory, where the profiler can write
|
||||
// profiles if necessary
|
||||
profiler_lookup_async_signal_dump_directory();
|
||||
|
||||
// Init mozilla::SharedThreadPool (which needs the service manager).
|
||||
mozilla::SharedThreadPool::InitStatics();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user