Bug 1826872 Part 2 - Move CUPS printer dest into CUPSPrinterInfo for nsPrinterCUPS r=dholbert

This puts all CUPS data into one structure, refactors the locking of this
data, and and simplifies logic for determining the connection to use in
TryEnsurePrinterInfo.

Differential Revision: https://phabricator.services.mozilla.com/D174925
This commit is contained in:
Emily McDonough 2023-04-10 21:11:28 +00:00
parent 60b538473f
commit 02b2360a11
2 changed files with 75 additions and 47 deletions

View file

@ -87,9 +87,8 @@ nsPrinterCUPS::~nsPrinterCUPS() {
if (lock->mPrinterInfo) {
mShim.cupsFreeDestInfo(lock->mPrinterInfo);
}
if (mPrinter) {
mShim.cupsFreeDests(1, mPrinter);
mPrinter = nullptr;
if (lock->mPrinter) {
mShim.cupsFreeDests(1, lock->mPrinter);
}
}
@ -101,14 +100,16 @@ nsPrinterCUPS::GetName(nsAString& aName) {
NS_IMETHODIMP
nsPrinterCUPS::GetSystemName(nsAString& aName) {
CopyUTF8toUTF16(MakeStringSpan(mPrinter->name), aName);
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
CopyUTF8toUTF16(MakeStringSpan(lock->mPrinter->name), aName);
return NS_OK;
}
void nsPrinterCUPS::GetPrinterName(nsAString& aName) const {
if (mDisplayName.IsEmpty()) {
aName.Truncate();
CopyUTF8toUTF16(MakeStringSpan(mPrinter->name), aName);
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
CopyUTF8toUTF16(MakeStringSpan(lock->mPrinter->name), aName);
} else {
aName = mDisplayName;
}
@ -122,7 +123,8 @@ const char* nsPrinterCUPS::LocalizeMediaName(http_t& aConnection,
return aMedia.media;
}
PrinterInfoLock lock = TryEnsurePrinterInfo();
return mShim.cupsLocalizeDestMedia(&aConnection, mPrinter, lock->mPrinterInfo,
return mShim.cupsLocalizeDestMedia(&aConnection, lock->mPrinter,
lock->mPrinterInfo,
CUPS_MEDIA_FLAGS_DEFAULT, &aMedia);
}
@ -151,7 +153,8 @@ bool nsPrinterCUPS::SupportsColor() const {
bool nsPrinterCUPS::SupportsCollation() const {
// We can't depend on cupsGetIntegerOption existing.
const char* const value = FindCUPSOption("printer-type");
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
const char* const value = FindCUPSOption(lock, "printer-type");
if (!value) {
return false;
}
@ -168,7 +171,7 @@ nsPrinterBase::PrinterInfo nsPrinterCUPS::CreatePrinterInfo() const {
bool nsPrinterCUPS::Supports(const char* aOption, const char* aValue) const {
PrinterInfoLock lock = TryEnsurePrinterInfo();
return mShim.cupsCheckDestSupported(CUPS_HTTP_DEFAULT, mPrinter,
return mShim.cupsCheckDestSupported(CUPS_HTTP_DEFAULT, lock->mPrinter,
lock->mPrinterInfo, aOption, aValue);
}
@ -235,13 +238,13 @@ PrintSettingsInitializer nsPrinterCUPS::DefaultSettings(
// and the IPP attribute appears to return more accurate defaults on Linux.
#ifdef XP_MACOSX
hasDefaultMedia = mShim.cupsGetDestMediaDefault(
CUPS_HTTP_DEFAULT, mPrinter, lock->mPrinterInfo, CUPS_MEDIA_FLAGS_DEFAULT,
&media);
CUPS_HTTP_DEFAULT, lock->mPrinter, lock->mPrinterInfo,
CUPS_MEDIA_FLAGS_DEFAULT, &media);
#else
{
ipp_attribute_t* defaultMediaIPP =
mShim.cupsFindDestDefault
? mShim.cupsFindDestDefault(CUPS_HTTP_DEFAULT, mPrinter,
? mShim.cupsFindDestDefault(CUPS_HTTP_DEFAULT, lock->mPrinter,
lock->mPrinterInfo, "media")
: nullptr;
@ -251,7 +254,7 @@ PrintSettingsInitializer nsPrinterCUPS::DefaultSettings(
hasDefaultMedia = defaultMediaName &&
mShim.cupsGetDestMediaByName(
CUPS_HTTP_DEFAULT, mPrinter, lock->mPrinterInfo,
CUPS_HTTP_DEFAULT, lock->mPrinter, lock->mPrinterInfo,
defaultMediaName, CUPS_MEDIA_FLAGS_DEFAULT, &media);
}
#endif
@ -277,7 +280,7 @@ PrintSettingsInitializer nsPrinterCUPS::DefaultSettings(
};
}
http_t* const connection = aConnection.GetConnection(mPrinter);
http_t* const connection = aConnection.GetConnection(lock->mPrinter);
// XXX Do we actually have the guarantee that this is utf-8?
NS_ConvertUTF8toUTF16 localizedName{
connection ? LocalizeMediaName(*connection, media) : ""};
@ -291,8 +294,9 @@ PrintSettingsInitializer nsPrinterCUPS::DefaultSettings(
nsTArray<mozilla::PaperInfo> nsPrinterCUPS::PaperList(
Connection& aConnection) const {
http_t* const connection = aConnection.GetConnection(mPrinter);
PrinterInfoLock lock = TryEnsurePrinterInfo(connection);
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
http_t* const connection = aConnection.GetConnection(lock->mPrinter);
TryEnsurePrinterInfo(lock, connection);
if (!lock->mPrinterInfo) {
return {};
@ -300,8 +304,8 @@ nsTArray<mozilla::PaperInfo> nsPrinterCUPS::PaperList(
const int paperCount = mShim.cupsGetDestMediaCount
? mShim.cupsGetDestMediaCount(
connection, mPrinter, lock->mPrinterInfo,
CUPS_MEDIA_FLAGS_DEFAULT)
connection, lock->mPrinter,
lock->mPrinterInfo, CUPS_MEDIA_FLAGS_DEFAULT)
: 0;
nsTArray<PaperInfo> paperList;
nsTHashtable<nsCharPtrHashKey> paperSet(std::max(paperCount, 0));
@ -309,9 +313,9 @@ nsTArray<mozilla::PaperInfo> nsPrinterCUPS::PaperList(
paperList.SetCapacity(paperCount);
for (int i = 0; i < paperCount; ++i) {
cups_size_t media;
const int getInfoSucceeded =
mShim.cupsGetDestMediaByIndex(connection, mPrinter, lock->mPrinterInfo,
i, CUPS_MEDIA_FLAGS_DEFAULT, &media);
const int getInfoSucceeded = mShim.cupsGetDestMediaByIndex(
connection, lock->mPrinter, lock->mPrinterInfo, i,
CUPS_MEDIA_FLAGS_DEFAULT, &media);
if (!getInfoSucceeded || !paperSet.EnsureInserted(media.media)) {
continue;
@ -334,22 +338,21 @@ nsTArray<mozilla::PaperInfo> nsPrinterCUPS::PaperList(
return paperList;
}
nsPrinterCUPS::PrinterInfoLock nsPrinterCUPS::TryEnsurePrinterInfo(
http_t* const aConnection) const {
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
if (lock->mPrinterInfo ||
(aConnection == CUPS_HTTP_DEFAULT ? lock->mTriedInitWithDefault
: lock->mTriedInitWithConnection)) {
return lock;
void nsPrinterCUPS::TryEnsurePrinterInfo(PrinterInfoLock& aLock,
http_t* const aConnection) const {
if (aLock->mPrinterInfo ||
(aConnection == CUPS_HTTP_DEFAULT ? aLock->mTriedInitWithDefault
: aLock->mTriedInitWithConnection)) {
return;
}
if (aConnection == CUPS_HTTP_DEFAULT) {
lock->mTriedInitWithDefault = true;
aLock->mTriedInitWithDefault = true;
} else {
lock->mTriedInitWithConnection = true;
aLock->mTriedInitWithConnection = true;
}
MOZ_ASSERT(mPrinter);
MOZ_ASSERT(aLock->mPrinter);
// httpGetAddress was only added in CUPS 2.0, and some systems still use
// CUPS 1.7.
@ -427,9 +430,10 @@ nsPrinterCUPS::PrinterInfoLock nsPrinterCUPS::TryEnsurePrinterInfo(
// Match the conditional at
// https://github.com/apple/cups/blob/23c45db76a8520fd6c3b1d9164dbe312f1ab1481/cups/dest.c#L1144
// but if device-uri is null do not call into CUPS.
if ((cupsDestDeviceFlag || !FindCUPSOption("printer-uri-supported")) &&
!FindCUPSOption("device-uri")) {
return lock;
if ((cupsDestDeviceFlag ||
!FindCUPSOption(aLock, "printer-uri-supported")) &&
!FindCUPSOption(aLock, "device-uri")) {
return;
}
}
}
@ -438,13 +442,12 @@ nsPrinterCUPS::PrinterInfoLock nsPrinterCUPS::TryEnsurePrinterInfo(
// All CUPS calls that take the printer info do null-checks internally, so we
// can fetch this info and only worry about the result of the later CUPS
// functions.
lock->mPrinterInfo = mShim.cupsCopyDestInfo(aConnection, mPrinter);
aLock->mPrinterInfo = mShim.cupsCopyDestInfo(aConnection, aLock->mPrinter);
// Even if we failed to fetch printer info, it is still possible we can talk
// to the print server and get its CUPS version.
FetchCUPSVersionForPrinter(mShim, mPrinter, lock->mCUPSMajor,
lock->mCUPSMinor, lock->mCUPSPatch);
return lock;
FetchCUPSVersionForPrinter(mShim, aLock->mPrinter, aLock->mCUPSMajor,
aLock->mCUPSMinor, aLock->mCUPSPatch);
}
void nsPrinterCUPS::ForEachExtraMonochromeSetting(

View file

@ -42,18 +42,17 @@ class nsPrinterCUPS final : public nsPrinterBase {
: nsPrinterBase(aArray),
mShim(aShim),
mDisplayName(std::move(aDisplayName)),
mPrinter(aPrinter),
mPrinterInfoMutex("nsPrinterCUPS::mPrinterInfoMutex") {}
mPrinterInfoMutex(CUPSPrinterInfo{aPrinter},
"nsPrinterCUPS::mPrinterInfoMutex") {}
static void ForEachExtraMonochromeSetting(
mozilla::FunctionRef<void(const nsACString&, const nsACString&)>);
inline const char* FindCUPSOption(const char* name) const {
return mShim.cupsGetOption(name, mPrinter->num_options, mPrinter->options);
}
private:
struct CUPSPrinterInfo {
explicit constexpr CUPSPrinterInfo(cups_dest_t* aPrinter)
: mPrinter(aPrinter) {}
cups_dest_t* mPrinter;
cups_dinfo_t* mPrinterInfo = nullptr;
uint64_t mCUPSMajor = 0;
uint64_t mCUPSMinor = 0;
@ -63,9 +62,17 @@ class nsPrinterCUPS final : public nsPrinterBase {
bool mTriedInitWithDefault = false;
// Whether we have attempted to fetch mPrinterInfo with a connection.
bool mTriedInitWithConnection = false;
CUPSPrinterInfo() = default;
CUPSPrinterInfo() = delete;
CUPSPrinterInfo(const CUPSPrinterInfo&) = delete;
CUPSPrinterInfo(CUPSPrinterInfo&&) = delete;
CUPSPrinterInfo(CUPSPrinterInfo&& aOther)
: mPrinter(aOther.mPrinter),
mPrinterInfo(aOther.mPrinterInfo),
mCUPSMajor(aOther.mCUPSMajor),
mCUPSMinor(aOther.mCUPSMinor),
mCUPSPatch(aOther.mCUPSPatch) {
aOther.mPrinter = nullptr;
aOther.mPrinterInfo = nullptr;
}
};
using PrinterInfoMutex =
@ -91,6 +98,11 @@ class nsPrinterCUPS final : public nsPrinterBase {
bool IsCUPSVersionAtLeast(uint64_t aCUPSMajor, uint64_t aCUPSMinor,
uint64_t aCUPSPatch) const;
const char* FindCUPSOption(PrinterInfoLock& aLock, const char* name) const {
const cups_dest_t* const printer = aLock->mPrinter;
return mShim.cupsGetOption(name, printer->num_options, printer->options);
}
class Connection {
public:
http_t* GetConnection(cups_dest_t* aDest);
@ -114,11 +126,24 @@ class nsPrinterCUPS final : public nsPrinterBase {
* on older versions of Ubuntu (18 and below).
*/
PrinterInfoLock TryEnsurePrinterInfo(
http_t* const aConnection = CUPS_HTTP_DEFAULT) const;
http_t* const aConnection = CUPS_HTTP_DEFAULT) const {
PrinterInfoLock lock = mPrinterInfoMutex.Lock();
TryEnsurePrinterInfo(lock, aConnection);
return lock;
}
/**
* TryEnsurePrinterInfo that uses a caller-provided PrinterInfoLock.
*
* This can be used to avoid unnecessarily redundant locking of
* mPrinterInfoLock when getting a connection through
* Connection::GetConnection and then passing that into TryEnsurePrinterInfo.
*/
void TryEnsurePrinterInfo(PrinterInfoLock& aLock,
http_t* const aConnection) const;
const nsCUPSShim& mShim;
nsString mDisplayName;
cups_dest_t* mPrinter;
mutable PrinterInfoMutex mPrinterInfoMutex;
};