Skip to content

feat: optimize the performance of blink css #779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andycall merged 4 commits into from
Dec 28, 2025
Merged

feat: optimize the performance of blink css #779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fac5343
feat: use property and value id for the ui commands.
andycall Dec 28, 2025
5254bdb
fix: fix list view scroll controller multi disposed
andycall Dec 28, 2025
49fe230
feat: optimize perf by removing logs.
andycall Dec 28, 2025
cabcdb3
feat: use built-in static strings instead of allocate
andycall Dec 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 70 additions & 8 deletions bridge/core/css/blink_inline_style_validation_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "foundation/native_type.h"
#include "foundation/string/wtf_string.h"
#include "foundation/ui_command_buffer.h"
#include "code_gen/css_property_names.h"
#include "code_gen/css_value_keywords.h"
#include "webf_test_env.h"

using namespace webf;
Expand All @@ -31,23 +33,83 @@ std::string SharedNativeStringToUTF8(const SharedNativeString* s) {
return String(reinterpret_cast<const UChar*>(s->string()), static_cast<size_t>(s->length())).ToUTF8String();
}

std::string ConvertCamelCaseToKebabCase(const std::string& property_name) {
std::string result;
result.reserve(property_name.size() + 8);
for (unsigned char c : property_name) {
if (std::isupper(c)) {
result.push_back('-');
result.push_back(static_cast<char>(std::tolower(c)));
} else {
result.push_back(static_cast<char>(c));
}
}
return result;
}

