Cleanup the selector cache and CSSOM methods.

RESOLVED FIXED in Firefox 61

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: emilio, Assigned: emilio)

Tracking

unspecified
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

No need to account for style backends.

Will do this after bug 1449321 lands.
Summary: Cleanup the selector cache. → Cleanup the selector cache and CSSOM methods.
This may conflict with bug 1447828. Probably consider wait after that.
Actually... I guess I have to clean up selector cache for removing style backend type.
Comment hidden (mozreview-request)

Comment 4

Last year
mozreview-review
Comment on attachment 8963570 [details]
Bug 1449502: Cleanup a bit more the selector cache and CSSOM methods.

https://reviewboard.mozilla.org/r/232494/#review237934

r=me with nits addressed.

::: dom/base/nsIDocument.h:1459
(Diff revision 1)
> -        nsCSSSelectorList* AsGecko() const
> -        {
> -          return mGecko;
> -        }
>  
> -        RawServoSelectorList* AsServo() const
> +    void CacheList(const nsAString& aSelector, SelectorList&& aSelectorList)

Use `SelectorList` without `&&` should work, since it intends to unconditionally accept the ownership of the selector list.

See https://searchfox.org/mozilla-central/rev/7e663b9fa578d425684ce2560e5fa2464f504b34/mfbt/UniquePtr.h#182-183

::: dom/base/nsIDocument.h:1484
(Diff revision 1)
> -      nsDataHashtable<nsStringHashKey, SelectorList> mTable;
> +    using SelectorTable =
> +      nsDataHashtable<nsStringHashKey, SelectorList>;

This can fit in one line.

::: dom/base/nsINode.cpp:2515
(Diff revision 1)
> -    return list->AsServo();
> +    return list->get();
>    }
>  
>    NS_ConvertUTF16toUTF8 selectorString(aSelectorString);
>  
>    auto* selectorList = Servo_SelectorList_Parse(&selectorString);

And I would recommend you to hold it in `UniquePtr` at the very beginning so that it never has a chance to leak. Something like
```c++
UniquePtr<RawServoSelectorList> selectorList(
  Servo_SelectorList_Parse(&selectorString));
```
should work.

Although in this case it probably doesn't matter a lot.
Attachment #8963570 - Flags: review?(xidorn+moz) → review+

Comment 5

Last year
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8407ca5e392
Cleanup a bit more the selector cache and CSSOM methods. r=xidorn

Comment 6

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/e8407ca5e392
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.