Skip to content

Commit

Permalink
Bug 1409635 (part 2) - Fix up nsIPrefLocalizedString. r=froydnj.
Browse files Browse the repository at this point in the history
nsIPrefLocalizedString is meant to be a wrapper for nsISupportsString,
basically identical but with a different identifier. But it's not a
sub-interface of nsISupportsString. Instead it (almost) duplicates
nsISupportsString's internals.

Specifically, nsISupportsString has `attribute AString data`, resulting in
these C++ methods:

> NS_IMETHOD GetData(nsAString& aData)
> NS_IMETHOD SetData(const nsAString& aData)

nsIPrefLocalizedString has `attribute wstring data`, resulting in these C++
methods:

> NS_IMETHOD GetData(char16_t** aData)
> NS_IMETHOD SetData(const char16_t* aData)

Then we have nsPrefLocalizedString, the concrete subclass of
nsIPrefLocalizedString. It implements the AString methods via
NS_FORWARD_NSISUPPORTSSTRING, which forwards to mUnicodeString. It also
implements the wstring methods explicitly, and they just call the AString
methods. It's all a bit of a mess.

(Both interfaces also have `wstring toString()`. The forwarding works more
smoothly for that method.)

This patch changes nsIPrefLocalizedString so it is a trivial sub-interface of
nsISupportsString. This change eliminates the need for the wstring methods, so
the patch removes them as well. The net result is we have less code, and fewer
conversions between C strings and Gecko strings. The patch also merges the
nsISupportsString and nsIPrefLocalizedString cases in
nsPrefBranch::SetComplexValue(), because they are now identical. (The
nsISupportsString and nsIPrefLocalizedString cases in
nsPrefBranch::GetComplexValue() remain distinct; indeed, that's the whole
reason for having them as separate interfaces.)
  • Loading branch information
nnethercote committed Oct 18, 2017
1 parent 289ca78 commit 4b08893
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 73 deletions.
63 changes: 7 additions & 56 deletions modules/libpref/Preferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2263,23 +2263,19 @@ class nsPrefBranch final

class nsPrefLocalizedString final
: public nsIPrefLocalizedString
, public nsISupportsString
{
public:
nsPrefLocalizedString();

NS_DECL_ISUPPORTS
NS_FORWARD_NSISUPPORTSSTRING(mUnicodeString->)
NS_FORWARD_NSISUPPORTSPRIMITIVE(mUnicodeString->)
NS_FORWARD_NSISUPPORTSSTRING(mUnicodeString->)

nsresult Init();

private:
virtual ~nsPrefLocalizedString();

NS_IMETHOD GetData(char16_t**) override;
NS_IMETHOD SetData(const char16_t* aData) override;

nsCOMPtr<nsISupportsString> mUnicodeString;
};

Expand Down Expand Up @@ -2587,12 +2583,12 @@ nsPrefBranch::GetComplexValue(const char* aPrefName,
nsAutoString utf16String;
rv = GetDefaultFromPropertiesFile(pref.get(), utf16String);
if (NS_SUCCEEDED(rv)) {
theString->SetData(utf16String.get());
theString->SetData(utf16String);
}
} else {
rv = GetCharPref(aPrefName, getter_Copies(utf8String));
if (NS_SUCCEEDED(rv)) {
theString->SetData(NS_ConvertUTF8toUTF16(utf8String).get());
theString->SetData(NS_ConvertUTF8toUTF16(utf8String));
}
}

Expand Down Expand Up @@ -2851,7 +2847,8 @@ nsPrefBranch::SetComplexValue(const char* aPrefName,
return SetCharPrefInternal(aPrefName, descriptorString.get());
}

if (aType.Equals(NS_GET_IID(nsISupportsString))) {
if (aType.Equals(NS_GET_IID(nsISupportsString)) ||
aType.Equals(NS_GET_IID(nsIPrefLocalizedString))) {
nsCOMPtr<nsISupportsString> theString = do_QueryInterface(aValue);

if (theString) {
Expand All @@ -2871,25 +2868,6 @@ nsPrefBranch::SetComplexValue(const char* aPrefName,
return rv;
}

if (aType.Equals(NS_GET_IID(nsIPrefLocalizedString))) {
nsCOMPtr<nsIPrefLocalizedString> theString = do_QueryInterface(aValue);

if (theString) {
nsString wideString;
rv = theString->GetData(getter_Copies(wideString));
if (NS_SUCCEEDED(rv)) {
// Check sanity of string length before any lengthy conversion
rv = CheckSanityOfStringLength(aPrefName, wideString);
if (NS_FAILED(rv)) {
return rv;
}
rv = SetCharPrefInternal(aPrefName,
NS_ConvertUTF16toUTF8(wideString).get());
}
}
return rv;
}

NS_WARNING("nsPrefBranch::SetComplexValue - Unsupported interface type");
return NS_NOINTERFACE;
}
Expand Down Expand Up @@ -3241,33 +3219,6 @@ nsPrefLocalizedString::Init()
return rv;
}

NS_IMETHODIMP
nsPrefLocalizedString::GetData(char16_t** aRetVal)
{
nsAutoString data;

nsresult rv = GetData(data);
if (NS_FAILED(rv)) {
return rv;
}

*aRetVal = ToNewUnicode(data);
if (!*aRetVal) {
return NS_ERROR_OUT_OF_MEMORY;
}

return NS_OK;
}

NS_IMETHODIMP
nsPrefLocalizedString::SetData(const char16_t* aData)
{
if (!aData) {
return SetData(EmptyString());
}
return SetData(nsDependentString(aData));
}

//----------------------------------------------------------------------------
// nsRelativeFilePref
//----------------------------------------------------------------------------
Expand Down Expand Up @@ -5018,7 +4969,7 @@ Preferences::GetLocalizedString(const char* aPref, nsAString& aResult)
aPref, NS_GET_IID(nsIPrefLocalizedString), getter_AddRefs(prefLocalString));
if (NS_SUCCEEDED(rv)) {
NS_ASSERTION(prefLocalString, "Succeeded but the result is NULL");
prefLocalString->GetData(getter_Copies(aResult));
prefLocalString->GetData(aResult);
}
return rv;
}
Expand Down Expand Up @@ -5498,7 +5449,7 @@ Preferences::GetDefaultLocalizedString(const char* aPref, nsAString& aResult)
aPref, NS_GET_IID(nsIPrefLocalizedString), getter_AddRefs(prefLocalString));
if (NS_SUCCEEDED(rv)) {
NS_ASSERTION(prefLocalString, "Succeeded but the result is NULL");
prefLocalString->GetData(getter_Copies(aResult));
prefLocalString->GetData(aResult);
}
return rv;
}
Expand Down
19 changes: 2 additions & 17 deletions modules/libpref/nsIPrefLocalizedString.idl
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "nsISupports.idl"
#include "nsISupportsPrimitives.idl"

/**
* The nsIPrefLocalizedString interface is simply a wrapper interface for
Expand All @@ -15,24 +16,8 @@
* @see nsIPrefBranch
* @see nsISupportsString
*/

[scriptable, uuid(ae419e24-1dd1-11b2-b39a-d3e5e7073802)]
interface nsIPrefLocalizedString : nsISupports
{
/**
* Provides access to string data stored in this property.
*
* @throws Error An error occurred.
*/
attribute wstring data;

/**
* Used to retrieve the contents of this object into a wide string.
*
* @return wstring The string containing the data stored within this object.
*/
wstring toString();
};
interface nsIPrefLocalizedString : nsISupportsString {};

%{C++

Expand Down

0 comments on commit 4b08893

Please sign in to comment.