feat: optimize the performance of blink css #779
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughIntroduces ID-based CSS style setting via a new Changes
Sequence Diagram(s)sequenceDiagram
participant Blink as Blink<br/>(C++)
participant CmdBuf as UICommand<br/>Buffer
participant Dart as Dart<br/>VM
participant View as View/Widget
Note over Blink,View: New ID-Based Style Setting Path
Blink->>Blink: Emit CSS style (custom property or regular)
Blink->>Blink: Compute value_slot<br/>(from identifier ID or string)
Blink->>CmdBuf: AddStyleByIdCommand(property_id,<br/>value_slot, base_href)
CmdBuf->>CmdBuf: Create UICommandItem<br/>type: kSetStyleById
CmdBuf->>CmdBuf: Encode: args_01=property_id,<br/>string_01=value_slot
alt Dedicated Context
CmdBuf->>CmdBuf: RecordStyleByIdCommand<br/>(item, ui_update)
else Non-Dedicated
CmdBuf->>CmdBuf: Forward to<br/>read_buffer_
end
CmdBuf->>Dart: FFI: kSetStyleById command
Dart->>Dart: Extract stylePropertyId,<br/>styleValueSlot
Dart->>Dart: Resolve property name<br/>via blinkCSSPropertyIdToStyleName[id]
Dart->>Dart: Resolve value<br/>negative slot → keyword via<br/>blinkCSSValueIdToKeyword,<br/>positive slot → NativeString
View->>View: setInlineStyle(key, value,<br/>baseHref)
View->>View: Update widget styling
Note over Blink,View: Replaces Direct Payload Path
Note over CmdBuf,Dart: Reduces FFI overhead via ID encoding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
bridge/core/css/invalidation/pending_invalidations_test.cc (1)
101-113: LGTM! Optimization aligns with PR objectives.The change correctly replaces per-run AtomicString allocation with a static instance, reducing overhead. The use of
constexpr const char[]andsizeof(kInnerHTML) - 1is correct, and static local initialization is thread-safe in C++11+.Consider applying the same pattern to line 54 for consistency:
- GetDocument().body()->setInnerHTML( - AtomicString::CreateFromUTF8("<div id='d'></div><i id='i'></i><span></span>"), exception_state); + constexpr const char kInnerHTML[] = "<div id='d'></div><i id='i'></i><span></span>"; + static const AtomicString kInnerHTMLAtom = + AtomicString::CreateFromUTF8(kInnerHTML, sizeof(kInnerHTML) - 1); + GetDocument().body()->setInnerHTML(kInnerHTMLAtom, exception_state);This would unify the approach across both tests, though the performance impact is minimal since line 54's string is shorter and the first test may have different optimization priorities.
bridge/foundation/ui_command_ring_buffer.h (1)
104-111: Consider assertingtypematchesitem.typeinAddCommandItem
AddCommandItemaccepts both aUICommandItemand a separateUICommand type. Since the implementation relies ontypeforShouldSplit(type)anditem.typefor the package contents, a mismatch would be hard to diagnose.Adding a debug assertion that
static_cast<int32_t>(type) == item.type(or derivingtypefromitem.typeinternally) would make the API safer for future callers.Illustrative change
- void AddCommandItem(const UICommandItem& item, UICommand type, bool request_ui_update = true); + void AddCommandItem(const UICommandItem& item, UICommand type, bool request_ui_update = true);And in the .cc implementation:
void UICommandPackageRingBuffer::AddCommandItem(const UICommandItem& item, UICommand type, bool request_ui_update) { std::lock_guard<std::mutex> lock(current_package_mutex_); + +#if DCHECK_IS_ON() + DCHECK_EQ(static_cast<int32_t>(type), item.type); +#endifbridge/foundation/ui_command_buffer.h (1)
100-110:AddStyleByIdCommandencoding is clear; ensure it’s classified as a style updateThe documented encoding (
args_01_length= property id,value_slotas either pointer or negative CSSValueID,nativePtr2as optional base href) gives a compact representation for style-by-id commands and matches the performance goal.Just double-check that
GetKindFromUICommand(UICommand::kSetStyleById)returnskStyleUpdatein the .cc implementation so these commands are tracked and batched consistently withkSetStyle/kSetPseudoStyle.bridge/foundation/shared_ui_command.h (1)
9-10: Typo in include guard:DOUBULEshould beDOUBLE.The include guard uses
MULTI_THREADING_DOUBULE_UI_COMMAND_H_which contains a typo. Consider fixing for consistency.🔎 Proposed fix
-#ifndef MULTI_THREADING_DOUBULE_UI_COMMAND_H_ -#define MULTI_THREADING_DOUBULE_UI_COMMAND_H_ +#ifndef MULTI_THREADING_DOUBLE_UI_COMMAND_H_ +#define MULTI_THREADING_DOUBLE_UI_COMMAND_H_And at the end:
-#endif // MULTI_THREADING_DOUBULE_UI_COMMAND_H_ +#endif // MULTI_THREADING_DOUBLE_UI_COMMAND_H_Also applies to: 81-81
bridge/foundation/ui_command_ring_buffer.cc (1)
272-285: LGTM!AddCommandItemfor pre-built UICommandItem support.The implementation correctly mirrors
AddCommandbut accepts a pre-built item. This enables the style-by-id command flow where items are constructed with custom field mappings.The logic in
AddCommandItem(lines 272-285) closely mirrorsAddCommand(lines 249-270). Consider extracting the common flow into a helper to reduce duplication:🔎 Optional refactor to reduce duplication
+void UICommandPackageRingBuffer::AddCommandInternal(const UICommandItem& item, UICommand type, bool request_ui_update) { + std::lock_guard<std::mutex> lock(current_package_mutex_); + + if (!current_package_->commands.empty() && current_package_->ShouldSplit(type)) { + FlushCurrentPackage(); + } + + current_package_->AddCommand(item); + + if (type == UICommand::kFinishRecordingCommand || type == UICommand::kAsyncCaller + || type == UICommand::kRequestAnimationFrame || type == UICommand::kRequestCanvasPaint) { + FlushCurrentPackage(); + } +} + void UICommandPackageRingBuffer::AddCommand(UICommand type, ...) { UICommandItem item(static_cast<int32_t>(type), args_01, native_binding_object, nativePtr2); - std::lock_guard<std::mutex> lock(current_package_mutex_); - // ... duplicated logic ... + AddCommandInternal(item, type, request_ui_update); } void UICommandPackageRingBuffer::AddCommandItem(const UICommandItem& item, UICommand type, bool request_ui_update) { - std::lock_guard<std::mutex> lock(current_package_mutex_); - // ... duplicated logic ... + AddCommandInternal(item, type, request_ui_update); }bridge/core/css/style_engine.cc (1)
1739-1773: Consider extracting duplicated white-space emission logic.The white-space shorthand emission logic at lines 1739-1773 is nearly identical to lines 1376-1412 in
RecalcStyleForSubtree. While not blocking, extracting this into a helper function would reduce code duplication and maintenance burden.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
bridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/element_rule_collector.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/properties/longhands/variable.hbridge/core/css/selector_checker.ccbridge/core/css/style_engine.ccbridge/core/dom/element.ccbridge/foundation/logging.hbridge/foundation/native_type.hbridge/foundation/shared_ui_command.ccbridge/foundation/shared_ui_command.hbridge/foundation/string/atomic_string.hbridge/foundation/string/string_impl_unittest.ccbridge/foundation/ui_command_buffer.ccbridge/foundation/ui_command_buffer.hbridge/foundation/ui_command_ring_buffer.ccbridge/foundation/ui_command_ring_buffer.hbridge/foundation/ui_command_strategy.ccbridge/foundation/ui_command_strategy.hbridge/scripts/code_generator/bin/code_generator.jsbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tsscripts/tasks.jswebf/lib/src/bridge/code_gen/blink_css_ids.dartwebf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/ui_command.dartwebf/lib/src/dom/element_widget_adapter.dart
💤 Files with no reviewable changes (3)
- bridge/foundation/logging.h
- bridge/core/css/element_rule_collector.cc
- bridge/foundation/string/atomic_string.h
🧰 Additional context used
📓 Path-based instructions (9)
bridge/**/*.{cc,cpp,h,hpp}
📄 CodeRabbit inference engine (bridge/CLAUDE.md)
bridge/**/*.{cc,cpp,h,hpp}: C++ code should follow Chromium style (.clang-format) with C++17 standard, 120 character column limit, and 2-space indentation
UseWEBF_EXPORT_Cmacro for exporting C functions to Dart FFI
In FFI contexts, useDart_Handleinstead ofHandlefor type compatibility
For C++ FFI function naming: useGetObjectPropertiesFromDartfor C++ exports,NativeGetObjectPropertiesFuncfor Dart typedefs, andGetObjectPropertiesFuncfor Dart functions
Lambda signatures in C++ must match expected function signatures to avoid FFI type mismatches
Choose power-of-2 buffer sizes (512, 1024, 2048) for ring buffer implementation, with smaller sizes for DEBUG_BUILD, MOBILE constraints, and larger sizes for DESKTOP performance
Files:
bridge/foundation/ui_command_buffer.hbridge/foundation/shared_ui_command.hbridge/core/css/selector_checker.ccbridge/foundation/shared_ui_command.ccbridge/foundation/ui_command_ring_buffer.hbridge/core/css/invalidation/pending_invalidations_test.ccbridge/foundation/native_type.hbridge/foundation/ui_command_ring_buffer.ccbridge/foundation/ui_command_strategy.hbridge/foundation/string/string_impl_unittest.ccbridge/core/css/blink_inline_style_validation_test.ccbridge/foundation/ui_command_buffer.ccbridge/core/css/style_engine.ccbridge/foundation/ui_command_strategy.ccbridge/core/dom/element.ccbridge/core/css/properties/longhands/variable.h
bridge/**/*.{h,hpp}
📄 CodeRabbit inference engine (bridge/CLAUDE.md)
bridge/**/*.{h,hpp}: Ring buffer command structure should useenum Type : uint8_tfor command types with union for type-specific data to ensure type-safe and cache-friendly command handling
Ring buffer implementation should usealignas(64)for atomic head and tail pointers,std::atomic<size_t>for thread-safe synchronization, and power-of-2 buffer sizes enforced withstatic_assert
Files:
bridge/foundation/ui_command_buffer.hbridge/foundation/shared_ui_command.hbridge/foundation/ui_command_ring_buffer.hbridge/foundation/native_type.hbridge/foundation/ui_command_strategy.hbridge/core/css/properties/longhands/variable.h
bridge/**/*.{cc,h}
📄 CodeRabbit inference engine (AGENTS.md)
C++ code in bridge module must use C++17 standard with 2-space indentation, 120 column limit, and follow Chromium style guide as defined in
.clang-format
bridge/**/*.{cc,h}: Use RAII patterns in C++ where possible for resource management
UsePostToJsfor executing operations on the JS thread in FFI
UsePostToDartfor returning results to Dart isolate
AvoidPostToJsSyncsynchronous execution when possibleC++ bridge code must use 2-space indent, 120 column line limit, and follow Chromium style via
.clang-format
Files:
bridge/foundation/ui_command_buffer.hbridge/foundation/shared_ui_command.hbridge/core/css/selector_checker.ccbridge/foundation/shared_ui_command.ccbridge/foundation/ui_command_ring_buffer.hbridge/core/css/invalidation/pending_invalidations_test.ccbridge/foundation/native_type.hbridge/foundation/ui_command_ring_buffer.ccbridge/foundation/ui_command_strategy.hbridge/foundation/string/string_impl_unittest.ccbridge/core/css/blink_inline_style_validation_test.ccbridge/foundation/ui_command_buffer.ccbridge/core/css/style_engine.ccbridge/foundation/ui_command_strategy.ccbridge/core/dom/element.ccbridge/core/css/properties/longhands/variable.h
{bridge/**/*.{cc,h},webf/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Document memory ownership clearly in FFI implementations
Files:
bridge/foundation/ui_command_buffer.hbridge/foundation/shared_ui_command.hbridge/core/css/selector_checker.ccbridge/foundation/shared_ui_command.ccwebf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/to_native.dartbridge/foundation/ui_command_ring_buffer.hbridge/core/css/invalidation/pending_invalidations_test.ccwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.hbridge/foundation/ui_command_ring_buffer.ccwebf/lib/src/bridge/ui_command.dartbridge/foundation/ui_command_strategy.hbridge/foundation/string/string_impl_unittest.ccwebf/lib/src/dom/element_widget_adapter.dartbridge/core/css/blink_inline_style_validation_test.ccbridge/foundation/ui_command_buffer.ccbridge/core/css/style_engine.ccbridge/foundation/ui_command_strategy.ccbridge/core/dom/element.ccbridge/core/css/properties/longhands/variable.h
bridge/**/*.{cc,cpp}
📄 CodeRabbit inference engine (bridge/CLAUDE.md)
bridge/**/*.{cc,cpp}: For async FFI operations, useDart_NewPersistentHandle_DLto keep Dart objects alive, convert back withDart_HandleFromPersistent_DLbefore use, and always callDart_DeletePersistentHandle_DLafter the async operation completes
For string handling in FFI, copy strings that might be freed usingstd::string(const_char_ptr), and usetoNativeUtf8()with proper memory deallocation
For async callbacks with potential errors, always provide error path in callbacks (e.g.,callback(object, nullptr)), handle cancellation cases in async operations, and propagate errors through callback parameters rather than exceptions
For thread-safe error reporting in FFI, copy error messages before crossing thread boundaries usingstd::stringto ensure string lifetime, and consider error callbacks separate from result callbacks
AvoidPostToJsSyncwhen threads may interdepend to prevent synchronous deadlocks in FFI communication
Ensure callback functions aren't invoked after context destruction to prevent use-after-free errors in FFI async operations
Implement ring buffer overflow handling with metrics and alerts to monitor command buffer capacity
Process multiple UI commands per frame in a loop with a MAX_COMMANDS_PER_FRAME limit to balance responsiveness and performance
Pin threads to cores for optimal cache usage in ring buffer operations by setting CPU affinity for UI threads
Use PostToJs for executing operations on the JS thread from other threads, PostToDart for returning results to Dart isolate, and avoid PostToJsSync to prevent deadlocks
Files:
bridge/core/css/selector_checker.ccbridge/foundation/shared_ui_command.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/foundation/ui_command_ring_buffer.ccbridge/foundation/string/string_impl_unittest.ccbridge/core/css/blink_inline_style_validation_test.ccbridge/foundation/ui_command_buffer.ccbridge/core/css/style_engine.ccbridge/foundation/ui_command_strategy.ccbridge/core/dom/element.cc
bridge/core/**
📄 CodeRabbit inference engine (bridge/AGENTS.md)
Core bridge code should be located in
bridge/core/directory
Files:
bridge/core/css/selector_checker.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/style_engine.ccbridge/core/dom/element.ccbridge/core/css/properties/longhands/variable.h
scripts/**/tasks.js
📄 CodeRabbit inference engine (scripts/CLAUDE.md)
scripts/**/tasks.js: Add global declarations fordocumentandwindowin themerge-bridge-typingstask defined intasks.js
Display task-level errors in type generation with specific error messages, build tool output, and parsing errors including file paths and line numbers
Files:
scripts/tasks.js
scripts/**/*.{js,json}
📄 CodeRabbit inference engine (CLAUDE.md)
scripts/**/*.{js,json}: Bundle QuickJS into the WebF library for Android builds by default using static STL
Use separate QuickJS library only in advanced scenarios when sharing QuickJS with other libraries
SetANDROID_STL=c++_staticas the default C++ standard library for Android builds
Files:
scripts/tasks.js
webf/**/*.dart
📄 CodeRabbit inference engine (webf/CLAUDE.md)
webf/**/*.dart: Follow rules in webf/analysis_options.yaml for Dart code style
Use single quotes for strings in Dart code
File names must use snake_case in Dart
Class names must use PascalCase in Dart
Variables and functions must use camelCase in Dart
Prefer final fields when applicable in Dart code
Lines should be max 120 characters in Dart code
Always free allocated memory in Dart FFI using malloc.free() for toNativeUtf8() allocations
Free FFI allocated memory in finally blocks to ensure cleanup on exceptions
Track ownership of allocated pointers in FFI callbacks
Free NativeValue pointers after converting with fromNativeValue in FFI code
Document memory ownership clearly in FFI async callbacks
Implement WidgetElement to create custom Flutter widgets integrated into WebF's DOM tree
Register custom WidgetElements using WidgetElementRegistry.register(tagName, builder)
Override buildWidget(BuildContext context) method in WidgetElement to build the Flutter widget
Call updateWidget() when attributes change in WidgetElement implementations
Dispose controllers and subscriptions in WidgetElement for memory management
Follow W3C event standards when dispatching events from WidgetElement
Minimize widget rebuilds in WidgetElement for performance optimization
Implement ARIA attributes in WidgetElement when applicable for accessibilityDart code in webf module must follow naming conventions: snake_case for file names, PascalCase for classes, and camelCase for class members
webf/**/*.dart: Always free allocated memory in Dart FFI
Usemalloc.free()fortoNativeUtf8()allocations in Dart FFI
Track pointer ownership in callbacks within Dart FFI
Files:
webf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartwebf/lib/src/bridge/ui_command.dartwebf/lib/src/dom/element_widget_adapter.dart
🧠 Learnings (47)
📓 Common learnings
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/lib/src/css/**/*.dart : Use CSSRenderStyle for style computation and storage in Dart CSS code
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp} : Process multiple UI commands per frame in a loop with a MAX_COMMANDS_PER_FRAME limit to balance responsiveness and performance
Applied to files:
bridge/foundation/ui_command_buffer.hbridge/foundation/shared_ui_command.hbridge/foundation/shared_ui_command.ccbridge/foundation/ui_command_ring_buffer.hbridge/foundation/ui_command_ring_buffer.ccbridge/foundation/ui_command_strategy.hbridge/foundation/ui_command_buffer.ccbridge/foundation/ui_command_strategy.cc
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: No build needed for Dart-only changes in webf/
Applied to files:
scripts/tasks.jswebf/lib/src/bridge/native_types.dartbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/scripts/code_generator/bin/code_generator.jswebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/bridge/core/**/*.d.ts : Scan all `.d.ts` files in `bridge/core/` directory and merge them into a single `bridge/typings/webf.d.ts` file using the `merge-bridge-typings` task
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/bridge/core/**/*.d.ts : Transform WebF-specific type annotations: `DartImpl<T>` → `T`, `StaticMember<T>` → `T`, `StaticMethod<T>` → `T`, `SupportAsync<T>` → generates both sync and async variants, `DependentsOnLayout<T>` → `T`
Applied to files:
scripts/tasks.jswebf/lib/src/bridge/native_types.dartbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/scripts/code_generator/bin/code_generator.jswebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/generate_binding_code.js : Run `node scripts/generate_binding_code.js` to regenerate all types in the series: merge-bridge-typings → update-typings-version → generate-bindings-code
Applied to files:
scripts/tasks.jsbridge/scripts/code_generator/bin/code_generator.js
📚 Learning: 2025-12-21T14:42:52.637Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.637Z
Learning: Applies to bridge/code_gen/** : Do not hand-edit files under `bridge/code_gen/` as they are generated by `npm run bindgen`
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/bridge/core/**/*.d.ts : DO NOT manually edit `bridge/typings/webf.d.ts` - it is generated from bridge/core/*.d.ts
Applied to files:
scripts/tasks.jsbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/code_gen/blink_css_ids.dart
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/generate_polyfill_bytecode.js : Run `cd bridge/typings && npm run generate` to generate only polyfill types using the pipeline defined in generate_polyfill_bytecode.js
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-13T16:32:47.644Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T16:32:47.644Z
Learning: Applies to bridge/**/*.{cc,h} : Use `PostToDart` for returning results to Dart isolate
Applied to files:
scripts/tasks.jsbridge/foundation/native_type.h
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/**/tasks.js : Display task-level errors in type generation with specific error messages, build tool output, and parsing errors including file paths and line numbers
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/**/tasks.js : Add global declarations for `document` and `window` in the `merge-bridge-typings` task defined in `tasks.js`
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-08T23:28:11.651Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:28:11.651Z
Learning: Generate bindings and types before commits by running `node scripts/generate_binding_code.js`
Applied to files:
scripts/tasks.js
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/lib/bridge.dart : lib/bridge.dart contains FFI bindings to C++ bridge
Applied to files:
webf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.hwebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp,h,hpp} : For C++ FFI function naming: use `GetObjectPropertiesFromDart` for C++ exports, `NativeGetObjectPropertiesFunc` for Dart typedefs, and `GetObjectPropertiesFunc` for Dart functions
Applied to files:
webf/lib/src/bridge/native_types.dartbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.hwebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Free NativeValue pointers after converting with fromNativeValue in FFI code
Applied to files:
webf/lib/src/bridge/native_types.dartbridge/foundation/native_type.h
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp,h,hpp} : Use `WEBF_EXPORT_C` macro for exporting C functions to Dart FFI
Applied to files:
webf/lib/src/bridge/native_types.dartbridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.hwebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp,h,hpp} : In FFI contexts, use `Dart_Handle` instead of `Handle` for type compatibility
Applied to files:
webf/lib/src/bridge/native_types.dartbridge/foundation/native_type.h
📚 Learning: 2025-12-13T16:32:47.644Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T16:32:47.644Z
Learning: Applies to {bridge/**/*.{cc,h},webf/**/*.dart} : Document memory ownership clearly in FFI implementations
Applied to files:
webf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.h
📚 Learning: 2025-12-21T14:42:52.637Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.637Z
Learning: Applies to bridge/webf/**/*.dart : Dart code in webf must follow `webf/analysis_options.yaml` with files named in `snake_case.dart`, types in `PascalCase`, and members in `camelCase`
Applied to files:
webf/lib/src/bridge/native_types.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartwebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-13T16:32:47.644Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T16:32:47.644Z
Learning: Applies to webf/**/*.dart : Use `malloc.free()` for `toNativeUtf8()` allocations in Dart FFI
Applied to files:
webf/lib/src/bridge/native_types.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/lib/src/css/**/*.dart : Use CSSRenderStyle for style computation and storage in Dart CSS code
Applied to files:
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/foundation/native_type.hbridge/scripts/code_generator/bin/code_generator.jswebf/lib/src/bridge/ui_command.dartbridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/style_engine.ccbridge/core/dom/element.cc
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: Applies to scripts/bridge/polyfill/**/*.{ts,tsx} : DO NOT manually edit `bridge/typings/polyfill.d.ts` - it is generated from polyfill TypeScript source
Applied to files:
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.ts
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Use single quotes for strings in Dart code
Applied to files:
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.ts
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Follow rules in webf/analysis_options.yaml for Dart code style
Applied to files:
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.tswebf/lib/src/bridge/to_native.dartwebf/lib/src/bridge/code_gen/blink_css_ids.dartbridge/scripts/code_generator/bin/code_generator.jswebf/lib/src/bridge/ui_command.dart
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{h,hpp} : Ring buffer command structure should use `enum Type : uint8_t` for command types with union for type-specific data to ensure type-safe and cache-friendly command handling
Applied to files:
bridge/foundation/ui_command_ring_buffer.hbridge/foundation/ui_command_ring_buffer.cc
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp} : Implement ring buffer overflow handling with metrics and alerts to monitor command buffer capacity
Applied to files:
bridge/foundation/ui_command_ring_buffer.hbridge/foundation/ui_command_ring_buffer.cc
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp,h,hpp} : Choose power-of-2 buffer sizes (512, 1024, 2048) for ring buffer implementation, with smaller sizes for DEBUG_BUILD, MOBILE constraints, and larger sizes for DESKTOP performance
Applied to files:
bridge/foundation/ui_command_ring_buffer.h
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{h,hpp} : Ring buffer implementation should use `alignas(64)` for atomic head and tail pointers, `std::atomic<size_t>` for thread-safe synchronization, and power-of-2 buffer sizes enforced with `static_assert`
Applied to files:
bridge/foundation/ui_command_ring_buffer.h
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp} : For async FFI operations, use `Dart_NewPersistentHandle_DL` to keep Dart objects alive, convert back with `Dart_HandleFromPersistent_DL` before use, and always call `Dart_DeletePersistentHandle_DL` after the async operation completes
Applied to files:
bridge/foundation/native_type.h
📚 Learning: 2025-12-08T23:27:41.357Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: scripts/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:41.357Z
Learning: The WebF type generation system is composed of interconnected tasks that generate TypeScript definitions from multiple sources in a coordinated pipeline
Applied to files:
bridge/scripts/code_generator/bin/code_generator.js
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp} : For string handling in FFI, copy strings that might be freed using `std::string(const_char_ptr)`, and use `toNativeUtf8()` with proper memory deallocation
Applied to files:
bridge/foundation/string/string_impl_unittest.cc
📚 Learning: 2025-12-21T14:42:52.637Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.637Z
Learning: Applies to bridge/test/**/*.{cc,h} : Bridge unit tests must use GoogleTest framework, keep tests small and behavior-focused, and mirror source paths under `bridge/test/`
Applied to files:
bridge/foundation/string/string_impl_unittest.cc
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Dispose controllers and subscriptions in WidgetElement for memory management
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Minimize widget rebuilds in WidgetElement for performance optimization
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Call updateWidget() when attributes change in WidgetElement implementations
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Implement ARIA attributes in WidgetElement when applicable for accessibility
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Implement WidgetElement to create custom Flutter widgets integrated into WebF's DOM tree
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Follow W3C event standards when dispatching events from WidgetElement
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-13T16:32:47.644Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T16:32:47.644Z
Learning: Applies to webf/**/*.dart : Track pointer ownership in callbacks within Dart FFI
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Prefer final fields when applicable in Dart code
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Track ownership of allocated pointers in FFI callbacks
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: Applies to webf/**/*.dart : Document memory ownership clearly in FFI async callbacks
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:27:15.946Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:15.946Z
Learning: Applies to bridge/**/*.{cc,cpp,h,hpp} : C++ code should follow Chromium style (.clang-format) with C++17 standard, 120 character column limit, and 2-space indentation
Applied to files:
bridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/style_engine.cc
📚 Learning: 2025-12-21T14:42:52.637Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.637Z
Learning: Applies to bridge/**/*.{cc,h} : C++ bridge code must use 2-space indent, 120 column line limit, and follow Chromium style via `.clang-format`
Applied to files:
bridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/style_engine.cc
📚 Learning: 2025-12-08T23:28:11.651Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T23:28:11.651Z
Learning: Applies to bridge/**/*.{cc,h} : C++ code in bridge module must use C++17 standard with 2-space indentation, 120 column limit, and follow Chromium style guide as defined in `.clang-format`
Applied to files:
bridge/core/css/blink_inline_style_validation_test.ccbridge/core/css/style_engine.cc
📚 Learning: 2025-12-08T23:28:00.818Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: webf/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:28:00.818Z
Learning: CSS property implementations are located in lib/src/css/
Applied to files:
bridge/core/css/style_engine.ccbridge/core/dom/element.cc
🧬 Code graph analysis (16)
bridge/foundation/shared_ui_command.h (3)
bridge/foundation/shared_ui_command.cc (2)
AddStyleByIdCommand(74-92)AddStyleByIdCommand(74-78)bridge/foundation/ui_command_buffer.cc (2)
AddStyleByIdCommand(84-97)AddStyleByIdCommand(84-88)bridge/foundation/native_type.h (1)
property_id(97-97)
bridge/foundation/shared_ui_command.cc (5)
bridge/foundation/ui_command_buffer.cc (2)
AddStyleByIdCommand(84-97)AddStyleByIdCommand(84-88)bridge/foundation/native_type.h (1)
property_id(97-97)bridge/foundation/ui_command_ring_buffer.cc (1)
item(254-254)bridge/foundation/ui_command_strategy.h (1)
item(68-68)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.ts (1)
bridge/scripts/code_generator/src/json/css_properties.ts (1)
CSSProperties(279-674)
bridge/foundation/ui_command_ring_buffer.h (3)
bridge/foundation/ui_command_ring_buffer.cc (3)
AddCommandItem(272-285)AddCommandItem(272-272)item(254-254)bridge/foundation/ui_command_strategy.h (1)
item(68-68)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/core/css/invalidation/pending_invalidations_test.cc (1)
bridge/core/css/style_engine.cc (2)
GetDocument(647-649)GetDocument(647-647)
bridge/foundation/native_type.h (1)
bridge/foundation/native_value.h (1)
public(56-64)
bridge/foundation/ui_command_ring_buffer.cc (2)
bridge/foundation/ui_command_strategy.h (1)
item(68-68)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/scripts/code_generator/bin/code_generator.js (1)
bridge/scripts/code_generator/src/json/make_dart_blink_css_ids.ts (1)
makeDartBlinkCssIds(45-92)
bridge/foundation/ui_command_strategy.h (3)
bridge/foundation/ui_command_strategy.cc (2)
RecordStyleByIdCommand(53-63)RecordStyleByIdCommand(53-53)bridge/foundation/ui_command_ring_buffer.cc (1)
item(254-254)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/foundation/string/string_impl_unittest.cc (2)
bridge/foundation/string/string_impl.cc (4)
CreateFromUTF8(465-470)CreateFromUTF8(465-465)CreateFromUTF8(472-513)CreateFromUTF8(472-472)bridge/foundation/string/atomic_string.cc (4)
CreateFromUTF8(103-107)CreateFromUTF8(103-103)CreateFromUTF8(108-110)CreateFromUTF8(108-108)
bridge/core/css/blink_inline_style_validation_test.cc (3)
bridge/foundation/native_type.h (3)
value(74-75)value(107-111)items(79-81)bridge/foundation/ui_command_ring_buffer.cc (1)
item(254-254)bridge/foundation/ui_command_strategy.h (1)
item(68-68)
bridge/foundation/ui_command_buffer.cc (5)
bridge/foundation/shared_ui_command.cc (2)
AddStyleByIdCommand(74-92)AddStyleByIdCommand(74-78)bridge/foundation/native_type.h (1)
property_id(97-97)bridge/foundation/ui_command_ring_buffer.cc (1)
item(254-254)bridge/foundation/ui_command_strategy.h (1)
item(68-68)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/core/css/style_engine.cc (1)
bridge/bindings/qjs/native_string_utils.cc (4)
stringToNativeString(45-50)stringToNativeString(45-45)stringToNativeString(52-70)stringToNativeString(52-52)
bridge/foundation/ui_command_strategy.cc (3)
bridge/foundation/ui_command_ring_buffer.cc (1)
item(254-254)bridge/foundation/ui_command_strategy.h (1)
item(68-68)bridge/foundation/ui_command_ring_buffer_test.cc (1)
item(106-106)
bridge/core/dom/element.cc (2)
bridge/core/css/style_engine.cc (2)
id(84-84)id(84-84)bridge/bindings/qjs/native_string_utils.cc (4)
stringToNativeString(45-50)stringToNativeString(45-45)stringToNativeString(52-70)stringToNativeString(52-52)
bridge/core/css/properties/longhands/variable.h (1)
bridge/foundation/string/atomic_string.h (4)
AtomicString(23-157)AtomicString(37-38)AtomicString(42-42)AtomicString(47-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build_windows_bridge (Debug)
- GitHub Check: build_bridge
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Analyze (javascript-typescript)
| // TODO(xiezuobing): | ||
| ExecutingContext* context; | ||
| static const AtomicString name = "variable"_as; | ||
| static const AtomicString name = AtomicString::CreateFromUTF8("variable", sizeof("variable") - 1); | ||
| return name; |
There was a problem hiding this comment.
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.
| // 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.
| // Custom properties require sending the property name across the bridge. | ||
| if (id == CSSPropertyID::kVariable) { | ||
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | ||
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | ||
| if (value_string.IsNull()) value_string = String(""); | ||
| if (value_string.IsEmpty()) { | ||
| value_string = String(" "); | ||
| } | ||
|
|
||
| // Skip white-space longhands; will emit shorthand later | ||
| if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) { | ||
| continue; | ||
| } | ||
|
|
||
| // Already cleared above. | ||
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | ||
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | ||
| payload->value = stringToNativeString(value_string).release(); | ||
| if (!base_href_string.IsEmpty()) { | ||
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | ||
| } else { | ||
| payload->href = nullptr; | ||
| } | ||
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), element->bindingObject(), | ||
| payload); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Dead code detected inside kVariable block.
Lines 1329-1331 contain a check for kWhiteSpaceCollapse or kTextWrap, but this code is unreachable because we're already inside the if (id == CSSPropertyID::kVariable) block. A property cannot simultaneously be kVariable and kWhiteSpaceCollapse/kTextWrap.
🔎 Proposed fix to remove dead code
if (value_string.IsEmpty()) {
value_string = String(" ");
}
- // Skip white-space longhands; will emit shorthand later
- if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) {
- continue;
- }
-
// Already cleared above.
auto key_ns = prop_name.ToStylePropertyNameNativeString();📝 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.
| // Custom properties require sending the property name across the bridge. | |
| if (id == CSSPropertyID::kVariable) { | |
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | |
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | |
| if (value_string.IsNull()) value_string = String(""); | |
| if (value_string.IsEmpty()) { | |
| value_string = String(" "); | |
| } | |
| // Skip white-space longhands; will emit shorthand later | |
| if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) { | |
| continue; | |
| } | |
| // Already cleared above. | |
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | |
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | |
| payload->value = stringToNativeString(value_string).release(); | |
| if (!base_href_string.IsEmpty()) { | |
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | |
| } else { | |
| payload->href = nullptr; | |
| } | |
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), element->bindingObject(), | |
| payload); | |
| continue; | |
| } | |
| // Custom properties require sending the property name across the bridge. | |
| if (id == CSSPropertyID::kVariable) { | |
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | |
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | |
| if (value_string.IsNull()) value_string = String(""); | |
| if (value_string.IsEmpty()) { | |
| value_string = String(" "); | |
| } | |
| // Already cleared above. | |
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | |
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | |
| payload->value = stringToNativeString(value_string).release(); | |
| if (!base_href_string.IsEmpty()) { | |
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | |
| } else { | |
| payload->href = nullptr; | |
| } | |
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), element->bindingObject(), | |
| payload); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In bridge/core/css/style_engine.cc around lines 1319 to 1345, there is an
unreachable check inside the kVariable branch: the if (id ==
CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) block
(lines ~1329-1331) can never be true because id is already kVariable; remove
that dead if-block from inside the kVariable handling. If the
whitespace-longhand skip was intended for non-variable properties, instead
relocate that check to the surrounding code before the kVariable branch so it
runs for other property ids; otherwise simply delete the unreachable lines.
| // Custom properties require sending the property name across the bridge. | ||
| if (id == CSSPropertyID::kVariable) { | ||
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | ||
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | ||
| if (value_string.IsNull()) value_string = String(""); | ||
| if (value_string.IsEmpty()) { | ||
| value_string = String(" "); | ||
| } | ||
|
|
||
| if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) { | ||
| continue; | ||
| } | ||
|
|
||
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | ||
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | ||
| payload->value = stringToNativeString(value_string).release(); | ||
| if (!base_href_string.IsEmpty()) { | ||
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | ||
| } else { | ||
| payload->href = nullptr; | ||
| } | ||
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), el->bindingObject(), payload); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Same dead code issue exists in RecalcStyleForElementOnly.
Lines 1695-1697 contain the same unreachable code checking for kWhiteSpaceCollapse/kTextWrap inside a kVariable block.
🔎 Proposed fix to remove dead code
if (value_string.IsEmpty()) {
value_string = String(" ");
}
- if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) {
- continue;
- }
-
auto key_ns = prop_name.ToStylePropertyNameNativeString();📝 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.
| // Custom properties require sending the property name across the bridge. | |
| if (id == CSSPropertyID::kVariable) { | |
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | |
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | |
| if (value_string.IsNull()) value_string = String(""); | |
| if (value_string.IsEmpty()) { | |
| value_string = String(" "); | |
| } | |
| if (id == CSSPropertyID::kWhiteSpaceCollapse || id == CSSPropertyID::kTextWrap) { | |
| continue; | |
| } | |
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | |
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | |
| payload->value = stringToNativeString(value_string).release(); | |
| if (!base_href_string.IsEmpty()) { | |
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | |
| } else { | |
| payload->href = nullptr; | |
| } | |
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), el->bindingObject(), payload); | |
| continue; | |
| } | |
| // Custom properties require sending the property name across the bridge. | |
| if (id == CSSPropertyID::kVariable) { | |
| String value_string = property_set->GetPropertyValueWithHint(prop_name, i); | |
| String base_href_string = property_set->GetPropertyBaseHrefWithHint(prop_name, i); | |
| if (value_string.IsNull()) value_string = String(""); | |
| if (value_string.IsEmpty()) { | |
| value_string = String(" "); | |
| } | |
| auto key_ns = prop_name.ToStylePropertyNameNativeString(); | |
| auto* payload = reinterpret_cast<NativeStyleValueWithHref*>(dart_malloc(sizeof(NativeStyleValueWithHref))); | |
| payload->value = stringToNativeString(value_string).release(); | |
| if (!base_href_string.IsEmpty()) { | |
| payload->href = stringToNativeString(base_href_string.ToUTF8String()).release(); | |
| } else { | |
| payload->href = nullptr; | |
| } | |
| ctx->uiCommandBuffer()->AddCommand(UICommand::kSetStyle, std::move(key_ns), el->bindingObject(), payload); | |
| continue; | |
| } |
🤖 Prompt for AI Agents
In bridge/core/css/style_engine.cc around lines 1686 to 1709, inside the
kVariable handling block there is an unreachable check for id ==
kWhiteSpaceCollapse || id == kTextWrap (lines ~1695-1697); remove that dead
conditional (and its continue) from this kVariable branch since id is guaranteed
to be kVariable here, and also remove the identical unreachable check in
RecalcStyleForElementOnly to keep both code paths consistent; simply delete the
redundant if statement so the kVariable flow only builds the payload and issues
the UICommand as intended.
andycall
changed the title
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.