From bf0c416c45448f9d264fe81cf0fcb89f95ae040d Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Fri, 30 Aug 2024 22:40:25 +0000 Subject: [PATCH] Bug 1915998 - Check for Omnijar::Init failure more consistently, r=mccr8 Differential Revision: https://phabricator.services.mozilla.com/D220748 --- toolkit/xre/nsAppRunner.cpp | 1 + xpcom/build/Omnijar.cpp | 14 +++++++++++++- xpcom/build/Omnijar.h | 9 +++++++-- xpcom/build/XPCOMInit.cpp | 8 ++++---- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index ed81a09447ef..7188ae9de151 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -6168,6 +6168,7 @@ nsresult XRE_InitCommandLine(int aArgc, char* aArgv[]) { return NS_ERROR_FAILURE; } mozilla::Omnijar::Init(greOmni, greOmni); + MOZ_RELEASE_ASSERT(IsPackagedBuild(), "Android builds are always packaged"); } #endif diff --git a/xpcom/build/Omnijar.cpp b/xpcom/build/Omnijar.cpp index a5d117df63fd..2f8fd1caa425 100644 --- a/xpcom/build/Omnijar.cpp +++ b/xpcom/build/Omnijar.cpp @@ -104,7 +104,7 @@ nsresult Omnijar::InitOne(nsIFile* aPath, Type aType) { return NS_OK; } -nsresult Omnijar::Init(nsIFile* aGrePath, nsIFile* aAppPath) { +nsresult Omnijar::FallibleInit(nsIFile* aGrePath, nsIFile* aAppPath) { // Even on error we do not want to come here again. sInitialized = true; @@ -118,6 +118,14 @@ nsresult Omnijar::Init(nsIFile* aGrePath, nsIFile* aAppPath) { return NS_OK; } +void Omnijar::Init(nsIFile* aGrePath, nsIFile* aAppPath) { + nsresult rv = FallibleInit(aGrePath, aAppPath); + if (NS_FAILED(rv)) { + MOZ_CRASH_UNSAFE_PRINTF("Omnijar::Init failed: %s", + mozilla::GetStaticErrorName(rv)); + } +} + void Omnijar::CleanUp() { CleanUpOne(GRE); CleanUpOne(APP); @@ -226,6 +234,10 @@ void Omnijar::ChildProcessInit(int& aArgc, char** aArgv) { } } +#if defined(MOZ_WIDGET_ANDROID) + MOZ_RELEASE_ASSERT(greOmni, "Android builds are always packaged"); +#endif + // If we're unified, then only the -greomni flag is present // (reflecting the state of sPath in the parent process) but that // path should be used for both (not nullptr, which will try to diff --git a/xpcom/build/Omnijar.h b/xpcom/build/Omnijar.h index f49f0931db66..0df7d418f10c 100644 --- a/xpcom/build/Omnijar.h +++ b/xpcom/build/Omnijar.h @@ -85,8 +85,13 @@ class Omnijar { * - a directory path, pointing to a directory containing an "omni.jar" file, * - nullptr for autodetection of an "omni.jar" file. */ - static nsresult Init(nsIFile* aGrePath = nullptr, - nsIFile* aAppPath = nullptr); + static void Init(nsIFile* aGrePath = nullptr, nsIFile* aAppPath = nullptr); + + /** + * Like `Init`, but returns a failed nsresult instead of crashing on failure. + */ + static nsresult FallibleInit(nsIFile* aGrePath = nullptr, + nsIFile* aAppPath = nullptr); /** * Initializes the Omnijar API for a child process, given its argument diff --git a/xpcom/build/XPCOMInit.cpp b/xpcom/build/XPCOMInit.cpp index 3a0468d81d16..6e6df43bab5e 100644 --- a/xpcom/build/XPCOMInit.cpp +++ b/xpcom/build/XPCOMInit.cpp @@ -391,10 +391,10 @@ NS_InitXPCOM(nsIServiceManager** aResult, nsIFile* aBinDirectory, // GeckoChildProcessHost.cpp which sets the greomni/appomni flags. MOZ_ASSERT(XRE_IsParentProcess() || XRE_IsContentProcess()); - // Note that the Omnijar::Init does not fail but returns NS_OK if the file - // is not found at all, as this is an expected possible way of running - // with an unpacked modules directory. - nsresult rv = mozilla::Omnijar::Init(); + // Note that the Omnijar::FallibleInit does not fail but returns NS_OK if + // the file is not found at all, as this is an expected possible way of + // running with an unpacked modules directory. + nsresult rv = mozilla::Omnijar::FallibleInit(); if (NS_FAILED(rv)) { XPCOM_INIT_FATAL("Omnijar::Init()", NS_ERROR_OMNIJAR_CORRUPT) }