Skip to content

Commit

Permalink
Bug 1682003 - Avoid UTF-8 -> UTF-16 conversion during CSSOM serializa…
Browse files Browse the repository at this point in the history 
…tion. r=heycam

This lifts a bunch of string conversions higher up the stack, but allows
us to make the servo code use utf-8 unconditionally, and seemed faster
in my benchmarking (see comment 0).

It should also make a bunch of attribute setters faster too (like
setting .cssText), now that we use UTF8String for them (we couldn't
because we couldn't specify different string types for the getter and
setters).

Differential Revision: https://phabricator.services.mozilla.com/D99590
  • Loading branch information
@emilio
emilio committed Dec 17, 2020
1 parent c2e233e commit 039592f
Show file tree
Hide file tree
Showing 122 changed files with 671 additions and 749 deletions.
8 changes: 6 additions & 2 deletions accessible/base/StyleInfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ StyleInfo::StyleInfo(dom::Element* aElement) : mElement(aElement) {

void StyleInfo::Display(nsAString& aValue) {
aValue.Truncate();
mComputedStyle->GetComputedPropertyValue(eCSSProperty_display, aValue);
nsAutoCString value;
mComputedStyle->GetComputedPropertyValue(eCSSProperty_display, value);
CopyUTF8toUTF16(value, aValue);
}

void StyleInfo::TextAlign(nsAString& aValue) {
aValue.Truncate();
mComputedStyle->GetComputedPropertyValue(eCSSProperty_text_align, aValue);
nsAutoCString value;
mComputedStyle->GetComputedPropertyValue(eCSSProperty_text_align, value);
CopyUTF8toUTF16(value, aValue);
}

void StyleInfo::TextIndent(nsAString& aValue) {
Expand Down
9 changes: 5 additions & 4 deletions accessible/windows/sdn/sdnAccessible.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ sdnAccessible::get_computedStyle(
for (index = realIndex = 0; index < length && realIndex < aMaxStyleProperties;
index++) {
nsAutoCString property;
nsAutoString value;
nsAutoCString value;

// Ignore -moz-* properties.
cssDecl->Item(index, property);
Expand All @@ -230,7 +230,8 @@ sdnAccessible::get_computedStyle(
if (!value.IsEmpty()) {
aStyleProperties[realIndex] =
::SysAllocString(NS_ConvertUTF8toUTF16(property).get());
aStyleValues[realIndex] = ::SysAllocString(value.get());
aStyleValues[realIndex] =
::SysAllocString(NS_ConvertUTF8toUTF16(value).get());
++realIndex;
}
}
Expand All @@ -256,12 +257,12 @@ sdnAccessible::get_computedStyleForProperties(

uint32_t index = 0;
for (index = 0; index < aNumStyleProperties; index++) {
nsAutoString value;
nsAutoCString value;
if (aStyleProperties[index])
cssDecl->GetPropertyValue(
NS_ConvertUTF16toUTF8(nsDependentString(aStyleProperties[index])),
value); // Get property value
aStyleValues[index] = ::SysAllocString(value.get());
aStyleValues[index] = ::SysAllocString(NS_ConvertUTF8toUTF16(value).get());
}

return S_OK;
Expand Down
2 changes: 1 addition & 1 deletion dom/animation/ComputedTimingFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ int32_t ComputedTimingFunction::Compare(
return 0;
}

void ComputedTimingFunction::AppendToString(nsAString& aResult) const {
void ComputedTimingFunction::AppendToString(nsACString& aResult) const {
nsTimingFunction timing;
switch (mType) {
case Type::CubicBezier:
Expand Down
2 changes: 1 addition & 1 deletion dom/animation/ComputedTimingFunction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class ComputedTimingFunction {
return !(*this == aOther);
}
int32_t Compare(const ComputedTimingFunction& aRhs) const;
void AppendToString(nsAString& aResult) const;
void AppendToString(nsACString& aResult) const;

static double GetPortion(const Maybe<ComputedTimingFunction>& aFunction,
double aPortion, BeforeFlag aBeforeFlag) {
Expand Down
15 changes: 7 additions & 8 deletions dom/animation/KeyframeEffect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1037,12 +1037,11 @@ void DumpAnimationProperties(
for (auto& p : aAnimationProperties) {
printf("%s\n", nsCString(nsCSSProps::GetStringValue(p.mProperty)).get());
for (auto& s : p.mSegments) {
nsString fromValue, toValue;
nsAutoCString fromValue, toValue;
s.mFromValue.SerializeSpecifiedValue(p.mProperty, aRawSet, fromValue);
s.mToValue.SerializeSpecifiedValue(p.mProperty, aRawSet, toValue);
printf(" %f..%f: %s..%s\n", s.mFromKey, s.mToKey,
NS_ConvertUTF16toUTF8(fromValue).get(),
NS_ConvertUTF16toUTF8(toValue).get());
printf(" %f..%f: %s..%s\n", s.mFromKey, s.mToKey, fromValue.get(),
toValue.get());
}
}
}
Expand Down Expand Up @@ -1131,7 +1130,7 @@ static void CreatePropertyValue(
aResult.mOffset = aOffset;

if (!aValue.IsNull()) {
nsString stringValue;
nsAutoCString stringValue;
aValue.SerializeSpecifiedValue(aProperty, aRawSet, stringValue);
aResult.mValue.Construct(stringValue);
}
Expand All @@ -1140,7 +1139,7 @@ static void CreatePropertyValue(
aResult.mEasing.Construct();
aTimingFunction->AppendToString(aResult.mEasing.Value());
} else {
aResult.mEasing.Construct(u"linear"_ns);
aResult.mEasing.Construct("linear"_ns);
}

aResult.mComposite = aComposite;
Expand Down Expand Up @@ -1298,7 +1297,7 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray<JSObject*>& aResult,

JS::Rooted<JSObject*> keyframeObject(aCx, &keyframeJSValue.toObject());
for (const PropertyValuePair& propertyValue : keyframe.mPropertyValues) {
nsAutoString stringValue;
nsAutoCString stringValue;
// Don't serialize the custom properties for this keyframe.
if (propertyValue.mProperty ==
nsCSSPropertyID::eCSSPropertyExtra_variable) {
Expand Down Expand Up @@ -1338,7 +1337,7 @@ void KeyframeEffect::GetKeyframes(JSContext* aCx, nsTArray<JSObject*>& aResult,
}

JS::Rooted<JS::Value> value(aCx);
if (!ToJSValue(aCx, stringValue, &value) ||
if (!NonVoidUTF8StringToJsval(aCx, stringValue, &value) ||
!JS_DefineProperty(aCx, keyframeObject, name, value,
JSPROP_ENUMERATE)) {
aRv.Throw(NS_ERROR_FAILURE);
Expand Down
28 changes: 14 additions & 14 deletions dom/animation/KeyframeUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ enum class ListAllowance { eDisallow, eAllow };
*/
struct PropertyValuesPair {
nsCSSPropertyID mProperty;
nsTArray<nsString> mValues;
nsTArray<nsCString> mValues;
};

/**
Expand Down Expand Up @@ -153,13 +153,13 @@ static bool GetPropertyValuesPairs(JSContext* aCx,
static bool AppendStringOrStringSequenceToArray(JSContext* aCx,
JS::Handle<JS::Value> aValue,
ListAllowance aAllowLists,
nsTArray<nsString>& aValues);
nsTArray<nsCString>& aValues);

static bool AppendValueAsString(JSContext* aCx, nsTArray<nsString>& aValues,
static bool AppendValueAsString(JSContext* aCx, nsTArray<nsCString>& aValues,
JS::Handle<JS::Value> aValue);

static Maybe<PropertyValuePair> MakePropertyValuePair(
nsCSSPropertyID aProperty, const nsAString& aStringValue,
nsCSSPropertyID aProperty, const nsACString& aStringValue,
dom::Document* aDocument);

static bool HasValidOffsets(const nsTArray<Keyframe>& aKeyframes);
Expand Down Expand Up @@ -574,7 +574,7 @@ static bool GetPropertyValuesPairs(JSContext* aCx,
static bool AppendStringOrStringSequenceToArray(JSContext* aCx,
JS::Handle<JS::Value> aValue,
ListAllowance aAllowLists,
nsTArray<nsString>& aValues) {
nsTArray<nsCString>& aValues) {
if (aAllowLists == ListAllowance::eAllow && aValue.isObject()) {
// The value is an object, and we want to allow lists; convert
// aValue to (DOMString or sequence<DOMString>).
Expand Down Expand Up @@ -613,17 +613,17 @@ static bool AppendStringOrStringSequenceToArray(JSContext* aCx,
/**
* Converts aValue to DOMString and appends it to aValues.
*/
static bool AppendValueAsString(JSContext* aCx, nsTArray<nsString>& aValues,
static bool AppendValueAsString(JSContext* aCx, nsTArray<nsCString>& aValues,
JS::Handle<JS::Value> aValue) {
return ConvertJSValueToString(aCx, aValue, dom::eStringify, dom::eStringify,
*aValues.AppendElement());
}

static void ReportInvalidPropertyValueToConsole(
nsCSSPropertyID aProperty, const nsAString& aInvalidPropertyValue,
nsCSSPropertyID aProperty, const nsACString& aInvalidPropertyValue,
dom::Document* aDoc) {
AutoTArray<nsString, 2> params;
params.AppendElement(aInvalidPropertyValue);
params.AppendElement(NS_ConvertUTF8toUTF16(aInvalidPropertyValue));
CopyASCIItoUTF16(nsCSSProps::GetStringValue(aProperty),
*params.AppendElement());
nsContentUtils::ReportToConsole(nsIScriptError::warningFlag, "Animation"_ns,
Expand All @@ -642,7 +642,7 @@ static void ReportInvalidPropertyValueToConsole(
* an invalid property value.
*/
static Maybe<PropertyValuePair> MakePropertyValuePair(
nsCSSPropertyID aProperty, const nsAString& aStringValue,
nsCSSPropertyID aProperty, const nsACString& aStringValue,
dom::Document* aDocument) {
MOZ_ASSERT(aDocument);
Maybe<PropertyValuePair> result;
Expand Down Expand Up @@ -1017,7 +1017,7 @@ static void GetKeyframeListFromPropertyIndexedKeyframe(
size_t n = pair.mValues.Length() - 1;
size_t i = 0;

for (const nsString& stringValue : pair.mValues) {
for (const nsCString& stringValue : pair.mValues) {
// For single-valued lists, the single value should be added to a
// keyframe with offset 1.
double offset = n ? i++ / double(n) : 1;
Expand Down Expand Up @@ -1097,7 +1097,7 @@ static void GetKeyframeListFromPropertyIndexedKeyframe(
// This corresponds to step 5, "Otherwise," branch, substeps 7-11 of
// https://drafts.csswg.org/web-animations/#processing-a-keyframes-argument
FallibleTArray<Maybe<ComputedTimingFunction>> easings;
auto parseAndAppendEasing = [&](const nsString& easingString,
auto parseAndAppendEasing = [&](const nsACString& easingString,
ErrorResult& aRv) {
auto easing = TimingParams::ParseEasing(easingString, aRv);
if (!aRv.Failed() && !easings.AppendElement(std::move(easing), fallible)) {
Expand All @@ -1106,14 +1106,14 @@ static void GetKeyframeListFromPropertyIndexedKeyframe(
};

auto& easing = keyframeDict.mEasing;
if (easing.IsString()) {
parseAndAppendEasing(easing.GetAsString(), aRv);
if (easing.IsUTF8String()) {
parseAndAppendEasing(easing.GetAsUTF8String(), aRv);
if (aRv.Failed()) {
aResult.Clear();
return;
}
} else {
for (const nsString& easingString : easing.GetAsStringSequence()) {
for (const auto& easingString : easing.GetAsUTF8StringSequence()) {
parseAndAppendEasing(easingString, aRv);
if (aRv.Failed()) {
aResult.Clear();
Expand Down
5 changes: 2 additions & 3 deletions dom/animation/TimingParams.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,10 @@ TimingParams TimingParams::MergeOptionalEffectTiming(

/* static */
Maybe<ComputedTimingFunction> TimingParams::ParseEasing(
const nsAString& aEasing, ErrorResult& aRv) {
const nsACString& aEasing, ErrorResult& aRv) {
nsTimingFunction timingFunction;
if (!ServoCSSParser::ParseEasing(aEasing, timingFunction)) {
aRv.ThrowTypeError<dom::MSG_INVALID_EASING_ERROR>(
NS_ConvertUTF16toUTF8(aEasing));
aRv.ThrowTypeError<dom::MSG_INVALID_EASING_ERROR>(aEasing);
return Nothing();
}

Expand Down
2 changes: 1 addition & 1 deletion dom/animation/TimingParams.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ struct TimingParams {
}
}

static Maybe<ComputedTimingFunction> ParseEasing(const nsAString& aEasing,
static Maybe<ComputedTimingFunction> ParseEasing(const nsACString& aEasing,
ErrorResult& aRv);

static StickyTimeDuration CalcActiveDuration(
Expand Down
4 changes: 2 additions & 2 deletions dom/base/AnonymousContent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ bool AnonymousContent::WrapObject(JSContext* aCx,

void AnonymousContent::GetComputedStylePropertyValue(
const nsAString& aElementId, const nsACString& aPropertyName,
DOMString& aResult, ErrorResult& aRv) {
nsACString& aResult, ErrorResult& aRv) {
Element* element = GetElementById(aElementId);
if (!element) {
aRv.Throw(NS_ERROR_NOT_AVAILABLE);
Expand Down Expand Up @@ -215,7 +215,7 @@ void AnonymousContent::SetStyle(const nsACString& aProperty,

nsGenericHTMLElement* element = nsGenericHTMLElement::FromNode(mContentNode);
nsCOMPtr<nsICSSDeclaration> declaration = element->Style();
declaration->SetProperty(aProperty, aValue, u""_ns, IgnoreErrors());
declaration->SetProperty(aProperty, aValue, ""_ns, IgnoreErrors());
}

} // namespace mozilla::dom
2 changes: 1 addition & 1 deletion dom/base/AnonymousContent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class AnonymousContent final {

void GetComputedStylePropertyValue(const nsAString& aElementId,
const nsACString& aPropertyName,
DOMString& aResult, ErrorResult& aRv);
nsACString& aResult, ErrorResult& aRv);

void GetTargetIdForEvent(Event& aEvent, DOMString& aResult);

Expand Down
7 changes: 3 additions & 4 deletions dom/base/DOMIntersectionObserver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,14 @@ DOMIntersectionObserver::CreateLazyLoadObserver(Document& aDocument) {
return observer.forget();
}

bool DOMIntersectionObserver::SetRootMargin(const nsAString& aString) {
bool DOMIntersectionObserver::SetRootMargin(const nsACString& aString) {
return Servo_IntersectionObserverRootMargin_Parse(&aString, &mRootMargin);
}

nsISupports* DOMIntersectionObserver::GetParentObject() const { return mOwner; }

void DOMIntersectionObserver::GetRootMargin(DOMString& aRetVal) {
nsString& retVal = aRetVal;
Servo_IntersectionObserverRootMargin_ToString(&mRootMargin, &retVal);
void DOMIntersectionObserver::GetRootMargin(nsACString& aRetVal) {
Servo_IntersectionObserverRootMargin_ToString(&mRootMargin, &aRetVal);
}

void DOMIntersectionObserver::GetThresholds(nsTArray<double>& aRetVal) {
Expand Down
6 changes: 3 additions & 3 deletions dom/base/DOMIntersectionObserver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ class DOMIntersectionObserver final : public nsISupports,

nsINode* GetRoot() const { return mRoot; }

void GetRootMargin(DOMString& aRetVal);
void GetRootMargin(nsACString&);
bool SetRootMargin(const nsACString&);

void GetThresholds(nsTArray<double>& aRetVal);
void Observe(Element& aTarget);
void Unobserve(Element& aTarget);
Expand All @@ -120,8 +122,6 @@ class DOMIntersectionObserver final : public nsISupports,

void TakeRecords(nsTArray<RefPtr<DOMIntersectionObserverEntry>>& aRetVal);

bool SetRootMargin(const nsAString& aString);

void Update(Document* aDocument, DOMHighResTimeStamp time);
MOZ_CAN_RUN_SCRIPT void Notify();

Expand Down
2 changes: 1 addition & 1 deletion dom/base/Document.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8736,7 +8736,7 @@ void Document::DoNotifyPossibleTitleChange() {
}

already_AddRefed<MediaQueryList> Document::MatchMedia(
const nsAString& aMediaQueryList, CallerType aCallerType) {
const nsACString& aMediaQueryList, CallerType aCallerType) {
RefPtr<MediaQueryList> result =
new MediaQueryList(this, aMediaQueryList, aCallerType);

Expand Down
2 changes: 1 addition & 1 deletion dom/base/Document.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2471,7 +2471,7 @@ class Document : public nsINode,
* Support for window.matchMedia()
*/

already_AddRefed<MediaQueryList> MatchMedia(const nsAString& aMediaQueryList,
already_AddRefed<MediaQueryList> MatchMedia(const nsACString& aMediaQueryList,
CallerType aCallerType);

LinkedList<MediaQueryList>& MediaQueryLists() { return mDOMMediaQueryLists; }
Expand Down
4 changes: 3 additions & 1 deletion dom/base/nsAttrValue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,9 @@ void nsAttrValue::ToString(nsAString& aResult) const {
aResult.Truncate();
MiscContainer* container = GetMiscContainer();
if (DeclarationBlock* decl = container->mValue.mCSSDeclaration) {
decl->ToString(aResult);
nsAutoCString result;
decl->ToString(result);
CopyUTF8toUTF16(result, aResult);
}

// This can be reached during parallel selector matching with attribute
Expand Down
15 changes: 11 additions & 4 deletions dom/base/nsDOMWindowUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2171,11 +2171,14 @@ nsDOMWindowUtils::GetVisitedDependentComputedStyle(
ENSURE_SUCCESS(rv, rv.StealNSResult());
}

nsAutoCString result;

static_cast<nsComputedDOMStyle*>(decl.get())->SetExposeVisitedStyle(true);
nsresult rv =
decl->GetPropertyValue(NS_ConvertUTF16toUTF8(aPropertyName), aResult);
decl->GetPropertyValue(NS_ConvertUTF16toUTF8(aPropertyName), result);
static_cast<nsComputedDOMStyle*>(decl.get())->SetExposeVisitedStyle(false);

CopyUTF8toUTF16(result, aResult);
return rv;
}

Expand Down Expand Up @@ -2822,8 +2825,10 @@ nsDOMWindowUtils::ComputeAnimationDistance(Element* aElement,
return NS_ERROR_ILLEGAL_VALUE;
}

AnimationValue v1 = AnimationValue::FromString(property, aValue1, aElement);
AnimationValue v2 = AnimationValue::FromString(property, aValue2, aElement);
AnimationValue v1 = AnimationValue::FromString(
property, NS_ConvertUTF16toUTF8(aValue1), aElement);
AnimationValue v2 = AnimationValue::FromString(
property, NS_ConvertUTF16toUTF8(aValue2), aElement);
if (v1.IsNull() || v2.IsNull()) {
return NS_ERROR_ILLEGAL_VALUE;
}
Expand Down Expand Up @@ -2883,8 +2888,10 @@ nsDOMWindowUtils::GetUnanimatedComputedStyle(Element* aElement,
if (!aElement->GetComposedDoc()) {
return NS_ERROR_FAILURE;
}
nsAutoCString result;
Servo_AnimationValue_Serialize(value, propertyID,
presShell->StyleSet()->RawSet(), &aResult);
presShell->StyleSet()->RawSet(), &result);
CopyUTF8toUTF16(result, aResult);
return NS_OK;
}

Expand Down
2 changes: 1 addition & 1 deletion dom/base/nsGlobalWindowInner.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3539,7 +3539,7 @@ void nsGlobalWindowInner::CancelAnimationFrame(int32_t aHandle,
}

already_AddRefed<MediaQueryList> nsGlobalWindowInner::MatchMedia(
const nsAString& aMediaQueryList, CallerType aCallerType,
const nsACString& aMediaQueryList, CallerType aCallerType,
ErrorResult& aError) {
ENSURE_ACTIVE_DOCUMENT(aError, nullptr);
return mDoc->MatchMedia(aMediaQueryList, aCallerType);
Expand Down
Loading

0 comments on commit 039592f

Please sign in to comment.