bool HasSetStyleWithKeyValue(ExecutingContext* context, const std::string& key, const std::string& value) {
const CSSPropertyID expected_property_id = CssPropertyID(context, ConvertCamelCaseToKebabCase(key));
auto* pack = static_cast<UICommandBufferPack*>(context->uiCommandBuffer()->data());
auto* items = static_cast<UICommandItem*>(pack->data);
for (int64_t i = 0; i < pack->length; ++i) {
const UICommandItem& item = items[i];
if (item.type != static_cast<int32_t>(UICommand::kSetStyle)) {
if (item.type == static_cast<int32_t>(UICommand::kSetStyle)) {
if (CommandArg01ToUTF8(item) != key) {
continue;
}
auto* payload =
reinterpret_cast<NativeStyleValueWithHref*>(static_cast<uintptr_t>(item.nativePtr2));
if (!payload) {
continue;
}
if (SharedNativeStringToUTF8(payload->value) == value) {
return true;
}
continue;
}
if (CommandArg01ToUTF8(item) != key) {

if (item.type != static_cast<int32_t>(UICommand::kSetStyleById)) {
continue;
}
auto* payload =
reinterpret_cast<NativeStyleValueWithHref*>(static_cast<uintptr_t>(item.nativePtr2));
if (!payload) {
if (item.args_01_length != static_cast<int32_t>(expected_property_id)) {
continue;
}
if (SharedNativeStringToUTF8(payload->value) == value) {

std::string value_text;
if (item.string_01 < 0) {
value_text = getValueName(static_cast<CSSValueID>(-item.string_01 - 1));
} else {
auto* value_ptr = reinterpret_cast<SharedNativeString*>(static_cast<uintptr_t>(item.string_01));
value_text = SharedNativeStringToUTF8(value_ptr);
}
if (value_text == value) {
return true;
}
}
return false;
}

bool HasSetStyleByIdWithKeyValue(ExecutingContext* context, const std::string& key, const std::string& value) {
const CSSPropertyID expected_property_id = CssPropertyID(context, ConvertCamelCaseToKebabCase(key));
auto* pack = static_cast<UICommandBufferPack*>(context->uiCommandBuffer()->data());
auto* items = static_cast<UICommandItem*>(pack->data);
for (int64_t i = 0; i < pack->length; ++i) {
const UICommandItem& item = items[i];
if (item.type != static_cast<int32_t>(UICommand::kSetStyleById)) {
continue;
}
if (item.args_01_length != static_cast<int32_t>(expected_property_id)) {
continue;
}

std::string value_text;
if (item.string_01 < 0) {
value_text = getValueName(static_cast<CSSValueID>(-item.string_01 - 1));
} else {
auto* value_ptr = reinterpret_cast<SharedNativeString*>(static_cast<uintptr_t>(item.string_01));
value_text = SharedNativeStringToUTF8(value_ptr);
}
if (value_text == value) {
return true;
}
}
Expand Down Expand Up @@ -75,7 +137,7 @@ TEST(BlinkCSSStyleDeclarationValidation, RejectsInvalidFontSize) {
const char* set_valid = "document.body.style.fontSize = '18px';";
env->page()->evaluateScript(set_valid, strlen(set_valid), "vm://", 0);
TEST_runLoop(context);
EXPECT_TRUE(HasSetStyleWithKeyValue(context, "fontSize", "18px"));
EXPECT_TRUE(HasSetStyleByIdWithKeyValue(context, "fontSize", "18px"));

context->uiCommandBuffer()->clear();

Expand All @@ -84,6 +146,6 @@ TEST(BlinkCSSStyleDeclarationValidation, RejectsInvalidFontSize) {
const char* set_invalid = "document.body.style.fontSize = '-1px';";
env->page()->evaluateScript(set_invalid, strlen(set_invalid), "vm://", 0);
TEST_runLoop(context);
EXPECT_FALSE(HasSetStyleWithKeyValue(context, "fontSize", "-1px"));
EXPECT_FALSE(HasSetStyleByIdWithKeyValue(context, "fontSize", "-1px"));
EXPECT_EQ(errorCalled, false);
}
8 changes: 0 additions & 8 deletions bridge/core/css/element_rule_collector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,6 @@ void ElementRuleCollector::CollectMatchingRulesForList(
if (pseudo_element_id_ == kPseudoIdNone && match_result.dynamic_pseudo != kPseudoIdNone) {
continue;
}
WEBF_COND_LOG(COLLECTOR, VERBOSE) << "Author rule matched!";
const std::shared_ptr<StyleRule>& rule = rule_data->Rule();

String selector = rule->SelectorsText();
WEBF_COND_LOG(COLLECTOR, VERBOSE) << "SELECTOR: " << selector.Characters8();

String s = rule->Properties().AsText();
WEBF_COND_LOG(COLLECTOR, VERBOSE) << "RULE: " << s.Characters8();
DidMatchRule(rule_data, cascade_origin, cascade_layer, match_request);
}
}
Expand Down
8 changes: 5 additions & 3 deletions bridge/core/css/invalidation/pending_invalidations_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ TEST_F(PendingInvalidationsTest, DescendantInvalidationOnDisplayNone) {
MemberMutationScope mutation_scope{GetDocument().GetExecutingContext()};

ExceptionState exception_state;
GetDocument().body()->setInnerHTML(R"HTML(
constexpr const char kInnerHTML[] = R"HTML(
<style>
#a { display: none }
.a .b { color: green }
Expand All @@ -107,8 +107,10 @@ TEST_F(PendingInvalidationsTest, DescendantInvalidationOnDisplayNone) {
<div class="b"></div>
<div class="b"></div>
</div>
)HTML"_as,
exception_state);
)HTML";
static const AtomicString kInnerHTMLAtom =
AtomicString::CreateFromUTF8(kInnerHTML, sizeof(kInnerHTML) - 1);
GetDocument().body()->setInnerHTML(kInnerHTMLAtom, exception_state);
ASSERT_FALSE(exception_state.HasException());

GetDocument().UpdateStyleForThisDocument();
Expand Down
2 changes: 1 addition & 1 deletion bridge/core/css/properties/longhands/variable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Variable : public Longhand {
const AtomicString& GetPropertyNameAtomicString() const override {
// TODO(xiezuobing):
ExecutingContext* context;
static const AtomicString name = "variable"_as;
static const AtomicString name = AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1);
return name;
Comment on lines 30 to 33
Copy link

@coderabbitai coderabbitai bot Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove unused ExecutingContext* context to avoid warnings

ExecutingContext* context; is never used in GetPropertyNameAtomicString() and will trip -Wunused-variable (potentially as an error in stricter builds). It’s safe to remove.

Proposed diff 
-  const AtomicString& GetPropertyNameAtomicString() const override {
-    // TODO(xiezuobing):
-    ExecutingContext* context;
-    static const AtomicString name = AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1);
+  const AtomicString& GetPropertyNameAtomicString() const override {
+    // TODO(xiezuobing):
+    static const AtomicString name =
+        AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// TODO(xiezuobing):
ExecutingContext* context;
static const AtomicString name = "variable"_as;
static const AtomicString name = AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1);
return name;
const AtomicString& GetPropertyNameAtomicString() const override {
// TODO(xiezuobing):
static const AtomicString name =
AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1);
return name;
}
🤖 Prompt for AI Agents 
In bridge/core/css/properties/longhands/variable.h around lines 30 to 33, the
local variable `ExecutingContext* context;` is declared but never used in
GetPropertyNameAtomicString(), which triggers -Wunused-variable warnings; remove
this unused declaration from the function so the method only constructs and
returns the AtomicString without the dead variable.

}

Expand Down
65 changes: 4 additions & 61 deletions bridge/core/css/selector_checker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,8 @@ SelectorChecker::MatchStatus SelectorChecker::MatchSelector(const SelectorChecki
}
#endif
if (!is_covered_by_bucketing && !CheckOne(context, sub_result)) {
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] CheckOne FAIL for selector '"
<< context.selector->SimpleSelectorTextForDebug().ToUTF8String()
<< "' on element " << DescribeElementForLog(context.element);
return kSelectorFailsLocally;
}
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] CheckOne OK for selector '"
<< context.selector->SimpleSelectorTextForDebug().ToUTF8String()
<< "' on element " << DescribeElementForLog(context.element);
if (sub_result.dynamic_pseudo != kPseudoIdNone) {
result.dynamic_pseudo = sub_result.dynamic_pseudo;
result.custom_highlight_name = sub_result.custom_highlight_name;
Expand All @@ -265,14 +259,8 @@ SelectorChecker::MatchStatus SelectorChecker::MatchSelector(const SelectorChecki
// ::before could also match ::after rules when the pseudo selector is the
// terminal simple selector, leading to mixed pseudo styles.
if (context.pseudo_id != kPseudoIdNone && result.dynamic_pseudo != context.pseudo_id) {
WEBF_COND_LOG(SELECTOR, VERBOSE)
<< "[Selector] Terminal simple selector rejected due to pseudo mismatch. requested="
<< static_cast<int>(context.pseudo_id) << ", matched=" << static_cast<int>(result.dynamic_pseudo)
<< " for element " << DescribeElementForLog(context.element);
return kSelectorFailsCompletely;
}
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] Terminal simple selector matched for element "
<< DescribeElementForLog(context.element);
return kSelectorMatches;
}

Expand All @@ -291,13 +279,6 @@ SelectorChecker::MatchStatus SelectorChecker::MatchSelector(const SelectorChecki
return kSelectorFailsCompletely;
}
webf::AutoReset<PseudoId> dynamic_pseudo_scope(&result.dynamic_pseudo, kPseudoIdNone);
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] Proceeding to relation '" << static_cast<int>(context.selector->Relation())
<< "' for element " << DescribeElementForLog(context.element)
<< ", next selector: '"
<< (context.selector->NextSimpleSelector()
? context.selector->NextSimpleSelector()->SimpleSelectorTextForDebug().ToUTF8String()
: std::string("<null>"))
<< "'";
return MatchForRelation(context, result);
}
}
Expand Down Expand Up @@ -418,8 +399,6 @@ SelectorChecker::MatchStatus SelectorChecker::MatchForRelation(const SelectorChe
}
for (next_context.element = ParentElement(next_context); next_context.element;
next_context.element = ParentElement(next_context)) {
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] Descendant: try parent "
<< DescribeElementForLog(next_context.element);
MatchStatus match = MatchSelector(next_context, result);
if (match == kSelectorMatches || match == kSelectorFailsCompletely) {
return match;
Expand Down Expand Up @@ -454,14 +433,6 @@ SelectorChecker::MatchStatus SelectorChecker::MatchForRelation(const SelectorChe
}
}
next_context.element = ElementTraversal::PreviousSibling(*context.element);
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] DirectAdjacent: previous sibling of "
<< DescribeElementForLog(context.element) << " is "
<< DescribeElementForLog(next_context.element)
<< "; left compound: '"
<< (next_context.selector
? next_context.selector->SimpleSelectorTextForDebug().ToUTF8String()
: std::string("<null>"))
<< "'";
if (!next_context.element) {
return kSelectorFailsAllSiblings;
}
Expand All @@ -472,18 +443,12 @@ SelectorChecker::MatchStatus SelectorChecker::MatchForRelation(const SelectorChe
if (!left->NextSimpleSelector() && left->Match() == CSSSelector::kAttributeExact) {
const AtomicString& lname = left->Attribute().LocalName();
const AtomicString& expect = left->Value();
if (lname == "class"_as) {
if (lname == g_class_atom) {
AtomicString cls = next_context.element->className();
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] DirectAdjacent fast-path: compare class '" << cls.ToUTF8String()
<< "' vs expected '" << expect.ToUTF8String() << "'";
if (!cls.IsNull() && cls == expect) {
return kSelectorMatches;
}
} else if (lname == "id"_as) {
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] DirectAdjacent fast-path: compare id '"
<< (next_context.element->HasID() ? next_context.element->IdForStyleResolution().ToUTF8String()
: std::string("<none>"))
<< "' vs expected '" << expect.ToUTF8String() << "'";
} else if (lname == g_id_atom) {
if (next_context.element->HasID() && next_context.element->IdForStyleResolution() == expect) {
return kSelectorMatches;
}
Expand All @@ -503,8 +468,6 @@ SelectorChecker::MatchStatus SelectorChecker::MatchForRelation(const SelectorChe
}
next_context.element = ElementTraversal::PreviousSibling(*context.element);
for (; next_context.element; next_context.element = ElementTraversal::PreviousSibling(*next_context.element)) {
WEBF_COND_LOG(SELECTOR, VERBOSE) << "[Selector] IndirectAdjacent: try previous sibling "
<< DescribeElementForLog(next_context.element);
MatchStatus match = MatchSelector(next_context, result);
if (match == kSelectorMatches || match == kSelectorFailsAllSiblings || match == kSelectorFailsCompletely) {
return match;
Expand Down Expand Up @@ -759,11 +722,6 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
const QualifiedName& selector_attr = selector.Attribute();
// Should not be possible from the CSS grammar.
DCHECK_NE(selector_attr.LocalName(), CSSSelector::UniversalSelectorAtom());
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Try '[" << selector_attr.LocalName().ToUTF8String()
<< "]' match type " << static_cast<int>(match)
<< " ns='" << selector_attr.NamespaceURI().ToUTF8String() << "'"
<< " value='" << selector.Value().ToUTF8String() << "' on "
<< DescribeElementForLog(&element);
// Synchronize the attribute in case it is lazy-computed.
// Currently all lazy properties have a null namespace, so only pass
// localName().
Expand All @@ -776,11 +734,7 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
// Get attributes from element
AttributeCollection attributes = element.Attributes();
if (!attributes.IsEmpty()) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Scanning " << attributes.size() << " attribute(s)";
for (const auto& attribute_item : attributes) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Candidate '"
<< attribute_item.GetName().LocalName().ToUTF8String() << "'='"
<< attribute_item.Value().ToUTF8String() << "'";
if (!attribute_item.Matches(selector_attr)) {
if (element.IsHTMLElement()) {
continue;
Expand All @@ -798,7 +752,6 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
}
}
if (AttributeValueMatches(attribute_item, match, selector_value, case_sensitivity)) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Matched by AttributeCollection";
return true;
}
if (case_sensitivity == kTextCaseASCIIInsensitive) {
Expand All @@ -821,7 +774,6 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
return false;
}
// Legacy case insensitive match succeeded
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Matched by legacy ASCII-insensitive";
return true;
}
if (selector_attr.NamespaceURI() != g_star_atom) {
Expand All @@ -840,22 +792,19 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
ExceptionState exception_state; // No exception expected for attribute access.
if (match == CSSSelector::kAttributeSet) {
if (element.hasAttribute(local, exception_state)) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Matched by ElementAttributes presence";
return true;
}
} else {
AtomicString attr_value = element.getAttribute(local, exception_state);
if (!attr_value.IsNull()) {
Attribute fake(QualifiedName(g_null_atom, local, g_star_atom), attr_value);
if (AttributeValueMatches(fake, match, selector_value, case_sensitivity)) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Matched by ElementAttributes value";
return true;
}
// Legacy ASCII-insensitive fallback for HTML when needed.
if (element.IsHTMLElement() && !selector.IsCaseSensitiveAttribute() &&
AttributeValueMatches(fake, match, selector_value, kTextCaseASCIIInsensitive)) {
if (selector.AttributeMatch() != CSSSelector::AttributeMatchType::kCaseSensitiveAlways) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Matched by ElementAttributes legacy ASCII-insensitive";
return true;
}
}
Expand All @@ -870,13 +819,10 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
if (any_ns) {
const AtomicString& local = selector_attr.LocalName();
// id attribute
if (local == "id"_as) {
if (local == g_id_atom) {
if (!element.HasID()) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Fallback id: element has no id";
return false;
}
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Fallback id compare '" << element.id().ToUTF8String()
<< "' vs '" << selector_value.ToUTF8String() << "'";
if (match == CSSSelector::kAttributeExact) {
if (case_sensitivity == kTextCaseSensitive) {
return StringView(element.id()) == StringView(selector_value);
Expand All @@ -889,15 +835,12 @@ static bool AnyAttributeMatches(Element& element, CSSSelector::MatchType match,
}
}
// class attribute
if (local == "class"_as) {
if (local == g_class_atom) {
// className() reflects the current attribute text.
AtomicString class_text = element.className();
if (class_text.IsNull()) {
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Fallback class: className is null";
return false;
}
WEBF_COND_LOG(ATTR, VERBOSE) << "[Selector][Attr] Fallback class compare '" << class_text.ToUTF8String()
<< "' vs '" << selector_value.ToUTF8String() << "'";
if (match == CSSSelector::kAttributeExact) {
if (case_sensitivity == kTextCaseSensitive) {
return StringView(class_text) == StringView(selector_value);
Expand Down
Loading
Loading