Skip to content

Commit

Permalink
Add partial support for @Property.
Browse files Browse the repository at this point in the history
It is generally working, with the following omissions:

 * Unregistering properties is not implemented. This means we can't
   cascade @Property rules. The temporary behavior is that the first
   @Property rule seen wins (forever). It also means mutating the
   rule via CSSOM is not supported (as it effectively re-registers
   the property).
 * TreeScope is ignored (and untested).
 * CSSPropertyRule::cssText() returns an empty string, for now.
   Implementing this in a nice way requires a medium-ish refactor, so
   it's better to do this in a separate CL.

Also, in my proposed PR for @Property, the CSSPropertyRule interface
contains a name, and a CSSStyleDeclaration with the descriptors, and this
CL matches that approach. After some GitHub discussion, I expect that
this will change such that the CSSPropertyRule contains a name, syntax,
inherits (bool) and initialValue instead. I would prefer to make the
corresponding change in Blink separately, however.

I2I=https://groups.google.com/a/chromium.org/d/msg/blink-dev/qoJkuLOMKqI/I3DGDTCRBQAJ
BUG=973830

Change-Id: I813ce53bc279a2c0e40fe6ed3ec6ed3084004d9d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1303340
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#673287}
  • Loading branch information
andruud authored and Commit Bot committed Jun 28, 2019
1 parent ec3ade7 commit 2b9ff48
Show file tree
Hide file tree
Showing 40 changed files with 869 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

module blink.mojom;

const int32 kMaximumCSSSampleId = 640;
const int32 kMaximumCSSSampleId = 643;

// This CSSSampleId represents page load for CSS histograms. It is recorded once
// per page visit for each CSS histogram being logged on the blink side and the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2326,6 +2326,7 @@ enum WebFeature {
kPeriodicBackgroundSyncGetTags = 2937,
kPeriodicBackgroundSyncUnregister = 2938,
kCreateObjectURLMediaSourceFromWorker = 2939,
kCSSAtRuleProperty = 2940,

// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/core_idl_files.gni
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ core_idl_files =
"css/css_style_rule.idl",
"css/css_style_sheet.idl",
"css/css_supports_rule.idl",
"css/css_property_rule.idl",
"css/css_viewport_rule.idl",
"css/font_face.idl",
"css/font_face_set.idl",
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/css/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ blink_core_sources("css") {
"css_property_id_templates.h",
"css_property_name.cc",
"css_property_name.h",
"css_property_rule.cc",
"css_property_rule.h",
"css_property_source_data.cc",
"css_property_source_data.h",
"css_property_value.cc",
Expand Down
18 changes: 18 additions & 0 deletions third_party/blink/renderer/core/css/css_properties.json5
Original file line number Diff line number Diff line change
Expand Up @@ -4553,6 +4553,24 @@
is_property: false,
runtime_flag: "DisplayCutoutAPI",
},
{
name: "syntax",
is_descriptor: true,
is_property: false,
runtime_flag: "CSSVariables2AtProperty",
},
{
name: "initial-value",
is_descriptor: true,
is_property: false,
runtime_flag: "CSSVariables2AtProperty",
},
{
name: "inherits",
is_descriptor: true,
is_property: false,
runtime_flag: "CSSVariables2AtProperty",
},

