tree 02389b844084813187aab58c6462df1bfcfd847c
parent 13af70a7f91d692d7eb5b27c9d40a9d830a4148b
author Oriol Brufau <obrufau@igalia.com> 1592398713 +0000
committer Commit Bot <commit-bot@chromium.org> 1592398713 +0000

Reland "[css-pseudo] Set default ::marker styles in a real UA sheet"

This is a reland of a53560e3c83ab4d7b379247d16995ca774e772ba

The problem in the reverted original patch was that I forgot to clear
marker_style_sheet_ in CSSDefaultStyleSheets::PrepareForLeakDetection.
Then, when running tests with --enable-leak-detection, it could happen
that EnsureDefaultStyleSheetsForPseudoElement would think that the UA
rules for ::marker were already in default_pseudo_element_style_. But
it would actually be empty, so the ::marker could get no rule, making
HasMatchedProperties() be false, so PseudoStyleForElementInternal would
return false, and PseudoStyleForElement would return a nullptr style.
So there would be no ::marker.

Original change's description:
> [css-pseudo] Set default ::marker styles in a real UA sheet
>
> Before this patch, default styles for ::marker were assigned manually
> when resolving the style. The problem was that, since the rules didn't
> actually exist in any stylesheet, they were not exposed in devtools.
> Also, devtools assume that if a pseudo-element exists, it must have some
> rule. This could not hold for ::marker, and then trying to add styles
> to it would fail.
>
> This patch fixes that by making the styles actual UA rules. However,
> adding a '::marker' selector directly to the UA sheet for elements would
> be bad: it would match every single element (even non-list-items) due to
> the universal selector, and then it would set an internal bit for the
> marker. So HasAnyPseudoElementStyles() would return true, which means
> extra work when handling style changes.
>
> Therefore, this patch adds a new UA sheet just for pseudo-elements. This
> sheet is not used for elements, ensuring that HasAnyPseudoElementStyles
> is still false if there are no author styles for a pseudo-element.
>
> The selector parser was treating ::marker as invalid if the runtime flag
> CSSMarkerPseudoElement was disabled. But now it's accepted in that case
> for UA styles.
>
> There already was a kHasMarkerPseudoElement counter to track how widely
> used is ::marker on the web. Now this counter will only be incremented
> for author rules.
>
> Bug: 457718, 1082377
>
> TEST=blink_unittests --gtest_filter=StyleResolverTest.CSSMarkerPseudoElement
> TEST=blink_unittests --gtest_filter=UseCounterHelperTest.CSSMarkerPseudoElementUA
> TEST=blink_unittests --gtest_filter=UseCounterHelperTest.CSSMarkerPseudoElementAuthor
> TEST=http/tests/devtools/elements/styles-2/pseudo-elements.js
>
> Change-Id: Ib5583dc1952bfec2cce8f33fe8332cc39016c34a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239579
> Reviewed-by: Mike West <mkwst@chromium.org>
> Reviewed-by: Rune Lillesveen <futhark@chromium.org>
> Commit-Queue: Oriol Brufau <obrufau@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#778303}

TBR=mkwst@chromium.org

Bug: 457718, 1082377
Change-Id: Ib6a2482ac474a424c3dfeb05fb2e2c00ce29b732
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247790
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#779284}