Commit 6b9a6186 authored by Eric Willigers's avatar Eric Willigers Committed by Commit Bot

CSS: retire use count :not with invalid selectors

This reverts the use counters added in
https://chromium-review.googlesource.com/c/chromium/src/+/1354751

The CSS Working Group was considering allowing :not() to
ignore selectors that are invalid or not accepted by the browser.

Use counts showed that this would affect a large percentage of pages
https://github.com/w3c/csswg-drafts/issues/3264#issuecomment-463036664
and so the decision was made to not change the behavior of :not
https://github.com/w3c/csswg-drafts/issues/3264#issuecomment-467190906

Data collection is no longer required.

BUG=568705

Change-Id: I38ddaee5339e19b70c1c13c9507a50ae27c354b0
Reviewed-on: https://chromium-review.googlesource.com/c/1487753
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635559}
parent 652d2d1e
......@@ -2077,9 +2077,6 @@ enum WebFeature {
kFlexboxSingleLineAlignContent = 2642,
kSignedExchangeInnerResponseInMainFrame = 2643,
kSignedExchangeInnerResponseInSubFrame = 2644,
kCSSSelectorNotWithValidList = 2645,
kCSSSelectorNotWithInvalidList = 2646,
kCSSSelectorNotWithPartiallyValidList = 2647,
// The above items are available in M72 branch.
kV8IDBFactory_Databases_Method = 2648,
......
......@@ -495,44 +495,6 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::ConsumeAttribute(
return selector;
}
void CSSSelectorParser::CountRejectedNot(CSSParserTokenRange& range) {
bool exists_valid = false;
bool exists_invalid = false;
do {
if (exists_valid || exists_invalid) {
DCHECK(range.Peek().GetType() == kCommaToken);
range.ConsumeIncludingWhitespace();
}
// else we are parsing the first complex selector
failed_parsing_ = false;
bool consumed_invalid = !ConsumeComplexSelector(range) || failed_parsing_;
range.ConsumeWhitespace();
while (!range.AtEnd() && range.Peek().GetType() != kCommaToken) {
consumed_invalid = true;
range.ConsumeIncludingWhitespace();
}
if (consumed_invalid)
exists_invalid = true;
else
exists_valid = true;
} while (!range.AtEnd());
WebFeature feature;
if (exists_valid) {
if (exists_invalid)
feature = WebFeature::kCSSSelectorNotWithPartiallyValidList;
else
feature = WebFeature::kCSSSelectorNotWithValidList;
} else {
feature = WebFeature::kCSSSelectorNotWithInvalidList;
}
context_->Count(feature);
failed_parsing_ = true;
}
std::unique_ptr<CSSParserSelector> CSSSelectorParser::ConsumePseudo(
CSSParserTokenRange& range) {
DCHECK_EQ(range.Peek().GetType(), kColonToken);
......@@ -623,17 +585,11 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::ConsumePseudo(
return selector;
}
case CSSSelector::kPseudoNot: {
CSSParserTokenRange fallback_block = block;
std::unique_ptr<CSSParserSelector> inner_selector =
ConsumeCompoundSelector(block);
block.ConsumeWhitespace();
if (!inner_selector || !inner_selector->IsSimple() ||
inner_selector->Relation() != CSSSelector::kSubSelector ||
!block.AtEnd()) {
CountRejectedNot(fallback_block);
if (!inner_selector || !inner_selector->IsSimple() || !block.AtEnd())
return nullptr;
}
Vector<std::unique_ptr<CSSParserSelector>> selector_vector;
selector_vector.push_back(std::move(inner_selector));
selector->AdoptSelectorVector(selector_vector);
......
......@@ -80,7 +80,6 @@ class CORE_EXPORT CSSSelectorParser {
SplitCompoundAtImplicitShadowCrossingCombinator(
std::unique_ptr<CSSParserSelector> compound_selector);
void RecordUsageAndDeprecations(const CSSSelectorList&);
void CountRejectedNot(CSSParserTokenRange&);
Member<const CSSParserContext> context_;
Member<const StyleSheetContents> style_sheet_;
......
......@@ -792,88 +792,4 @@ TEST(CSSSelectorParserTest, ImplicitShadowCrossingCombinators) {
}
}
TEST(CSSSelectorParserTest, UseCountRejectedNot) {
auto ExpectCount = [](const char* selector, WebFeature feature) {
std::unique_ptr<DummyPageHolder> dummy_holder =
DummyPageHolder::Create(IntSize(500, 500));
Document* doc = &dummy_holder->GetDocument();
Page::InsertOrdinaryPageForTesting(&dummy_holder->GetPage());
CSSParserContext* context = CSSParserContext::Create(
kHTMLStandardMode, SecureContextMode::kSecureContext,
CSSParserContext::kLiveProfile, doc);
StyleSheetContents* sheet = StyleSheetContents::Create(context);
EXPECT_FALSE(UseCounter::IsCounted(*doc, feature));
CSSTokenizer tokenizer(selector);
const auto tokens = tokenizer.TokenizeToEOF();
CSSParserTokenRange range(tokens);
CSSSelectorParser::ParseSelector(range, context, sheet);
bool result = UseCounter::IsCounted(*doc, feature);
EXPECT_TRUE(result);
};
ExpectCount(":not(:nonsense :gibberish)",
WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(:nonsense :gibberish, .a)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not()", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(,)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(,,)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(* .a)", WebFeature::kCSSSelectorNotWithValidList);
ExpectCount(":not(:nonsense)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(* * .previouslyFailed)",
WebFeature::kCSSSelectorNotWithValidList);
ExpectCount(":not(* :nonsense)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(:nonsense *)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(*,)", WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(*,,)", WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(:nonsense ,)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(:nonsense,,)", WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(*, *)", WebFeature::kCSSSelectorNotWithValidList);
ExpectCount(":not(*, :nonsense)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(* , * *)", WebFeature::kCSSSelectorNotWithValidList);
ExpectCount(":not(*, * :nonsense)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(:nonsense,*)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(:nonsense , :nonsense)",
WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(:nonsense, * *)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(:nonsense, * :nonsense )",
WebFeature::kCSSSelectorNotWithInvalidList);
ExpectCount(":not(*, :not(* * :nonsense))",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(:not(* * :nonsense) , *)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
ExpectCount(":not(.a || :nonsense, *)",
WebFeature::kCSSSelectorNotWithPartiallyValidList);
}
TEST(CSSSelectorParserTest, SimpleNotNeverCounted) {
// :not with a simple selector from CSS Selectors 3 is not counted.
std::unique_ptr<DummyPageHolder> dummy_holder =
DummyPageHolder::Create(IntSize(500, 500));
Document* doc = &dummy_holder->GetDocument();
Page::InsertOrdinaryPageForTesting(&dummy_holder->GetPage());
CSSParserContext* context = CSSParserContext::Create(
kHTMLStandardMode, SecureContextMode::kSecureContext,
CSSParserContext::kLiveProfile, doc);
StyleSheetContents* sheet = StyleSheetContents::Create(context);
CSSTokenizer tokenizer(":not(*)");
const auto tokens = tokenizer.TokenizeToEOF();
CSSParserTokenRange range(tokens);
CSSSelectorParser::ParseSelector(range, context, sheet);
EXPECT_FALSE(
UseCounter::IsCounted(*doc, WebFeature::kCSSSelectorNotWithValidList));
EXPECT_FALSE(
UseCounter::IsCounted(*doc, WebFeature::kCSSSelectorNotWithInvalidList));
EXPECT_FALSE(UseCounter::IsCounted(
*doc, WebFeature::kCSSSelectorNotWithPartiallyValidList));
}
} // namespace blink
......@@ -21527,9 +21527,9 @@ Called by update_net_error_codes.py.-->
<int value="2642" label="FlexboxSingleLineAlignContent"/>
<int value="2643" label="SignedExchangeInnerResponseInMainFrame"/>
<int value="2644" label="SignedExchangeInnerResponseInSubFrame"/>
<int value="2645" label="CSSSelectorNotWithValidList"/>
<int value="2646" label="CSSSelectorNotWithInvalidList"/>
<int value="2647" label="CSSSelectorNotWithPartiallyValidList"/>
<int value="2645" label="OBSOLETE_CSSSelectorNotWithValidList"/>
<int value="2646" label="OBSOLETE_CSSSelectorNotWithInvalidList"/>
<int value="2647" label="OBSOLETE_CSSSelectorNotWithPartiallyValidList"/>
<int value="2648" label="V8IDBFactory_Databases_Method"/>
<int value="2649" label="OpenerNavigationDownloadCrossOrigin"/>
<int value="2650" label="V8RegExpMatchIsTrueishOnNonJSRegExp"/>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment