Skip to content

Commit

Permalink
Bug 1607006 - Remove utf-16 versions of nsCSSProps::LookupProperty* a…
Browse files Browse the repository at this point in the history
…nd ServoCSSParser::ComputeColor. r=bzbarsky

Now that we have UTF8String in the WebIDL, we can remove quite a few of the
conversions. Do that, and lift the remaining string conversions up as needed.

Also deindent Servo_ComputeColor while touching it.

Most of the remaining copies are because either bug 1606994, or because they're
WebIDL attributes that we still need to serialize back as UTF-16 (bug 1606995).

Differential Revision: https://phabricator.services.mozilla.com/D58687

--HG--
extra : moz-landing-system : lando
  • Loading branch information
emilio committed Jan 8, 2020
1 parent 65ad59b commit 86a70df
Show file tree
Hide file tree
Showing 26 changed files with 126 additions and 130 deletions.
2 changes: 1 addition & 1 deletion dom/animation/KeyframeUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ static bool GetPropertyValuesPairs(JSContext* aCx,
return false;
}
for (size_t i = 0, n = ids.length(); i < n; i++) {
nsAutoJSString propName;
nsAutoJSCString propName;
if (!propName.init(aCx, ids[i])) {
return false;
}
Expand Down
6 changes: 4 additions & 2 deletions dom/base/nsDOMWindowUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2531,7 +2531,8 @@ nsDOMWindowUtils::ComputeAnimationDistance(Element* aElement,
double* aResult) {
NS_ENSURE_ARG_POINTER(aElement);

nsCSSPropertyID property = nsCSSProps::LookupProperty(aProperty);
nsCSSPropertyID property =
nsCSSProps::LookupProperty(NS_ConvertUTF16toUTF8(aProperty));
if (property == eCSSProperty_UNKNOWN || nsCSSProps::IsShorthand(property)) {
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -2558,7 +2559,8 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement,
return NS_ERROR_INVALID_ARG;
}

nsCSSPropertyID propertyID = nsCSSProps::LookupProperty(aProperty);
nsCSSPropertyID propertyID =
nsCSSProps::LookupProperty(NS_ConvertUTF16toUTF8(aProperty));
if (propertyID == eCSSProperty_UNKNOWN ||
nsCSSProps::IsShorthand(propertyID)) {
return NS_ERROR_INVALID_ARG;
Expand Down
7 changes: 5 additions & 2 deletions dom/base/nsJSUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,8 @@ bool nsJSUtils::DumpEnabled() {
// nsDOMJSUtils.h
//

bool nsAutoJSString::init(const JS::Value& v) {
template <typename T>
bool nsTAutoJSString<T>::init(const JS::Value& v) {
// Note: it's okay to use danger::GetJSContext here instead of AutoJSAPI,
// because the init() call below is careful not to run script (for instance,
// it only calls JS::ToString for non-object values).
Expand All @@ -642,6 +643,8 @@ bool nsAutoJSString::init(const JS::Value& v) {
JS_ClearPendingException(cx);
return false;
}

return true;
}

template bool nsTAutoJSString<char16_t>::init(const JS::Value&);
template bool nsTAutoJSString<char>::init(const JS::Value&);
14 changes: 10 additions & 4 deletions dom/base/nsJSUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -335,13 +335,14 @@ inline void AssignJSLinearString(nsAString& dest, JSLinearString* s) {
js::CopyLinearStringChars(dest.BeginWriting(), s, len);
}

class nsAutoJSString : public nsAutoString {
template <typename T>
class nsTAutoJSString : public nsTAutoString<T> {
public:
/**
* nsAutoJSString should be default constructed, which leaves it empty
* nsTAutoJSString should be default constructed, which leaves it empty
* (this->IsEmpty()), and initialized with one of the init() methods below.
*/
nsAutoJSString() {}
nsTAutoJSString() {}

bool init(JSContext* aContext, JSString* str) {
return AssignJSString(aContext, *this, str);
Expand Down Expand Up @@ -371,7 +372,12 @@ class nsAutoJSString : public nsAutoString {

bool init(const JS::Value& v);

~nsAutoJSString() {}
~nsTAutoJSString() = default;
};

using nsAutoJSString = nsTAutoJSString<char16_t>;

// Note that this is guaranteed to be UTF-8.
using nsAutoJSCString = nsTAutoJSString<char>;

#endif /* nsJSUtils_h__ */
2 changes: 1 addition & 1 deletion dom/canvas/CanvasGradient.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class CanvasGradient : public nsWrapperCache {
}

// WebIDL
void AddColorStop(float offset, const nsAString& colorstr, ErrorResult& rv);
void AddColorStop(float offset, const nsACString& colorstr, ErrorResult& rv);

JSObject* WrapObject(JSContext* aCx,
JS::Handle<JSObject*> aGivenProto) override {
Expand Down
11 changes: 5 additions & 6 deletions dom/canvas/CanvasRenderingContext2D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ void CanvasPattern::SetTransform(SVGMatrix& aMatrix) {
mTransform = ToMatrix(aMatrix.GetMatrix());
}

void CanvasGradient::AddColorStop(float aOffset, const nsAString& aColorstr,
void CanvasGradient::AddColorStop(float aOffset, const nsACString& aColorstr,
ErrorResult& aRv) {
if (aOffset < 0.0 || aOffset > 1.0) {
aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
Expand All @@ -720,7 +720,6 @@ void CanvasGradient::AddColorStop(float aOffset, const nsAString& aColorstr,
nscolor color;
bool ok = ServoCSSParser::ComputeColor(styleSet, NS_RGB(0, 0, 0), aColorstr,
&color);

if (!ok) {
aRv.Throw(NS_ERROR_DOM_SYNTAX_ERR);
return;
Expand Down Expand Up @@ -995,7 +994,7 @@ JSObject* CanvasRenderingContext2D::WrapObject(
return CanvasRenderingContext2D_Binding::Wrap(aCx, this, aGivenProto);
}

bool CanvasRenderingContext2D::ParseColor(const nsAString& aString,
bool CanvasRenderingContext2D::ParseColor(const nsACString& aString,
nscolor* aColor) {
Document* document = mCanvasElement ? mCanvasElement->OwnerDoc() : nullptr;
css::Loader* loader = document ? document->CSSLoader() : nullptr;
Expand Down Expand Up @@ -1075,7 +1074,7 @@ void CanvasRenderingContext2D::SetStyleFromString(const nsAString& aStr,
MOZ_ASSERT(!aStr.IsVoid());

nscolor color;
if (!ParseColor(aStr, &color)) {
if (!ParseColor(NS_ConvertUTF16toUTF8(aStr), &color)) {
return;
}

Expand Down Expand Up @@ -2193,7 +2192,7 @@ already_AddRefed<CanvasPattern> CanvasRenderingContext2D::CreatePattern(
//
void CanvasRenderingContext2D::SetShadowColor(const nsAString& aShadowColor) {
nscolor color;
if (!ParseColor(aShadowColor, &color)) {
if (!ParseColor(NS_ConvertUTF16toUTF8(aShadowColor), &color)) {
return;
}

Expand Down Expand Up @@ -4699,7 +4698,7 @@ void CanvasRenderingContext2D::GetGlobalCompositeOperation(

void CanvasRenderingContext2D::DrawWindow(nsGlobalWindowInner& aWindow,
double aX, double aY, double aW,
double aH, const nsAString& aBgColor,
double aH, const nsACString& aBgColor,
uint32_t aFlags,
ErrorResult& aError) {
if (int32_t(aW) == 0 || int32_t(aH) == 0) {
Expand Down
4 changes: 2 additions & 2 deletions dom/canvas/CanvasRenderingContext2D.h
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ class CanvasRenderingContext2D final : public nsICanvasRenderingContextInternal,
}

void DrawWindow(nsGlobalWindowInner& aWindow, double aX, double aY, double aW,
double aH, const nsAString& aBgColor, uint32_t aFlags,
double aH, const nsACString& aBgColor, uint32_t aFlags,
mozilla::ErrorResult& aError);

// Eventually this should be deprecated. Keeping for now to keep the binding
Expand Down Expand Up @@ -561,7 +561,7 @@ class CanvasRenderingContext2D final : public nsICanvasRenderingContextInternal,
Style aWhichStyle);

// Returns whether a color was successfully parsed.
bool ParseColor(const nsAString& aString, nscolor* aColor);
bool ParseColor(const nsACString& aString, nscolor* aColor);

static void StyleColorToString(const nscolor& aColor, nsAString& aStr);

Expand Down
17 changes: 9 additions & 8 deletions dom/chrome-webidl/InspectorUtils.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,23 @@ namespace InspectorUtils {
unsigned long selectorIndex,
optional [TreatNullAs=EmptyString] DOMString pseudo = "",
optional boolean includeVisitedStyle = false);
boolean isInheritedProperty(DOMString property);
boolean isInheritedProperty(UTF8String property);
sequence<DOMString> getCSSPropertyNames(optional PropertyNamesOptions options = {});
sequence<PropertyPref> getCSSPropertyPrefs();
[Throws] sequence<DOMString> getCSSValuesForProperty(DOMString property);
[Throws] sequence<DOMString> getCSSValuesForProperty(UTF8String property);
[Throws] DOMString rgbToColorName(octet r, octet g, octet b);
InspectorRGBATuple? colorToRGBA(DOMString colorString);
boolean isValidCSSColor(DOMString colorString);
[Throws] sequence<DOMString> getSubpropertiesForCSSProperty(DOMString property);
[Throws] boolean cssPropertyIsShorthand(DOMString property);
InspectorRGBATuple? colorToRGBA(UTF8String colorString);
boolean isValidCSSColor(UTF8String colorString);
[Throws] sequence<DOMString> getSubpropertiesForCSSProperty(UTF8String property);
[Throws] boolean cssPropertyIsShorthand(UTF8String property);

[Throws] boolean cssPropertySupportsType(DOMString property, InspectorPropertyType type);
[Throws] boolean cssPropertySupportsType(UTF8String property, InspectorPropertyType type);

boolean isIgnorableWhitespace(CharacterData dataNode);
Node? getParentForNode(Node node, boolean showingAnonymousContent);
[NewObject] NodeList getChildrenForNode(Node node,
boolean showingAnonymousContent);
// FIXME(emilio, bug 1607408): Remove this.
sequence<DOMString> getBindingURLs(Element element);
[Throws] boolean setContentState(Element element, unsigned long long state);
[Throws] boolean removeContentState(
Expand All @@ -74,7 +75,7 @@ namespace InspectorUtils {
void removePseudoClassLock(Element element, DOMString pseudoClass);
boolean hasPseudoClassLock(Element element, DOMString pseudoClass);
void clearPseudoClassLocks(Element element);
[Throws] void parseStyleSheet(CSSStyleSheet sheet, DOMString input);
[Throws] void parseStyleSheet(CSSStyleSheet sheet, UTF8String input);
boolean isCustomElementName([TreatNullAs=EmptyString] DOMString name,
DOMString? namespaceURI);

Expand Down
2 changes: 1 addition & 1 deletion dom/chrome-webidl/WindowGlobalActors.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ interface WindowGlobalParent {
[Throws]
Promise<ImageBitmap> drawSnapshot(DOMRect? rect,
double scale,
DOMString backgroundColor);
UTF8String backgroundColor);

/**
* Fetches the securityInfo object for this window. This function will
Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/WindowGlobalParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ mozilla::ipc::IPCResult WindowGlobalParent::RecvShare(
}

already_AddRefed<mozilla::dom::Promise> WindowGlobalParent::DrawSnapshot(
const DOMRect* aRect, double aScale, const nsAString& aBackgroundColor,
const DOMRect* aRect, double aScale, const nsACString& aBackgroundColor,
mozilla::ErrorResult& aRv) {
nsIGlobalObject* global = GetParentObject();
RefPtr<Promise> promise = Promise::Create(global, aRv);
Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/WindowGlobalParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class WindowGlobalParent final : public WindowGlobalActor,
bool HasBeforeUnload() { return mHasBeforeUnload; }

already_AddRefed<mozilla::dom::Promise> DrawSnapshot(
const DOMRect* aRect, double aScale, const nsAString& aBackgroundColor,
const DOMRect* aRect, double aScale, const nsACString& aBackgroundColor,
mozilla::ErrorResult& aRv);

already_AddRefed<Promise> GetSecurityInfo(ErrorResult& aRv);
Expand Down
2 changes: 1 addition & 1 deletion dom/smil/SMILCompositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ nsCSSPropertyID SMILCompositor::GetCSSPropertyToAnimate() const {
}

nsCSSPropertyID propID =
nsCSSProps::LookupProperty(nsDependentAtomString(mKey.mAttributeName));
nsCSSProps::LookupProperty(nsAtomCString(mKey.mAttributeName));

if (!SMILCSSProperty::IsPropertyAnimatable(propID)) {
return eCSSProperty_UNKNOWN;
Expand Down
9 changes: 4 additions & 5 deletions dom/svg/SVGElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,10 +1111,11 @@ void MappedAttrParser::ParseMappedAttrValue(nsAtom* aMappedAttrName,

// Get the nsCSSPropertyID ID for our mapped attribute.
nsCSSPropertyID propertyID =
nsCSSProps::LookupProperty(nsDependentAtomString(aMappedAttrName));
nsCSSProps::LookupProperty(nsAtomCString(aMappedAttrName));
if (propertyID != eCSSProperty_UNKNOWN) {
bool changed = false; // outparam for ParseProperty.
NS_ConvertUTF16toUTF8 value(aMappedAttrValue);

// FIXME (bug 1343964): Figure out a better solution for sending the base
// uri to servo
nsCOMPtr<nsIReferrerInfo> referrerInfo =
Expand All @@ -1139,7 +1140,7 @@ void MappedAttrParser::ParseMappedAttrValue(nsAtom* aMappedAttrName,
}
MOZ_ASSERT(aMappedAttrName == nsGkAtoms::lang,
"Only 'lang' should be unrecognized!");
// nsCSSParser doesn't know about 'lang', so we need to handle it specially.
// CSS parser doesn't know about 'lang', so we need to handle it specially.
if (aMappedAttrName == nsGkAtoms::lang) {
propertyID = eCSSProperty__x_lang;
RefPtr<nsAtom> atom = NS_Atomize(aMappedAttrValue);
Expand All @@ -1152,9 +1153,7 @@ void MappedAttrParser::TellStyleAlreadyParsedResult(
if (!mDecl) {
mDecl = new DeclarationBlock();
}
nsCSSPropertyID propertyID =
nsCSSProps::LookupProperty(nsDependentAtomString(aAtom));

nsCSSPropertyID propertyID = nsCSSProps::LookupProperty(nsAtomCString(aAtom));
SVGElement::UpdateDeclarationBlockFromLength(*mDecl, propertyID, aLength,
SVGElement::ValToUse::Base);
}
Expand Down
4 changes: 2 additions & 2 deletions dom/webidl/CanvasRenderingContext2D.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ interface CanvasRenderingContext2D {
*/
[Throws, Func="CanvasUtils::HasDrawWindowPrivilege"]
void drawWindow(Window window, double x, double y, double w, double h,
DOMString bgColor, optional unsigned long flags = 0);
UTF8String bgColor, optional unsigned long flags = 0);

/**
* This causes a context that is currently using a hardware-accelerated
Expand Down Expand Up @@ -339,7 +339,7 @@ interface CanvasGradient {
// opaque object
[Throws]
// addColorStop should take a double
void addColorStop(float offset, DOMString color);
void addColorStop(float offset, UTF8String color);
};

[Exposed=Window]
Expand Down
6 changes: 3 additions & 3 deletions editor/libeditor/CSSEditUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ nsresult CSSEditUtils::GetCSSInlinePropertyBase(nsINode* aNode,

// from these declarations, get the one we want and that one only
//
// FIXME(bug 1606994): nsAtomCString copies unnecessarily.
// FIXME(bug 1606994): nsAtomCString copies, we should just keep around the
// property id.
MOZ_ALWAYS_SUCCEEDS(
cssDecl->GetPropertyValue(nsAtomCString(aProperty), aValue));

Expand All @@ -468,8 +469,7 @@ nsresult CSSEditUtils::GetCSSInlinePropertyBase(nsINode* aNode,
return NS_OK;
}

nsCSSPropertyID prop =
nsCSSProps::LookupProperty(nsDependentAtomString(aProperty));
nsCSSPropertyID prop = nsCSSProps::LookupProperty(nsAtomCString(aProperty));
MOZ_ASSERT(prop != eCSSProperty_UNKNOWN);

decl->GetPropertyValueByID(prop, aValue);
Expand Down
3 changes: 2 additions & 1 deletion editor/libeditor/EditorBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5523,7 +5523,8 @@ void EditorBase::AutoEditActionDataSetter::SetColorData(

bool wasCurrentColor = false;
nscolor color = NS_RGB(0, 0, 0);
if (!ServoCSSParser::ComputeColor(nullptr, NS_RGB(0, 0, 0), aData, &color,
if (!ServoCSSParser::ComputeColor(nullptr, NS_RGB(0, 0, 0),
NS_ConvertUTF16toUTF8(aData), &color,
&wasCurrentColor)) {
// If we cannot parse aData, let's set original value as-is. It could be
// new format defined by newer spec.
Expand Down
Loading

0 comments on commit 86a70df

Please sign in to comment.