Closed Bug 1448763 Opened 2 years ago Closed 2 years ago

Remove nsCSSValue::AppendToString and some other unused stuff in nsCSSValue

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(10 files)

59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
59 bytes, text/x-review-board-request
emilio
: review+
Details
41 bytes, text/x-github-pull-request
Details | Review
That is quite a bit of unused code now.

The only remaining uses as far as I can see are serialization of descriptors of @counter-style and @font-face. We should replace them with Servo serialization or just move the needed code to those files directly rather than keeping the huge serialization code.
Depends on: 1449068
Depends on: 1449087
Assignee: nobody → xidorn+moz
So there are two remaining references to the serialization code:
* nsComputedDOMStyle::DoGetAlignContent uses nsCSSValue::AppendAlignJustifyValueToString
* DOMIntersectionObserver::GetRootMargin uses nsCSSRect::AppendToString
Summary: Remove nsCSSValue::AppendToString and its friends → Remove nsCSSValue::AppendToString and other unused stuff in nsCSSValue
I believe majority of units and methods of nsCSSValue can be removed nowadays, but that need some more time to investigate.

I guess it probably makes more sense to remove the whole nsCSSValue at some point when we can drop all dependencies to it.
Summary: Remove nsCSSValue::AppendToString and other unused stuff in nsCSSValue → Remove nsCSSValue::AppendToString and some other unused stuff in nsCSSValue
Comment on attachment 8965211 [details]
Bug 1448763 part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it.

https://reviewboard.mozilla.org/r/233912/#review239554

::: dom/base/DOMIntersectionObserver.cpp:125
(Diff revision 1)
>  }
>  
>  void
>  DOMIntersectionObserver::GetRootMargin(mozilla::dom::DOMString& aRetVal)
>  {
> -  mRootMargin.AppendToString(eCSSProperty_DOM, aRetVal);
> +  nsString& retVal = aRetVal;

Huh, is this needed? I'd expect &aRetVal to just work.
Attachment #8965211 - Flags: review?(emilio) → review+
Comment on attachment 8965212 [details]
Bug 1448763 part 2 - Remove serialization code for specified value.

https://reviewboard.mozilla.org/r/233914/#review239556

Nice!
Attachment #8965212 - Flags: review?(emilio) → review+
Comment on attachment 8965213 [details]
Bug 1448763 part 3 - Remove nsCSSValueTokenStream.

https://reviewboard.mozilla.org/r/233916/#review239558
Attachment #8965213 - Flags: review?(emilio) → review+
Comment on attachment 8965214 [details]
Bug 1448763 part 4 - Remove nsCSSValueGradient and its friends.

https://reviewboard.mozilla.org/r/233918/#review239560
Attachment #8965214 - Flags: review?(emilio) → review+
Comment on attachment 8965215 [details]
Bug 1448763 part 5 - Remove all color stuff from nsCSSValue.

https://reviewboard.mozilla.org/r/233920/#review239562
Attachment #8965215 - Flags: review?(emilio) → review+
Comment on attachment 8965216 [details]
Bug 1448763 part 6 - Remove nsCSSValueTriplet.

https://reviewboard.mozilla.org/r/233922/#review239564
Attachment #8965216 - Flags: review?(emilio) → review+
Comment on attachment 8965217 [details]
Bug 1448763 part 7 - Remove nsCSSCornerSizes.

https://reviewboard.mozilla.org/r/233924/#review239566
Attachment #8965217 - Flags: review?(emilio) → review+
Comment on attachment 8965218 [details]
Bug 1448763 part 8 - Remove CSSCalc.h.

https://reviewboard.mozilla.org/r/233926/#review239568
Attachment #8965218 - Flags: review?(emilio) → review+
Comment on attachment 8965219 [details]
Bug 1448763 part 9 - Remove nsCSSRect.

https://reviewboard.mozilla.org/r/233928/#review239570
Attachment #8965219 - Flags: review?(emilio) → review+
Comment on attachment 8965211 [details]
Bug 1448763 part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it.

https://reviewboard.mozilla.org/r/233912/#review239554

> Huh, is this needed? I'd expect &aRetVal to just work.

Yes, it is needed, it cannot automatically convert a `DOMString*` to `nsAString*`, we need a semi-explicit conversion.
Attached file Servo PR
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d746dd4fe0e8
part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ce6f0d98da1f
part 2 - Remove serialization code for specified value. r=emilio
https://hg.mozilla.org/integration/autoland/rev/64aab4d047bd
part 3 - Remove nsCSSValueTokenStream. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9c43f7ce61e1
part 4 - Remove nsCSSValueGradient and its friends. r=emilio
https://hg.mozilla.org/integration/autoland/rev/1754cae1e273
part 5 - Remove all color stuff from nsCSSValue. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e56285e2657c
part 6 - Remove nsCSSValueTriplet. r=emilio
https://hg.mozilla.org/integration/autoland/rev/f43518ccb6f9
part 7 - Remove nsCSSCornerSizes. r=emilio
https://hg.mozilla.org/integration/autoland/rev/c44d6b666b2f
part 8 - Remove CSSCalc.h. r=emilio
https://hg.mozilla.org/integration/autoland/rev/7c0093bf53b5
part 9 - Remove nsCSSRect. r=emilio
You need to log in before you can comment on or make changes to this bug.