// Shorthands
{
Expand Down
49 changes: 49 additions & 0 deletions third_party/blink/renderer/core/css/css_property_rule.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "third_party/blink/renderer/core/css/css_property_rule.h"

#include "third_party/blink/renderer/core/css/css_property_value_set.h"
#include "third_party/blink/renderer/core/css/style_rule.h"
#include "third_party/blink/renderer/core/css/style_rule_css_style_declaration.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"

namespace blink {

CSSPropertyRule::CSSPropertyRule(StyleRuleProperty* property_rule,
CSSStyleSheet* sheet)
: CSSRule(sheet), property_rule_(property_rule) {}

CSSPropertyRule::~CSSPropertyRule() = default;

CSSStyleDeclaration* CSSPropertyRule::style() const {
if (!properties_cssom_wrapper_) {
properties_cssom_wrapper_ =
MakeGarbageCollected<StyleRuleCSSStyleDeclaration>(
property_rule_->MutableProperties(),
const_cast<CSSPropertyRule*>(this));
}

return properties_cssom_wrapper_.Get();
}

String CSSPropertyRule::cssText() const {
// TODO(https://crbug.com/978783): Implement this.
return "";
}

void CSSPropertyRule::Reattach(StyleRuleBase* rule) {
DCHECK(rule);
property_rule_ = To<StyleRuleProperty>(rule);
if (properties_cssom_wrapper_)
properties_cssom_wrapper_->Reattach(property_rule_->MutableProperties());
}

void CSSPropertyRule::Trace(blink::Visitor* visitor) {
visitor->Trace(property_rule_);
visitor->Trace(properties_cssom_wrapper_);
CSSRule::Trace(visitor);
}

} // namespace blink
48 changes: 48 additions & 0 deletions third_party/blink/renderer/core/css/css_property_rule.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_RULE_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_RULE_H_

#include "third_party/blink/renderer/core/css/css_rule.h"
#include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/wtf/casting.h"

namespace blink {

class CSSStyleDeclaration;
class StyleRuleProperty;
class StyleRuleCSSStyleDeclaration;

class CSSPropertyRule final : public CSSRule {
DEFINE_WRAPPERTYPEINFO();

public:
CSSPropertyRule(StyleRuleProperty*, CSSStyleSheet*);
~CSSPropertyRule() override;

String cssText() const override;
void Reattach(StyleRuleBase*) override;

CSSStyleDeclaration* style() const;

void Trace(blink::Visitor*) override;

private:
CSSRule::Type type() const override { return kPropertyRule; }

Member<StyleRuleProperty> property_rule_;
mutable Member<StyleRuleCSSStyleDeclaration> properties_cssom_wrapper_;
};

template <>
struct DowncastTraits<CSSPropertyRule> {
static bool AllowFrom(const CSSRule& rule) {
return rule.type() == CSSRule::kPropertyRule;
}
};

} // namespace blink

#endif // THIRD_PARTY_BLINK_RENDERER_CORE_CSS_CSS_PROPERTY_RULE_H_
9 changes: 9 additions & 0 deletions third_party/blink/renderer/core/css/css_property_rule.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

[
RuntimeEnabled=CSSVariables2AtProperty
] interface CSSPropertyRule : CSSRule {
readonly attribute CSSStyleDeclaration style;
};
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/css/css_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class CORE_EXPORT CSSRule : public ScriptWrappable {
public:
~CSSRule() override = default;

// The values must match the table in [1]. See also css_rule.idl.
// [1] https://wiki.csswg.org/spec/cssom-constants
enum Type {
kStyleRule = 1,
kCharsetRule = 2,
Expand All @@ -55,6 +57,11 @@ class CORE_EXPORT CSSRule : public ScriptWrappable {
kSupportsRule = 12,
kFontFeatureValuesRule = 14,
kViewportRule = 15,
// Experimental features below. Such features must be greater than 1000:
// the 0-1000 range is reserved by the CSS Working Group.
//
// TODO(https://crbug.com/978781): Spec a proper number.
kPropertyRule = 1001,
};

virtual Type type() const = 0;
Expand Down
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/css/css_rule.idl
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,8 @@
// CSS Device Adaptation
// https://drafts.csswg.org/css-device-adapt/#css-rule-interface
[RuntimeEnabled=CSSViewport] const unsigned short VIEWPORT_RULE = 15;

// CSS Properties and Values Level 1
// TODO(https://crbug.com/978781): Spec a proper number.
[RuntimeEnabled=CSSVariables2AtProperty] const unsigned short PROPERTY_RULE = 1001;
};
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/css/css_value_keywords.json5
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,10 @@
// pan-down
// none

// @property
"true",
"false",

// (prefers-*:) media features
"no-preference",

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@

#include "third_party/blink/renderer/core/css/parser/at_rule_descriptor_parser.h"

#include "third_party/blink/renderer/core/css/css_custom_property_declaration.h"
#include "third_party/blink/renderer/core/css/css_font_face_src_value.h"
#include "third_party/blink/renderer/core/css/css_string_value.h"
#include "third_party/blink/renderer/core/css/css_unicode_range_value.h"
#include "third_party/blink/renderer/core/css/css_value.h"
#include "third_party/blink/renderer/core/css/parser/css_parser_context.h"
#include "third_party/blink/renderer/core/css/parser/css_parser_mode.h"
#include "third_party/blink/renderer/core/css/parser/css_parser_token_range.h"
#include "third_party/blink/renderer/core/css/parser/css_tokenizer.h"
#include "third_party/blink/renderer/core/css/parser/css_variable_parser.h"
#include "third_party/blink/renderer/core/css/properties/css_parsing_utils.h"
#include "third_party/blink/renderer/core/css/properties/css_property.h"

Expand Down Expand Up @@ -209,14 +212,53 @@ CSSValue* AtRuleDescriptorParser::ParseFontFaceDeclaration(
return ParseFontFaceDescriptor(id, range, context);
}

CSSValue* AtRuleDescriptorParser::ParseAtPropertyDescriptor(
AtRuleDescriptorID id,
CSSParserTokenRange& range,
const CSSParserContext& context) {
CSSValue* parsed_value = nullptr;
switch (id) {
case AtRuleDescriptorID::Syntax:
range.ConsumeWhitespace();
parsed_value = css_property_parser_helpers::ConsumeString(range);
break;
case AtRuleDescriptorID::InitialValue: {
// Note that we must retain leading whitespace here.
return CSSVariableParser::ParseDeclarationValue(
g_null_atom, range, false /* is_animation_tainted */, context);
}
case AtRuleDescriptorID::Inherits:
range.ConsumeWhitespace();
parsed_value =
css_property_parser_helpers::ConsumeIdent<CSSValueID::kTrue,
CSSValueID::kFalse>(range);
break;
default:
break;
}

if (!parsed_value || !range.AtEnd())
return nullptr;

return parsed_value;
}

bool AtRuleDescriptorParser::ParseAtRule(
AtRuleDescriptorID id,
CSSParserTokenRange& range,
const CSSParserContext& context,
HeapVector<CSSPropertyValue, 256>& parsed_descriptors) {
const CSSParserTokenRange original_range = range;

// TODO(meade): Handle other descriptor types here.
CSSValue* result =
AtRuleDescriptorParser::ParseFontFaceDescriptor(id, range, context);

if (!result) {
range = original_range;
result =
AtRuleDescriptorParser::ParseAtPropertyDescriptor(id, range, context);
}
if (!result)
return false;
// Convert to CSSPropertyID for legacy compatibility,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ class AtRuleDescriptorParser {
const CSSParserContext&);
static CSSValue* ParseFontFaceDeclaration(CSSParserTokenRange&,
const CSSParserContext&);
static CSSValue* ParseAtPropertyDescriptor(AtRuleDescriptorID,
CSSParserTokenRange&,
const CSSParserContext&);
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@
{
name: "height",
},
{
name: "inherits"
},
{
name: "initial-value"
},
{
name: "max-height",
},
Expand All @@ -63,6 +69,9 @@
{
name: "src",
},
{
name: "syntax"
},
{
name: "unicode-range",
},
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/css/parser/css_at_rule_id.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ CSSAtRuleID CssAtRuleID(StringView name) {
return kCSSAtRuleNamespace;
if (EqualIgnoringASCIICase(name, "page"))
return kCSSAtRulePage;
if (EqualIgnoringASCIICase(name, "property"))
return kCSSAtRuleProperty;
if (EqualIgnoringASCIICase(name, "supports"))
return kCSSAtRuleSupports;
if (EqualIgnoringASCIICase(name, "viewport"))
Expand Down Expand Up @@ -63,6 +65,9 @@ void CountAtRule(const CSSParserContext* context, CSSAtRuleID rule_id) {
case kCSSAtRulePage:
feature = WebFeature::kCSSAtRulePage;
break;
case kCSSAtRuleProperty:
feature = WebFeature::kCSSAtRuleProperty;
return;
case kCSSAtRuleSupports:
feature = WebFeature::kCSSAtRuleSupports;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ enum CSSAtRuleID {
kCSSAtRuleMedia,
kCSSAtRuleNamespace,
kCSSAtRulePage,
kCSSAtRuleProperty,
kCSSAtRuleSupports,
kCSSAtRuleViewport,
kCSSAtRuleWebkitKeyframes,
Expand Down
28 changes: 27 additions & 1 deletion third_party/blink/renderer/core/css/parser/css_parser_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,8 @@ StyleRuleBase* CSSParserImpl::ConsumeAtRule(CSSParserTokenStream& stream,
return ConsumeKeyframesRule(false, prelude, prelude_offset, stream);
case kCSSAtRulePage:
return ConsumePageRule(prelude, prelude_offset, stream);
case kCSSAtRuleProperty:
return ConsumePropertyRule(prelude, prelude_offset, stream);
default:
return nullptr; // Parse error, unrecognised at-rule with block
}
Expand Down Expand Up @@ -863,6 +865,30 @@ StyleRulePage* CSSParserImpl::ConsumePageRule(const CSSParserTokenRange prelude,
CreateCSSPropertyValueSet(parsed_properties_, context_->Mode()));
}

StyleRuleProperty* CSSParserImpl::ConsumePropertyRule(
CSSParserTokenRange prelude,
const RangeOffset& prelude_offset,
CSSParserTokenStream& block) {
if (!RuntimeEnabledFeatures::CSSVariables2AtPropertyEnabled())
return nullptr;

const CSSParserToken& name_token = prelude.ConsumeIncludingWhitespace();
if (!prelude.AtEnd())
return nullptr;
if (!CSSVariableParser::IsValidVariableName(name_token))
return nullptr;
String name = name_token.Value().ToString();

if (observer_) {
observer_->StartRuleHeader(StyleRule::kProperty, prelude_offset.start);
observer_->EndRuleHeader(prelude_offset.end);
}

ConsumeDeclarationList(block, StyleRule::kProperty);
return StyleRuleProperty::Create(
name, CreateCSSPropertyValueSet(parsed_properties_, context_->Mode()));
}

StyleRuleKeyframe* CSSParserImpl::ConsumeKeyframeStyleRule(
const CSSParserTokenRange prelude,
const RangeOffset& prelude_offset,
Expand Down Expand Up @@ -1019,7 +1045,7 @@ void CSSParserImpl::ConsumeDeclaration(CSSParserTokenRange range,

CSSPropertyID unresolved_property = CSSPropertyID::kInvalid;
AtRuleDescriptorID atrule_id = AtRuleDescriptorID::Invalid;
if (rule_type == StyleRule::kFontFace) {
if (rule_type == StyleRule::kFontFace || rule_type == StyleRule::kProperty) {
if (important) // Invalid
return;
atrule_id = lhs.ParseAsAtRuleDescriptorID();
Expand Down
Loading

0 comments on commit 2b9ff48

Please sign in to comment.