From 87a2b0ae6f7fb78ebb26b918eb0c2649eae3abe4 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Mon, 18 Mar 2024 21:55:00 -0400 Subject: [PATCH] Backed out changeset 102fa1186f2a (bug 1884265) for non-unified build bustage and test_getFromTypeAndExtension.js failures. --- netwerk/mime/nsIMIMEInfo.idl | 18 --- uriloader/exthandler/moz.build | 1 - uriloader/exthandler/nsLocalHandlerApp.cpp | 98 --------------- uriloader/exthandler/nsLocalHandlerApp.h | 10 -- .../unit/test_getFromTypeAndExtension.js | 50 -------- .../exthandler/win/nsLocalHandlerAppWin.cpp | 116 ------------------ .../exthandler/win/nsLocalHandlerAppWin.h | 34 ----- uriloader/exthandler/win/nsMIMEInfoWin.cpp | 20 ++- uriloader/exthandler/win/nsMIMEInfoWin.h | 2 +- 9 files changed, 9 insertions(+), 340 deletions(-) delete mode 100644 uriloader/exthandler/win/nsLocalHandlerAppWin.cpp delete mode 100644 uriloader/exthandler/win/nsLocalHandlerAppWin.h diff --git a/netwerk/mime/nsIMIMEInfo.idl b/netwerk/mime/nsIMIMEInfo.idl index 10cafb808b08..a7ffcfe513d7 100644 --- a/netwerk/mime/nsIMIMEInfo.idl +++ b/netwerk/mime/nsIMIMEInfo.idl @@ -280,24 +280,6 @@ interface nsILocalHandlerApp : nsIHandlerApp { */ readonly attribute unsigned long parameterCount; - /** - * Asynchronously returns the pretty (user friendly) name of the - * executable. - * - * On Linux and Mac, this is the same as the name - * property. On Mac, that happens to be a nicer name than - * the executable's name without the file extension. - * - * On Windows, this name will be nicer, looked up from the - * registry when it exists and falling back to the FileDescription - * getVersionFieldInfo when the registry data doesn't exist. - * This has the side effect that the prettyName returned - * generally will match the text returned by defaultDescription in - * nsIHandlerInfo. - */ - [implicit_jscontext] - Promise prettyNameAsync(); - /** * Clears the current list of command line parameters. */ diff --git a/uriloader/exthandler/moz.build b/uriloader/exthandler/moz.build index e9009eb063c2..cb96c690a69a 100644 --- a/uriloader/exthandler/moz.build +++ b/uriloader/exthandler/moz.build @@ -95,7 +95,6 @@ elif CONFIG["MOZ_WIDGET_TOOLKIT"] == "android": ] elif CONFIG["MOZ_WIDGET_TOOLKIT"] == "windows": UNIFIED_SOURCES += [ - "win/nsLocalHandlerAppWin.cpp", "win/nsMIMEInfoWin.cpp", ] diff --git a/uriloader/exthandler/nsLocalHandlerApp.cpp b/uriloader/exthandler/nsLocalHandlerApp.cpp index 9e4172836a21..6212d2aeb441 100644 --- a/uriloader/exthandler/nsLocalHandlerApp.cpp +++ b/uriloader/exthandler/nsLocalHandlerApp.cpp @@ -8,15 +8,11 @@ #include "nsIURI.h" #include "nsIProcess.h" #include "nsComponentManagerUtils.h" -#include "mozilla/dom/Promise.h" -#include "nsProxyRelease.h" // XXX why does nsMIMEInfoImpl have a threadsafe nsISupports? do we need one // here too? NS_IMPL_ISUPPORTS(nsLocalHandlerApp, nsILocalHandlerApp, nsIHandlerApp) -using namespace mozilla; - //////////////////////////////////////////////////////////////////////////////// //// nsIHandlerApp @@ -32,100 +28,6 @@ NS_IMETHODIMP nsLocalHandlerApp::GetName(nsAString& aName) { return NS_OK; } -/** - * This method returns a std::function that will be executed on a thread other - * than the main thread. To facilitate things, it should effectively be a global - * function that does not maintain a reference to the this pointer. There should - * be no reference to any objects that will be shared across threads. Sub-class - * implementations should make local copies of everything they need and capture - * those in the callback. - */ -std::function -nsLocalHandlerApp::GetPrettyNameOnNonMainThreadCallback() { - nsString name; - - // Calculate the name now, on the main thread, so as to avoid - // doing anything with the this pointer on the other thread - auto result = GetName(name); - - return [name, result](nsString& aName) { - aName = name; - return result; - }; -} - -NS_IMETHODIMP -nsLocalHandlerApp::PrettyNameAsync(JSContext* aCx, dom::Promise** aPromise) { - NS_ENSURE_ARG_POINTER(aPromise); - - *aPromise = nullptr; - - if (!mExecutable) { - return NS_ERROR_FAILURE; - } - - nsIGlobalObject* global = xpc::CurrentNativeGlobal(aCx); - if (NS_WARN_IF(!global)) { - return NS_ERROR_FAILURE; - } - - ErrorResult err; - RefPtr outer = dom::Promise::Create(global, err); - if (NS_WARN_IF(err.Failed())) { - return err.StealNSResult(); - } - - outer.forget(aPromise); - - nsAutoString executablePath; - nsresult result = mExecutable->GetPath(executablePath); - - if (NS_FAILED(result) || executablePath.IsEmpty()) { - (*aPromise)->MaybeReject(result); - return NS_OK; - } - - nsMainThreadPtrHandle promiseHolder( - new nsMainThreadPtrHolder( - "nsLocalHandlerApp::prettyExecutableName Promise", *aPromise)); - - auto prettyNameGetter = GetPrettyNameOnNonMainThreadCallback(); - - result = NS_DispatchBackgroundTask( - NS_NewRunnableFunction( - __func__, - [promiseHolder /* can't move this because if the dispatch fails, we - call reject on the promiseHolder */ - , - prettyNameGetter = std::move(prettyNameGetter)]() mutable -> void { - nsAutoString prettyExecutableName; - - nsresult result = prettyNameGetter(prettyExecutableName); - - DebugOnly rv = - NS_DispatchToMainThread(NS_NewRunnableFunction( - __func__, - [promiseHolder = std::move(promiseHolder), - prettyExecutableName = std::move(prettyExecutableName), - result]() { - if (NS_FAILED(result)) { - promiseHolder.get()->MaybeReject(result); - } else { - promiseHolder.get()->MaybeResolve(prettyExecutableName); - } - })); - NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), - "NS_DispatchToMainThread failed"); - }), - NS_DISPATCH_EVENT_MAY_BLOCK); - - if (NS_FAILED(result)) { - promiseHolder.get()->MaybeReject(result); - } - - return NS_OK; -} - NS_IMETHODIMP nsLocalHandlerApp::SetName(const nsAString& aName) { mName.Assign(aName); diff --git a/uriloader/exthandler/nsLocalHandlerApp.h b/uriloader/exthandler/nsLocalHandlerApp.h index ebe43f7dcb49..3ea8e3e4fca4 100644 --- a/uriloader/exthandler/nsLocalHandlerApp.h +++ b/uriloader/exthandler/nsLocalHandlerApp.h @@ -12,8 +12,6 @@ #include "nsIFile.h" #include "nsTArray.h" -#include - class nsLocalHandlerApp : public nsILocalHandlerApp { public: NS_DECL_ISUPPORTS @@ -31,9 +29,6 @@ class nsLocalHandlerApp : public nsILocalHandlerApp { protected: virtual ~nsLocalHandlerApp() {} - virtual std::function - GetPrettyNameOnNonMainThreadCallback(); - nsString mName; nsString mDetailedDescription; nsTArray mParameters; @@ -57,11 +52,6 @@ class nsLocalHandlerApp : public nsILocalHandlerApp { # include "mac/nsLocalHandlerAppMac.h" typedef nsLocalHandlerAppMac PlatformLocalHandlerApp_t; # endif -#elif XP_WIN -# ifndef NSLOCALHANDLERAPPWIN_H_ -# include "win/nsLocalHandlerAppWin.h" -typedef nsLocalHandlerAppWin PlatformLocalHandlerApp_t; -# endif #else typedef nsLocalHandlerApp PlatformLocalHandlerApp_t; #endif diff --git a/uriloader/exthandler/tests/unit/test_getFromTypeAndExtension.js b/uriloader/exthandler/tests/unit/test_getFromTypeAndExtension.js index 14d630cc4478..6f4fe52a49f6 100644 --- a/uriloader/exthandler/tests/unit/test_getFromTypeAndExtension.js +++ b/uriloader/exthandler/tests/unit/test_getFromTypeAndExtension.js @@ -15,53 +15,3 @@ add_task(async function test_utf8_extension() { Assert.equal(someMIME.primaryExtension, ".ั‚ะตัั‚"); } }); - -add_task(async function test_pretty_name_for_edge() { - if (AppConstants.platform == "win" && !AppConstants.IS_ESR) { - const mimeService = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService); - - let mimeInfo = mimeService.getFromTypeAndExtension("text/html", "html"); - let appList = mimeInfo?.possibleLocalHandlers || []; - - for (let index = 0; index < appList.length; index++) { - let app = appList.queryElementAt(index, Ci.nsILocalHandlerApp); - let executableName = app.executable.displayName; - let prettyName = await app.prettyNameAsync(); - - // Hardcode Edge, as an extra test, when it's installed - if (executableName == "msedge.exe") { - Assert.equal( - prettyName, - "Microsoft Edge", - "The generated pretty name for MS Edge should match the expectation." - ); - } - - // The pretty name should always be something nicer than the executable name. - // This isn't testing that's nice, but should be good enough to validate that - // something other than the executable is found. - Assert.notEqual(executableName, prettyName); - } - } -}); - -add_task(async function test_pretty_names_match_names_on_non_windows() { - if (AppConstants.platform != "win") { - const mimeService = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService); - - let mimeInfo = mimeService.getFromTypeAndExtension("text/html", "html"); - let appList = mimeInfo?.possibleLocalHandlers || []; - - for (let index = 0; index < appList.length; index++) { - let app = appList.queryElementAt(index, Ci.nsILocalHandlerApp); - let name = app.executable.name; - let prettyName = await app.prettyNameAsync(); - - Assert.equal( - prettyName, - name, - "On platforms other than windows, the prettyName and the name of file handlers should be the same." - ); - } - } -}); diff --git a/uriloader/exthandler/win/nsLocalHandlerAppWin.cpp b/uriloader/exthandler/win/nsLocalHandlerAppWin.cpp deleted file mode 100644 index d1fe75ef04bb..000000000000 --- a/uriloader/exthandler/win/nsLocalHandlerAppWin.cpp +++ /dev/null @@ -1,116 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "nsLocalHandlerAppWin.h" -#include "nsString.h" -#include "nsIWindowsRegKey.h" - -static nsresult GetPrettyNameFromFileDescription( - const nsCOMPtr& executableOnWindows, - const nsString& assignedName, nsString& aName) { - nsresult result = NS_ERROR_FAILURE; - - if (executableOnWindows) { - result = executableOnWindows->GetVersionInfoField("FileDescription", aName); - - if (NS_FAILED(result) || aName.IsEmpty()) { - if (!assignedName.IsEmpty()) { - aName = assignedName; - } else { - result = executableOnWindows->GetLeafName(aName); - } - - if (!aName.IsEmpty()) { - result = NS_OK; - } else { - result = NS_ERROR_FAILURE; - } - } - } - - return result; -} - -static nsresult GetValueFromRegistry(nsString& aName, - const nsCOMPtr& appKey, - const nsString& registryPath, - const nsString& valueName) { - nsresult rv = - appKey->Open(nsIWindowsRegKey::ROOT_KEY_CLASSES_ROOT, registryPath, - nsIWindowsRegKey::ACCESS_QUERY_VALUE); - - if (NS_SUCCEEDED(rv)) { - nsAutoString applicationName; - if (NS_SUCCEEDED(appKey->ReadStringValue(valueName, applicationName))) { - aName = applicationName; - return NS_OK; - } - } - - return NS_ERROR_FAILURE; -}; - -std::function -nsLocalHandlerAppWin::GetPrettyNameOnNonMainThreadCallback() { - // Make a copy of executable so that we don't have to worry about any other - // threads - nsCOMPtr executable; - mExecutable->Clone(getter_AddRefs(executable)); - - // Get the windows interface to the file - nsCOMPtr executableOnWindows(do_QueryInterface(executable)); - auto appIdOrName = mAppIdOrName; - auto assignedName = mName; - - std::function callback = - [assignedName, appIdOrName, - executableOnWindows = std::move(executableOnWindows)](nsString& aName) { - // On all platforms, we want a human readable name for an application. - // For example: msedge -> Microsoft Edge Browser - // - // This is generated on mac directly in nsLocalHandlerAppMac::GetName. - // The auto-test coverage for GetName isn't thorough enough to be - // confident that changing GetName on Windows won't cause problems. - // - // Besides that, this is a potentially slow thing to execute, and making - // it asynchronous is preferable. There's a fallback to GetName() in the - // nsLocalHandlerApp::PrettyNameAsync to cover Mac and Linux. - - if (appIdOrName.IsEmpty()) { - return GetPrettyNameFromFileDescription(executableOnWindows, - assignedName, aName); - } - - nsCOMPtr appKey = - do_CreateInstance("@mozilla.org/windows-registry-key;1"); - if (!appKey) { - return GetPrettyNameFromFileDescription(executableOnWindows, - assignedName, aName); - } - - // Check for ApplicationName first. Path: - // HKEY_CLASSES_ROOT\${APP_ID}\Application, Value entry: ApplicationName - nsresult rv = GetValueFromRegistry(aName, appKey, - appIdOrName + u"\\Application"_ns, - u"ApplicationName"_ns); - if (NS_SUCCEEDED(rv) && !aName.IsEmpty()) { - return rv; - } - - // Check for the default on the Applications entry next. - // Path: HKEY_CLASSES_ROOT\Applications\${APP_ID}, Value entry: "" - // (default) - rv = GetValueFromRegistry(aName, appKey, - u"Applications\\"_ns + appIdOrName, u""_ns); - if (NS_SUCCEEDED(rv) && !aName.IsEmpty()) { - return rv; - } - - // Fallthrough to getting the name from the file description - return GetPrettyNameFromFileDescription(executableOnWindows, - assignedName, aName); - }; - - return callback; -} diff --git a/uriloader/exthandler/win/nsLocalHandlerAppWin.h b/uriloader/exthandler/win/nsLocalHandlerAppWin.h deleted file mode 100644 index 68deda05f483..000000000000 --- a/uriloader/exthandler/win/nsLocalHandlerAppWin.h +++ /dev/null @@ -1,34 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef NSLOCALHANDLERAPPWIN_H_ -#define NSLOCALHANDLERAPPWIN_H_ - -#include "nsLocalHandlerApp.h" -#include "nsString.h" - -class nsLocalHandlerAppWin : public nsLocalHandlerApp { - public: - nsLocalHandlerAppWin() {} - - nsLocalHandlerAppWin(const char16_t* aName, nsIFile* aExecutable) - : nsLocalHandlerApp(aName, aExecutable) {} - - nsLocalHandlerAppWin(const nsAString& aName, nsIFile* aExecutable) - : nsLocalHandlerApp(aName, aExecutable) {} - virtual ~nsLocalHandlerAppWin() {} - - void SetAppIdOrName(const nsString& appIdOrName) { - mAppIdOrName = appIdOrName; - } - - protected: - std::function GetPrettyNameOnNonMainThreadCallback() - override; - - private: - nsString mAppIdOrName; -}; - -#endif /*NSLOCALHANDLERAPPMAC_H_*/ diff --git a/uriloader/exthandler/win/nsMIMEInfoWin.cpp b/uriloader/exthandler/win/nsMIMEInfoWin.cpp index 88f781bb4233..758b2018a7cd 100644 --- a/uriloader/exthandler/win/nsMIMEInfoWin.cpp +++ b/uriloader/exthandler/win/nsMIMEInfoWin.cpp @@ -554,7 +554,6 @@ bool nsMIMEInfoWin::GetProgIDVerbCommandHandler(const nsAString& appProgIDName, // entries to lower case and stores them in the trackList array. void nsMIMEInfoWin::ProcessPath(nsCOMPtr& appList, nsTArray& trackList, - const nsAutoString& appIdOrName, const nsAString& appFilesystemCommand) { nsAutoString lower(appFilesystemCommand); ToLowerCase(lower); @@ -570,9 +569,6 @@ void nsMIMEInfoWin::ProcessPath(nsCOMPtr& appList, nsCOMPtr aApp; if (!GetLocalHandlerApp(appFilesystemCommand, aApp)) return; - // Track the app id so that the pretty name can be determined later - (static_cast(aApp.get()))->SetAppIdOrName(appIdOrName); - // Save in our main tracking arrays appList->AppendElement(aApp); trackList.AppendElement(lower); @@ -677,7 +673,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { if (GetProgIDVerbCommandHandler(appProgId, appFilesystemCommand, false) && !IsPathInList(appFilesystemCommand, trackList)) { - ProcessPath(appList, trackList, appProgId, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } } @@ -705,7 +701,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appName, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } regKey->Close(); @@ -733,7 +729,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appProgId, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } regKey->Close(); @@ -766,7 +762,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appValue, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } } @@ -795,7 +791,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appProgId, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } regKey->Close(); @@ -833,7 +829,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appName, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } } @@ -861,7 +857,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { if (!GetAppsVerbCommandHandler(appName, appFilesystemCommand, false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appName, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } regKey->Close(); @@ -886,7 +882,7 @@ nsMIMEInfoWin::GetPossibleLocalHandlers(nsIArray** _retval) { if (!GetAppsVerbCommandHandler(appName, appFilesystemCommand, false) || IsPathInList(appFilesystemCommand, trackList)) continue; - ProcessPath(appList, trackList, appName, appFilesystemCommand); + ProcessPath(appList, trackList, appFilesystemCommand); } } } diff --git a/uriloader/exthandler/win/nsMIMEInfoWin.h b/uriloader/exthandler/win/nsMIMEInfoWin.h index 53d73e6ff368..fa972b23a908 100644 --- a/uriloader/exthandler/win/nsMIMEInfoWin.h +++ b/uriloader/exthandler/win/nsMIMEInfoWin.h @@ -68,7 +68,7 @@ class nsMIMEInfoWin : public nsMIMEInfoBase, public nsIPropertyBag { // Helper routine used in tracking app lists void ProcessPath(nsCOMPtr& appList, - nsTArray& trackList, const nsAutoString& appId, + nsTArray& trackList, const nsAString& appFilesystemCommand); // Helper routine to call mozilla::ShellExecuteByExplorer