feat: Use vsync to drive style calculations #777
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds executing-context idle-state tracking and uses it to gate script entry, callbacks, and microtask draining; introduces display-none invalidation tracking and propagation through style recalculation; adds per-frame Blink style-update FFI and scheduling; introduces HTML fragment parsing helpers and tests for CSS invalidation and pending invalidations. Changes
Sequence Diagram(s)sequenceDiagram
participant Frame as Frame Scheduler
participant VC as ViewController
participant Page as WebFPage
participant Native as Native Bridge
participant JS as ExecutingContext / JS Thread
participant Doc as Document
participant SE as StyleEngine
Note over Frame,VC: Per-frame Blink style update scheduling
Frame->>VC: begin frame
VC->>VC: _scheduleBlinkStyleUpdateForNextFrame()
VC->>Page: getAllocatedPage(contextId)
Page->>Native: updateStyleForThisDocument(page)
alt native enqueues update
Native->>JS: PostCallback(update style)
JS->>JS: SetIsIdle(false)
JS->>Doc: UpdateStyleForThisDocument() within MutationScope
Doc->>SE: RecalcStyleForSubtree / RecalcStyleForElement
SE->>SE: set display-none flag & normalize variables
JS->>JS: Queue UI finish command
JS->>JS: DrainMicrotasks()
JS->>JS: SetIsIdle(true)
else native skipped (in-progress / invalid)
Native-->>VC: return skipped
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bridge/test/css_unittests.cmake (1)
80-80: Remove duplicate commented test discovery call for clarity.Lines 108–109 contain a commented duplicate of the
gtest_discover_tests()call andinclude(GoogleTest)from lines 79–80. Remove these commented lines to avoid confusion.
🧹 Nitpick comments (9)
bridge/core/html/parser/html_parser.cc (2)
302-410: Consider extracting shared node-handling logic to reduce duplication.The
append_gumbo_childlambda duplicates most of the child-handling logic fromtraverseHTML(lines 138-234). Both handle ELEMENT, TEXT, WHITESPACE, COMMENT, and CDATA nodes with nearly identical code.Consider extracting a shared helper to handle individual
GumboNodeconversion, which bothtraverseHTMLand the fragment flattening logic can call. This would improve maintainability and reduce the risk of divergence.
235-235: Stale comment: comments are now handled.The comment states "Ignore other node types (comments, etc.)" but comments are explicitly handled at lines 210-221. Consider updating to reflect what's actually being ignored (e.g., GUMBO_NODE_DOCUMENT, GUMBO_NODE_TEMPLATE).
webf/lib/src/bridge/binding_object.dart (1)
386-400: Consider adding scroll-related geometry properties.The layout-dependent property set covers offset and client dimensions but may be missing scroll-related properties like
scrollWidth,scrollHeight,scrollLeft, andscrollTop, which also depend on layout calculations.const Set<String> _layoutDependentBindingProperties = <String>{ 'offsetLeft', 'offsetTop', 'offsetWidth', 'offsetHeight', 'clientLeft', 'clientTop', 'clientWidth', 'clientHeight', + 'scrollLeft', + 'scrollTop', + 'scrollWidth', + 'scrollHeight', };Based on coding guidelines for Dart code.
bridge/core/dom/element.cc (2)
139-167: Consider extracting the duplicated style-update pattern.The pre-binding style update logic (lines 140-147) is identical to
getBoundingClientRect(lines 117-124). This could be extracted into a helper method to reduce duplication.🔎 Suggested helper extraction
+void Element::EnsureStyleUpdatedForLayout() { + ExecutingContext* context = GetExecutingContext(); + if (context && context->isBlinkEnabled()) { + Document* doc = context->document(); + if (doc) { + MemberMutationScope mutation_scope{context}; + doc->UpdateStyleForThisDocument(); + } + } +} BoundingClientRect* Element::getBoundingClientRect(ExceptionState& exception_state) { - ExecutingContext* context = GetExecutingContext(); - if (context && context->isBlinkEnabled()) { - Document* doc = context->document(); - if (doc) { - MemberMutationScope mutation_scope{context}; - doc->UpdateStyleForThisDocument(); - } - } + EnsureStyleUpdatedForLayout(); NativeValue result = InvokeBindingMethod(
1058-1063: Clarify the purpose of normalizing empty CSS variable values to a single space.The code converts empty
value_stringto" "for CSS variables. This appears to be a workaround for the Dart side handling ofvar()declarations. A brief comment explaining why this normalization is necessary would improve maintainability.🔎 Suggested documentation
CSSPropertyID id = property.Id(); AtomicString prop_name = property.Name().ToAtomicString(); String value_string = inline_style->GetPropertyValueWithHint(prop_name, i); + // CSS custom properties (variables) with empty values must be normalized + // to a single space to ensure proper var() handling on the Dart side. + // An empty string would be treated as "invalid" rather than "initial". if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) { value_string = String(" "); }bridge/core/css/invalidation/pending_invalidations_test.cc (1)
127-254: Consider reducing the debug trace verbosity or extracting it.The
build_debug_tracelambda spans ~130 lines and performs extensive style resolution and inspection. While valuable for debugging failures, this could:
- Be extracted to a shared test utility if used elsewhere
- Be conditionally compiled or gated behind a verbose flag
- Include only the most essential diagnostics
The trace does full style cascade resolution (
CollectAllRules,StyleCascade,ExportWinningPropertySet) which is heavy for a test assertion helper.bridge/core/dom/element.h (1)
423-428: Clarify the dual-path attribute checking logic.The updated
hasAttributes()checks bothAttributes()(from element_data_) and the separateattributes_member. This mirrors the pattern inHasAttributeIgnoringNamespace.Consider adding a brief comment explaining why both paths are necessary (e.g., Blink-style vs legacy attribute storage).
🔎 Suggested documentation
inline bool Element::hasAttributes() const { + // Check both Blink-style element data attributes and legacy ElementAttributes. + // The former is used for CSS selector matching; the latter for DOM API compatibility. if (!Attributes().IsEmpty()) { return true; } return attributes_ != nullptr && attributes_->hasAttributes(); }bridge/core/executing_context.h (1)
201-203: Idle-state flag wiring is straightforward; consider thread-safety constraintsThe new
isIdle()/SetIsIdle()accessors andis_idle_member are simple and consistent. This is fine as long as all reads/writes happen on the JS thread, like the rest of the non-atomic flags here.If there’s any chance
SetIsIdle()is invoked off the JS thread (e.g., from dispatcher callbacks on other threads), consider makingis_idle_anstd::atomic<bool>for clarity and safety.Please verify (by grepping the .cc files) that all
SetIsIdlecall sites run on the JS thread or behindPostToJs, and adjust tostd::atomic<bool>if not. -->Also applies to: 298-300
bridge/core/css/style_engine.cc (1)
1165-1171: Display:none tracking and CSS variable normalization look correct; consider de-duplicating helper logic
- Computing
display_none_for_invalidationfrom the finaldisplayvalue and storing it viaSetDisplayNoneForStyleInvalidationin both subtree and element-only recalc paths is consistent and gives the invalidation layer accurate per-element information about localdisplay: none.- The normalization
if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) value_string = " ";is applied uniformly for element styles and pseudo styles, avoiding empty-string payloads for custom properties across the bridge.The behavior is sound; the only small nit is that the
kVariablenormalization snippet is repeated several times. If this evolves further, you might want a tiny helper likeNormalizeCustomPropertyValue(id, value_string)to keep the logic centralized.-->
Also applies to: 1205-1223, 1373-1380, 1479-1485, 1517-1523, 1613-1616, 1667-1673
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
integration_tests/snapshots/blink/css1/box_properties/padding_left.ts.f039ce881.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-flexbox/relayout-align.ts.6eab38941.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-variables/variable-declaration-45.html.6172c1901.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-variables/variable-declaration-57.html.889f7f911.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-variables/variable-declaration-58.html.e5a37f891.pngis excluded by!**/*.png
📒 Files selected for processing (33)
bridge/bindings/qjs/qjs_function.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/binding_object.ccbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/css/invalidation/pending_invalidations.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/css/style_engine.hbridge/core/dom/document.ccbridge/core/dom/document.hbridge/core/dom/element.ccbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/executing_context.ccbridge/core/executing_context.hbridge/core/frame/script_idle_task_controller.ccbridge/core/html/parser/html_parser.ccbridge/core/page.hbridge/include/webf_bridge.hbridge/polyfill/src/test/index.jsbridge/test/css_unittests.cmakebridge/webf_bridge.ccintegration_tests/lib/bridge/from_native.dartintegration_tests/lib/playground.dartintegration_tests/lib/webf_tester.dartintegration_tests/specs/blink/css1/box_properties/padding_left.tsintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.tsintegration_tests/specs/css/css-text-mixin/text_effects_test.tswebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.dartwebf/lib/src/css/style_declaration.dartwebf/lib/src/dom/element_widget_adapter.dartwebf/lib/src/launcher/view_controller.dart
🧰 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/include/webf_bridge.hbridge/core/css/invalidation/pending_invalidations.ccbridge/core/dom/document.ccbridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/core/executing_context.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/bindings/qjs/script_promise_resolver.ccbridge/core/dom/document.hbridge/core/binding_object.ccbridge/core/css/style_engine.hbridge/bindings/qjs/qjs_function.ccbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/dom/element.cc
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/include/webf_bridge.hbridge/core/executing_context.hbridge/core/dom/element.hbridge/core/page.hbridge/core/dom/document.hbridge/core/css/style_engine.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/include/webf_bridge.hbridge/core/css/invalidation/pending_invalidations.ccbridge/core/dom/document.ccbridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/core/executing_context.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/bindings/qjs/script_promise_resolver.ccbridge/core/dom/document.hbridge/core/binding_object.ccbridge/core/css/style_engine.hbridge/bindings/qjs/qjs_function.ccbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/dom/element.cc
{bridge/**/*.{cc,h},webf/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Document memory ownership clearly in FFI implementations
Files:
bridge/include/webf_bridge.hbridge/core/css/invalidation/pending_invalidations.ccbridge/core/dom/document.ccbridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccwebf/lib/src/launcher/view_controller.dartbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccwebf/lib/src/dom/element_widget_adapter.dartbridge/core/html/parser/html_parser.ccbridge/core/executing_context.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/bindings/qjs/script_promise_resolver.ccbridge/core/dom/document.hwebf/lib/src/bridge/binding_object.dartbridge/core/binding_object.ccbridge/core/css/style_engine.hbridge/bindings/qjs/qjs_function.ccwebf/lib/src/css/style_declaration.dartwebf/lib/src/bridge/to_native.dartbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/dom/element.cc
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/invalidation/pending_invalidations.ccbridge/core/dom/document.ccbridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/webf_bridge.ccbridge/core/dom/frame_request_callback_collection.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/binding_object.ccbridge/bindings/qjs/qjs_function.ccbridge/core/css/invalidation/invalidation_set_test.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/invalidation/pending_invalidations.ccbridge/core/dom/document.ccbridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/core/executing_context.hbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/core/dom/document.hbridge/core/binding_object.ccbridge/core/css/style_engine.hbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/dom/element.cc
integration_tests/specs/**/*.ts
📄 CodeRabbit inference engine (integration_tests/CLAUDE.md)
integration_tests/specs/**/*.ts: Place tests in appropriate directories underspecs/(css/, dom/, or window/)
Use TypeScript (.ts extension) for test files
Usedone()callback for async tests
Usesnapshot()for visual regression tests to capture current rendering
UsesimulateClickwith coordinates for hit testing tests
Test assets should reference files inassets/directory
Usefdescribe()instead ofdescribe()to run only specific test specs (Jasmine feature)
Usefit()instead ofit()to run only specific test cases
Snapshots are stored as images for comparison and failed snapshots generate diff images
The max width of testing window is 340px
Test specs will always pass if there are no existing snapshots
Group related tests in describe blocks
Each test should be independent
Remove created elements after tests (test cleanup)
Use clear, descriptive test names
Test behavior, not implementation
Include edge cases and error conditions in tests
Batch DOM operations to minimize reflows
Use async/await and proper async patterns for asynchronous operations in tests
Measure operations usingperformance.now()for timing in performance-critical tests
Files:
integration_tests/specs/blink/css1/box_properties/padding_left.tsintegration_tests/specs/css/css-text-mixin/text_effects_test.tsintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
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/launcher/view_controller.dartwebf/lib/src/dom/element_widget_adapter.dartwebf/lib/src/bridge/binding_object.dartwebf/lib/src/css/style_declaration.dartwebf/lib/src/bridge/to_native.dart
webf/lib/src/css/**/*.dart
📄 CodeRabbit inference engine (webf/CLAUDE.md)
Use CSSRenderStyle for style computation and storage in Dart CSS code
Files:
webf/lib/src/css/style_declaration.dart
🧠 Learnings (65)
📓 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
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
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} : 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
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Batch DOM operations to minimize reflows
📚 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 webf/test/**/*.dart : Dart/Flutter widget and unit tests must be placed in `webf/test/` and use `WebFWidgetTestUtils` and `pumpAndSettle()` where needed
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Use WebFWidgetTestUtils.prepareWidgetTest() to test HTML/CSS rendering in widget unit tests
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Call setupTest() in setUpAll() method for one-time setup in unit tests
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Use mock bundles from test/src/foundation/mock_bundle.dart for testing in unit tests
Applied to files:
integration_tests/lib/webf_tester.dartwebf/lib/src/bridge/binding_object.dart
📚 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 webf/integration_test/**/*.dart : Dart/Flutter integration tests must be placed in `webf/integration_test/` directory
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : When testing with WebFController, wait for initialization with: await controller.controlledInitCompleter.future
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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: Use widget unit tests to verify rendering changes with: cd webf && flutter test test/src/rendering/
Applied to files:
integration_tests/lib/webf_tester.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/integration_tests/specs/**/*.dart : Use snapshot tests in integration_tests/specs/ for visual regression testing with await snapshot()
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Access render objects in tests to verify layout calculations
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.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: Run Flutter dart tests with: cd webf && flutter test
Applied to files:
integration_tests/lib/webf_tester.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/bridge.dart : lib/bridge.dart contains FFI bindings to C++ bridge
Applied to files:
integration_tests/lib/webf_tester.dartbridge/include/webf_bridge.hintegration_tests/lib/bridge/from_native.dartbridge/webf_bridge.ccintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/bindings/qjs/qjs_function.ccwebf/lib/src/bridge/to_native.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 bridge/**/*.{cc,h} : Use `PostToDart` for returning results to Dart isolate
Applied to files:
integration_tests/lib/webf_tester.dartbridge/include/webf_bridge.hintegration_tests/lib/bridge/from_native.dartbridge/webf_bridge.ccintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/core/binding_object.ccwebf/lib/src/bridge/to_native.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} : Use `WEBF_EXPORT_C` macro for exporting C functions to Dart FFI
Applied to files:
bridge/include/webf_bridge.hbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.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 {bridge/**/*.{cc,h},webf/**/*.dart} : Document memory ownership clearly in FFI implementations
Applied to files:
bridge/include/webf_bridge.hintegration_tests/lib/bridge/from_native.dartbridge/webf_bridge.ccwebf/lib/src/bridge/to_native.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/include/webf_bridge.hwebf/lib/src/launcher/view_controller.dartbridge/core/css/style_engine.ccbridge/core/binding_object.ccwebf/lib/src/css/style_declaration.dartwebf/lib/src/bridge/to_native.dartbridge/core/dom/element.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 : Call updateWidget() when attributes change in WidgetElement implementations
Applied to files:
bridge/include/webf_bridge.hwebf/lib/src/dom/element_widget_adapter.dartwebf/lib/src/bridge/binding_object.dartbridge/core/binding_object.ccwebf/lib/src/bridge/to_native.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:
bridge/include/webf_bridge.hintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/core/binding_object.ccwebf/lib/src/bridge/to_native.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:
bridge/include/webf_bridge.hintegration_tests/lib/bridge/from_native.dartwebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.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 : 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:
bridge/include/webf_bridge.hbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.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: No build needed for Dart-only changes in webf/
Applied to files:
bridge/include/webf_bridge.hwebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.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: DOM element implementations are located in lib/src/dom/
Applied to files:
bridge/core/css/invalidation/pending_invalidations.cc
📚 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} : Avoid `PostToJsSync` synchronous execution when possible
Applied to files:
bridge/core/executing_context.ccbridge/webf_bridge.ccbridge/core/dom/frame_request_callback_collection.ccbridge/bindings/qjs/script_promise_resolver.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} : Avoid `PostToJsSync` when threads may interdepend to prevent synchronous deadlocks in FFI communication
Applied to files:
bridge/core/executing_context.ccbridge/core/frame/script_idle_task_controller.ccbridge/webf_bridge.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/bindings/qjs/script_promise_resolver.ccbridge/bindings/qjs/qjs_function.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} : 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
Applied to files:
bridge/core/executing_context.ccbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartbridge/bindings/qjs/qjs_function.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} : Ensure callback functions aren't invoked after context destruction to prevent use-after-free errors in FFI async operations
Applied to files:
bridge/core/frame/script_idle_task_controller.ccbridge/core/dom/frame_request_callback_collection.cc
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Test behavior, not implementation
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.tsintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Batch DOM operations to minimize reflows
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.tsbridge/polyfill/src/test/index.jsintegration_tests/specs/css/css-text-mixin/text_effects_test.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Place tests in appropriate directories under `specs/` (css/, dom/, or window/)
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.tsintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : The max width of testing window is 340px
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Remove created elements after tests (test cleanup)
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.tsbridge/polyfill/src/test/index.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,h,hpp} : In FFI contexts, use `Dart_Handle` instead of `Handle` for type compatibility
Applied to files:
integration_tests/lib/bridge/from_native.dartintegration_tests/lib/playground.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} : 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:
integration_tests/lib/bridge/from_native.dartwebf/lib/src/bridge/binding_object.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:
integration_tests/lib/bridge/from_native.dartintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/core/binding_object.ccbridge/bindings/qjs/qjs_function.ccwebf/lib/src/bridge/to_native.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:
integration_tests/lib/bridge/from_native.dartintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.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:
integration_tests/lib/bridge/from_native.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:
integration_tests/lib/bridge/from_native.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 : Always free allocated memory in Dart FFI using malloc.free() for toNativeUtf8() allocations
Applied to files:
integration_tests/lib/bridge/from_native.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 : Always free allocated memory in Dart FFI
Applied to files:
integration_tests/lib/bridge/from_native.dart
📚 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/test/**/*.{cc,h} : Bridge unit tests must be added under `bridge/test/` and use Google Test framework
Applied to files:
bridge/core/css/invalidation/pending_invalidations_test.ccbridge/test/css_unittests.cmakebridge/core/css/invalidation/invalidation_set_test.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/test/**/*.{cc,cpp} : Add bridge unit test files to `bridge/test/` directory and use Google Test macros: `TEST()`, `EXPECT_EQ()`, etc. Tests are automatically discovered by CMake
Applied to files:
bridge/core/css/invalidation/pending_invalidations_test.ccbridge/test/css_unittests.cmakebridge/core/css/invalidation/invalidation_set_test.cc
📚 Learning: 2025-12-21T14:42:52.627Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.627Z
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/core/css/invalidation/pending_invalidations_test.ccbridge/test/css_unittests.cmakebridge/core/css/invalidation/invalidation_set_test.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/invalidation/pending_invalidations_test.ccbridge/webf_bridge.ccbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/dom/element.cc
📚 Learning: 2025-12-21T14:42:52.627Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: bridge/AGENTS.md:0-0
Timestamp: 2025-12-21T14:42:52.627Z
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/invalidation/pending_invalidations_test.ccbridge/webf_bridge.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 : Minimize widget rebuilds in WidgetElement for performance optimization
Applied to files:
webf/lib/src/dom/element_widget_adapter.dartbridge/core/binding_object.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 : Follow W3C event standards when dispatching events from WidgetElement
Applied to files:
webf/lib/src/dom/element_widget_adapter.dartbridge/core/binding_object.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 : 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.dartwebf/lib/src/bridge/binding_object.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 : Dispose controllers and subscriptions in WidgetElement for memory management
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/webf_bridge.ccbridge/core/dom/element.cc
📚 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 `PostToJs` for executing operations on the JS thread in FFI
Applied to files:
bridge/webf_bridge.ccbridge/core/dom/frame_request_callback_collection.ccbridge/bindings/qjs/qjs_function.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,d.ts} : Include a reference to `webf.d.ts` in polyfill type generation for accessing core types like `EventTarget`
Applied to files:
bridge/webf_bridge.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/core/**/*.d.ts : Create a `webf` namespace containing all type exports from merged bridge core types
Applied to files:
bridge/webf_bridge.cc
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Measure operations using `performance.now()` for timing in performance-critical tests
Applied to files:
bridge/polyfill/src/test/index.js
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use `snapshot()` for visual regression tests to capture current rendering
Applied to files:
bridge/polyfill/src/test/index.jsintegration_tests/specs/css/css-text-mixin/text_effects_test.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use `fdescribe()` instead of `describe()` to run only specific test specs (Jasmine feature)
Applied to files:
bridge/polyfill/src/test/index.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 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
Applied to files:
bridge/core/dom/frame_request_callback_collection.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/lib/src/rendering/**/*.dart : Use RenderBoxModel as base class for layout in Dart rendering code
Applied to files:
webf/lib/src/bridge/binding_object.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 FFI allocated memory in finally blocks to ensure cleanup on exceptions
Applied to files:
webf/lib/src/bridge/binding_object.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 rules in webf/analysis_options.yaml for Dart code style
Applied to files:
bridge/core/binding_object.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/**/*.{cpp,h,hpp} : Use RAII patterns in C++ where possible for memory management
Applied to files:
bridge/bindings/qjs/qjs_function.cc
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Include edge cases and error conditions in tests
Applied to files:
integration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use clear, descriptive test names
Applied to files:
integration_tests/specs/css/css-text-mixin/text_comprehensive_test.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: CSS property implementations are located in lib/src/css/
Applied to files:
webf/lib/src/css/style_declaration.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 : Variables and functions must use camelCase in Dart
Applied to files:
webf/lib/src/css/style_declaration.dart
🧬 Code graph analysis (8)
bridge/include/webf_bridge.h (1)
bridge/webf_bridge.cc (2)
updateStyleForThisDocument(309-363)updateStyleForThisDocument(309-309)
bridge/core/dom/document.cc (1)
bridge/core/dom/document.h (1)
update_style_for_this_document_in_progress_(277-289)
bridge/core/html/parser/html_parser.cc (1)
integration_tests/scripts/html_loader.js (1)
root(141-141)
bridge/core/executing_context.h (2)
bridge/core/dom/document.h (1)
bool(278-279)bridge/core/page.h (1)
bool(144-144)
bridge/core/dom/element.h (1)
bridge/core/dom/element.cc (2)
Attributes(76-81)Attributes(76-76)
bridge/core/page.h (1)
bridge/core/dom/document.h (2)
update_style_for_this_document_in_progress_(277-289)bool(278-279)
bridge/core/css/style_engine.h (1)
bridge/core/css/invalidation/pending_invalidations.h (1)
PendingInvalidations(106-131)
bridge/core/dom/element.cc (2)
bridge/core/dom/element.h (1)
GetElementData(365-365)bridge/core/css/style_engine.cc (2)
id(83-83)id(83-83)
🪛 GitHub Actions: Run Code Linter
bridge/test/css_unittests.cmake
[error] 67-67: CMake configuration failed: No SOURCES given to target 'webf_css_unittests' while running 'cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON .'
⏰ 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 (javascript-typescript)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (c-cpp)
integration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
Outdated
Show resolved
Hide resolved
integration_tests/specs/css/css-text-mixin/text_effects_test.ts
Outdated
Show resolved
Hide resolved
- Replaced `contextId` with `testContext` to align with API requirements. - Added memory management for native method allocation to prevent leaks.
- Added `IsDisplayNoneForStyleInvalidation` to skip descendant invalidations for `display:none` subtrees.
- Improved `HTMLParser` with separate `parseDocument` and `parseFragment` methods for better context handling.
- Updated Gumbo tree traversal logic to handle complex fragment parsing scenarios.
- Added new unit tests for CSS invalidation behavior.
also fixes 'PendingInvalidationsTest.DescendantInvalidationOnDisplayNone'
- Root cause: setInnerHTML() used a full-document Gumbo parse and then traversed only <body>, so <style> often got moved into <head> by Gumbo and was dropped
(no author sheets → no rule match → display:none flag stayed false).
- Fix: core/html/parser/html_parser.cc:39 now uses Gumbo fragment parsing (fragment_context from the target element) and core/html/parser/html_parser.cc:412
flattens parsed <html>/<head>/<body> wrappers so fragment <style> nodes are preserved/inserted.
- Debug help: core/css/invalidation/pending_invalidations_test.cc:206 now also reports whether a <style> exists anywhere in the document (and its parent),
not just directly under <body>.
- Updated `text_comprehensive_test` to include `backgroundClip` fallback for text gradient tests. - Added a `FIXME` comment to highlight removal after longhand support is implemented.
- Updated `text_effects_test` to include explicit `backgroundClip` styling. - Added `FIXME` comment for removal after longhand `background-clip` support is implemented.
The text should be green, but dart side did not handle `--a` inherits from body when there's no body in the html. The orignal snapshot is red, which is more off than black, change to black for now.
The correct snapshot should be only upside shown fixes: integration_tests/specs/css/css-flexbox/relayout-align.ts
The text should be green, but dart side did not handle `--a` inherits from body when there's no body in the html. The orignal snapshot is red, which is more off than black, change to black for now.
…ies/padding_left` integration test When Blink is disabled, the test sets ul.style.paddingLeft = '25px' before the <ul> is inserted/connected (see your log: setStyle paddingLeft ... happens before insertAdjacentNode). On connection, WebF applies UA default styles for lists: webf/lib/src/html/grouping_content.dart sets PADDING_INLINE_START: '40px' for <ul>/<ol>. During style flush, that later-added paddingInlineStart is mapped to the physical side (webf/lib/src/css/render_style.dart: paddingInlineStart → paddingLeft in LTR) and ends up overwriting the earlier inline paddingLeft: 25px because it’s emitted later in the same flush batch. When Blink is enabled, styles are re-applied after insertion (your log shows clearStyle then setStyle paddingLeft/backgroundColor after nodes are in the tree), so the author inline paddingLeft: 25px runs last and correctly overrides the UA list padding — snapshot matches.
- Added logic to update style during `getBoundingClientRect` and `getClientRects` calls when Blink is enabled. - Improved style invalidation handling for layout-dependent DOM properties and methods. - Updated test harness to ensure layout flushing aligns with `requestAnimationFrame`.
- Added logic to update style during `getBoundingClientRect` and `getClientRects` calls when Blink is enabled. - Improved style invalidation handling for layout-dependent DOM properties and methods. - Updated test harness to ensure layout flushing aligns with `requestAnimationFrame`.
- Updated `_setProperty` to bypass normalization for custom properties. 1. The CSS properties are case censitive 2. The CSS property should not be trimmed
This fixes Tailwind CSS gradients - Added fallback for empty `CSSPropertyID::kVariable` values to prevent issues during style normalization. - Ensures empty values default to a single space string to maintain consistency.
This reverts commit 1b31a21.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bridge/core/executing_context.cc (1)
188-204: EnsureSetIsIdle(false)is always paired with a path back to idleRight now several eval error paths set the context non‑idle and then return before
DrainMicrotasks()(which callsSetIsIdle(true)), leavingisIdle()stuck atfalseand effectively blocking vsync‑driven style updates:
EvaluateJavaScript(const char* …, uint8_t** parsed_bytecodes, …)bytecode path whenJS_Eval(..., COMPILE_ONLY)returns an exception.EvaluateModule(...)in both bytecode and non‑bytecode paths when the compile step fails.EvaluateByteCode(...)whenJS_ReadObjectreturns an exception.Also,
SetIsIdle(false)is currently placed before the 100MB size check in the JS and bytecode eval paths, so a size error can leave the context marked busy even though no script ran.To avoid contexts getting permanently “non‑idle”:
- Move
SetIsIdle(false)after the size‑limit guards so purely‑validation failures never touch idle state.- In all branches that return immediately after
HandleException(...)on a compiled script/module/bytecode failure, callDrainMicrotasks()(or at leastSetIsIdle(true)) before returning so the idle flag is restored even on errors.For example:
- JSValue byte_object = - JS_Eval(script_state_.ctx(), code, code_len, sourceURL, JS_EVAL_TYPE_MODULE | JS_EVAL_FLAG_COMPILE_ONLY); - - if (JS_IsException(byte_object)) { - HandleException(&byte_object); - return false; - } + JSValue byte_object = + JS_Eval(script_state_.ctx(), code, code_len, sourceURL, JS_EVAL_TYPE_MODULE | JS_EVAL_FLAG_COMPILE_ONLY); + + if (JS_IsException(byte_object)) { + HandleException(&byte_object); + DrainMicrotasks(); // Will call SetIsIdle(true) + return false; + }and similarly for the other early‑return sites, plus moving
SetIsIdle(false)below size checks in the eval entry points.Also applies to: 295-347, 380-431, 546-551
bridge/core/css/style_engine.cc (1)
1203-1221: Fix uses ofvalue_stringbefore declaration in pseudo‑style emissionIn two pseudo‑style loops,
value_stringis checked for emptiness before it has been declared/initialized, which will not compile and would be undefined behavior even if it did:
RecalcStyleForSubtree→emit_pseudo_if_anylambda:if (id == CSSPropertyID::kVariable && value_string.IsEmpty())precedes theString value_string = …declaration.RecalcStyleForElementOnly→send_pseudo_forlambda (second one): same pattern.The normalization to a single‑space placeholder for empty CSS variable values should run after you’ve obtained
value_string. For example:Proposed fix for both pseudo‑style loops
@@ RecalcStyleForSubtree → emit_pseudo_if_any - for (unsigned i = 0; i < pseudo_set->PropertyCount(); ++i) { - auto prop = pseudo_set->PropertyAt(i); - CSSPropertyID id = prop.Id(); - if (id == CSSPropertyID::kInvalid) { - continue; - } - const auto* value_ptr = prop.Value(); - if (!value_ptr || !(*value_ptr)) { - continue; - } - if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) { - value_string = String(" "); - } - AtomicString prop_name = prop.Name().ToAtomicString(); - String value_string = pseudo_set->GetPropertyValueWithHint(prop_name, i); + for (unsigned i = 0; i < pseudo_set->PropertyCount(); ++i) { + auto prop = pseudo_set->PropertyAt(i); + CSSPropertyID id = prop.Id(); + if (id == CSSPropertyID::kInvalid) { + continue; + } + const auto* value_ptr = prop.Value(); + if (!value_ptr || !(*value_ptr)) { + continue; + } + AtomicString prop_name = prop.Name().ToAtomicString(); + String value_string = pseudo_set->GetPropertyValueWithHint(prop_name, i); if (value_string.IsNull()) { value_string = (*value_ptr)->CssTextForSerialization(); } + if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) { + value_string = String(" "); + } @@ RecalcStyleForElementOnly → send_pseudo_for (second instance) - for (unsigned i = 0; i < pseudo_set->PropertyCount(); ++i) { - auto prop = pseudo_set->PropertyAt(i); - CSSPropertyID id = prop.Id(); - if (id == CSSPropertyID::kInvalid) { - continue; - } - const auto* value_ptr = prop.Value(); - if (!value_ptr || !(*value_ptr)) { - continue; - } - if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) { - value_string = String(" "); - } - AtomicString prop_name = prop.Name().ToAtomicString(); - String value_string = pseudo_set->GetPropertyValueWithHint(prop_name, i); + for (unsigned i = 0; i < pseudo_set->PropertyCount(); ++i) { + auto prop = pseudo_set->PropertyAt(i); + CSSPropertyID id = prop.Id(); + if (id == CSSPropertyID::kInvalid) { + continue; + } + const auto* value_ptr = prop.Value(); + if (!value_ptr || !(*value_ptr)) { + continue; + } + AtomicString prop_name = prop.Name().ToAtomicString(); + String value_string = pseudo_set->GetPropertyValueWithHint(prop_name, i); if (value_string.IsNull()) { value_string = (*value_ptr)->CssTextForSerialization(); } + if (id == CSSPropertyID::kVariable && value_string.IsEmpty()) { + value_string = String(" "); + }This preserves the intended placeholder behavior while fixing the compile‑time error.
Also applies to: 1689-1696
♻️ Duplicate comments (1)
bridge/bindings/qjs/script_promise_resolver.cc (1)
54-54: ChangeSetIsIdle(true)toSetIsIdle(false)before promise callback execution.The pattern across the codebase is consistent:
SetIsIdle(false)is set before executing any JS callbacks (e.g.,QJSFunction::Invoke,FrameRequestCallbackCollection::Fire,ScriptIdleTaskController::Fire), thenSetIsIdle(true)after completion viaDrainMicrotasks().At line 54,
SetIsIdle(true)is set beforeJS_Call, which is semantically incorrect—the context is not idle while executing the promise resolve/reject callbacks. Change toSetIsIdle(false)to match other callback execution patterns.
🧹 Nitpick comments (5)
bridge/core/html/parser/html_parser.cc (2)
302-410: Consider extracting common element creation logic to reduce duplication.The
append_gumbo_childlambda duplicates significant logic fromtraverseHTML()(lines 138-196 for elements, 198-233 for text/comment/CDATA). Both segments perform identical operations: tag name extraction, namespace-aware element creation, exception handling, attribute parsing, and child traversal.This duplication creates maintenance overhead—bug fixes or enhancements must be applied in multiple locations.
💡 Refactoring approach
Consider extracting the common element-creation and child-handling logic into separate static helper functions:
// Extract element creation logic static Element* createElementFromGumbo( ExecutingContext* context, GumboNode* child, ExceptionState& exception_state) { // Lines 307-334 logic here // Returns nullptr on failure } // Extract child appending logic static bool appendGumboNodeChild( ContainerNode* parent, GumboNode* child, ExecutingContext* context) { // Current lambda body here }Then both
append_gumbo_childand the element handling intraverseHTMLcan call these shared helpers, ensuring consistent behavior and reducing the risk of divergence.
302-410: Consider extracting the lambda to a static helper function.This 108-line lambda handles multiple node types and includes substantial logic. Extracting it to a static function would improve readability, testability, and allow reuse without capturing context variables.
💡 Suggested extraction
static bool appendGumboNodeChild( ContainerNode* parent, GumboNode* child, ExecutingContext* context) { // Current lambda body }Then call it from both the lambda location and potentially from other parsing contexts.
integration_tests/specs/blink/css1/box_properties/padding_left.ts (1)
154-156: Font-family set for rendering consistency.Setting
fontFamilyto 'Times' before the snapshot ensures consistent baseline rendering for the list items. This is a good practice for visual regression tests.Consider whether other tests in this file would benefit from the same font-family setup for cross-platform consistency.
integration_tests/lib/webf_tester.dart (1)
31-31: Consider usinglatefor testContext.Since
testContextis initialized before use inonControllerInit(line 48) and used non-null (line 49), consider declaring it aslate Pointer<Void> testContext;instead of nullable to better express the initialization contract.webf/lib/src/bridge/binding_object.dart (1)
386-427: Layout‑aware flush before geometry binding callsThe
_layoutDependentBindingProperties/_layoutDependentBindingMethodsfilters plus_shouldFlushLayoutForBindingCalland_flushFlutterLayoutIfSafegive a targeted layout flush only for geometry‑sensitive calls and only in safe scheduler phases, which is a good balance between correctness and overhead. Please just double‑check that all entry points into_invokeBindingMethodFromNativeImploccur whenSchedulerBinding/RendererBindingare already initialized, as a call very early in app startup could otherwise trip theirinstanceaccessors.Also applies to: 442-444
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
integration_tests/snapshots/blink/css1/box_properties/padding_left.ts.f039ce881.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-flexbox/relayout-align.ts.6eab38941.pngis excluded by!**/*.pngintegration_tests/snapshots/css/css-variables/variable-declaration-57.html.889f7f911.pngis excluded by!**/*.png
📒 Files selected for processing (32)
bridge/bindings/qjs/qjs_function.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/binding_object.ccbridge/core/css/invalidation/invalidation_set_test.ccbridge/core/css/invalidation/pending_invalidations.ccbridge/core/css/invalidation/pending_invalidations_test.ccbridge/core/css/style_engine.ccbridge/core/css/style_engine.hbridge/core/dom/document.ccbridge/core/dom/document.hbridge/core/dom/element.ccbridge/core/dom/element.hbridge/core/dom/frame_request_callback_collection.ccbridge/core/executing_context.ccbridge/core/executing_context.hbridge/core/frame/script_idle_task_controller.ccbridge/core/html/parser/html_parser.ccbridge/core/page.hbridge/include/webf_bridge.hbridge/polyfill/src/test/index.jsbridge/test/css_unittests.cmakebridge/webf_bridge.ccintegration_tests/lib/bridge/from_native.dartintegration_tests/lib/playground.dartintegration_tests/lib/webf_tester.dartintegration_tests/specs/blink/css1/box_properties/padding_left.tsintegration_tests/specs/css/css-text-mixin/text_comprehensive_test.tswebf/lib/src/bridge/binding_object.dartwebf/lib/src/bridge/to_native.dartwebf/lib/src/css/style_declaration.dartwebf/lib/src/dom/element_widget_adapter.dartwebf/lib/src/launcher/view_controller.dart
🚧 Files skipped from review as they are similar to previous changes (13)
- bridge/core/executing_context.h
- bridge/core/binding_object.cc
- bridge/core/css/style_engine.h
- webf/lib/src/css/style_declaration.dart
- bridge/core/css/invalidation/pending_invalidations.cc
- bridge/core/css/invalidation/invalidation_set_test.cc
- bridge/core/frame/script_idle_task_controller.cc
- bridge/core/css/invalidation/pending_invalidations_test.cc
- bridge/test/css_unittests.cmake
- integration_tests/lib/bridge/from_native.dart
- webf/lib/src/launcher/view_controller.dart
- bridge/core/dom/document.h
- integration_tests/specs/css/css-text-mixin/text_comprehensive_test.ts
🧰 Additional context used
📓 Path-based instructions (8)
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/to_native.dartwebf/lib/src/bridge/binding_object.dartwebf/lib/src/dom/element_widget_adapter.dart
{bridge/**/*.{cc,h},webf/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Document memory ownership clearly in FFI implementations
Files:
webf/lib/src/bridge/to_native.dartbridge/bindings/qjs/qjs_function.ccbridge/core/dom/element.ccbridge/core/dom/document.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.hwebf/lib/src/dom/element_widget_adapter.dartbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.cc
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/bindings/qjs/qjs_function.ccbridge/core/dom/element.ccbridge/core/dom/document.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/include/webf_bridge.hbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.cc
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/bindings/qjs/qjs_function.ccbridge/core/dom/element.ccbridge/core/dom/document.ccbridge/core/dom/frame_request_callback_collection.ccbridge/webf_bridge.ccbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.cc
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/bindings/qjs/qjs_function.ccbridge/core/dom/element.ccbridge/core/dom/document.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/webf_bridge.ccbridge/core/dom/element.hbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/include/webf_bridge.hbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.cc
bridge/core/**
📄 CodeRabbit inference engine (bridge/AGENTS.md)
Core bridge code should be located in
bridge/core/directory
Files:
bridge/core/dom/element.ccbridge/core/dom/document.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/core/dom/element.hbridge/core/css/style_engine.ccbridge/core/html/parser/html_parser.ccbridge/core/executing_context.cc
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/core/page.hbridge/core/dom/element.hbridge/include/webf_bridge.h
integration_tests/specs/**/*.ts
📄 CodeRabbit inference engine (integration_tests/CLAUDE.md)
integration_tests/specs/**/*.ts: Place tests in appropriate directories underspecs/(css/, dom/, or window/)
Use TypeScript (.ts extension) for test files
Usedone()callback for async tests
Usesnapshot()for visual regression tests to capture current rendering
UsesimulateClickwith coordinates for hit testing tests
Test assets should reference files inassets/directory
Usefdescribe()instead ofdescribe()to run only specific test specs (Jasmine feature)
Usefit()instead ofit()to run only specific test cases
Snapshots are stored as images for comparison and failed snapshots generate diff images
The max width of testing window is 340px
Test specs will always pass if there are no existing snapshots
Group related tests in describe blocks
Each test should be independent
Remove created elements after tests (test cleanup)
Use clear, descriptive test names
Test behavior, not implementation
Include edge cases and error conditions in tests
Batch DOM operations to minimize reflows
Use async/await and proper async patterns for asynchronous operations in tests
Measure operations usingperformance.now()for timing in performance-critical tests
Files:
integration_tests/specs/blink/css1/box_properties/padding_left.ts
🧠 Learnings (44)
📚 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/to_native.dartbridge/bindings/qjs/qjs_function.ccintegration_tests/lib/webf_tester.dartbridge/webf_bridge.ccintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.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} : 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/to_native.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.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/to_native.dartbridge/webf_bridge.ccintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.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} : Use `PostToDart` for returning results to Dart isolate
Applied to files:
webf/lib/src/bridge/to_native.dartintegration_tests/lib/webf_tester.dartbridge/webf_bridge.ccintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.h
📚 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:
webf/lib/src/bridge/to_native.dartbridge/core/dom/element.ccbridge/core/css/style_engine.ccbridge/include/webf_bridge.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/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:
webf/lib/src/bridge/to_native.dartbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.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/to_native.dartbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.h
📚 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/bridge/to_native.dartintegration_tests/lib/webf_tester.dartintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.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 webf/**/*.dart : Track pointer ownership in callbacks within Dart FFI
Applied to files:
webf/lib/src/bridge/to_native.dartbridge/bindings/qjs/qjs_function.ccintegration_tests/lib/webf_tester.dartintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.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} : 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
Applied to files:
webf/lib/src/bridge/to_native.dartbridge/bindings/qjs/qjs_function.ccbridge/webf_bridge.ccwebf/lib/src/bridge/binding_object.dartbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.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 : Call updateWidget() when attributes change in WidgetElement implementations
Applied to files:
webf/lib/src/bridge/to_native.dartbridge/include/webf_bridge.hwebf/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 bridge/**/*.{cc,h} : Use `PostToJs` for executing operations on the JS thread in FFI
Applied to files:
bridge/bindings/qjs/qjs_function.ccbridge/webf_bridge.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} : Avoid `PostToJsSync` when threads may interdepend to prevent synchronous deadlocks in FFI communication
Applied to files:
bridge/bindings/qjs/qjs_function.ccbridge/core/dom/frame_request_callback_collection.ccbridge/core/page.hbridge/webf_bridge.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.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/**/*.{cpp,h,hpp} : Use RAII patterns in C++ where possible for memory management
Applied to files:
bridge/bindings/qjs/qjs_function.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} : 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/dom/element.ccbridge/webf_bridge.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/dom/element.ccbridge/webf_bridge.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} : Ensure callback functions aren't invoked after context destruction to prevent use-after-free errors in FFI async operations
Applied to files:
bridge/core/dom/frame_request_callback_collection.ccbridge/bindings/qjs/script_promise_resolver.cc
📚 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} : Avoid `PostToJsSync` synchronous execution when possible
Applied to files:
bridge/core/dom/frame_request_callback_collection.ccbridge/bindings/qjs/script_promise_resolver.ccbridge/core/executing_context.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} : 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
Applied to files:
bridge/core/dom/frame_request_callback_collection.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 : Track ownership of allocated pointers in FFI callbacks
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dart
📚 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 webf/test/**/*.dart : Dart/Flutter widget and unit tests must be placed in `webf/test/` and use `WebFWidgetTestUtils` and `pumpAndSettle()` where needed
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Use WebFWidgetTestUtils.prepareWidgetTest() to test HTML/CSS rendering in widget unit tests
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Call setupTest() in setUpAll() method for one-time setup in unit tests
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.dart
📚 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 webf/integration_test/**/*.dart : Dart/Flutter integration tests must be placed in `webf/integration_test/` directory
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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: Run Flutter dart tests with: cd webf && flutter test
Applied to files:
integration_tests/lib/webf_tester.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/test/**/*_test.dart : When testing with WebFController, wait for initialization with: await controller.controlledInitCompleter.future
Applied to files:
integration_tests/lib/webf_tester.dartintegration_tests/lib/playground.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/test/**/*_test.dart : Access render objects in tests to verify layout calculations
Applied to files:
integration_tests/lib/webf_tester.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/test/**/*_test.dart : Use mock bundles from test/src/foundation/mock_bundle.dart for testing in unit tests
Applied to files:
integration_tests/lib/webf_tester.dartwebf/lib/src/bridge/binding_object.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/polyfill/**/*.{ts,tsx,d.ts} : Include a reference to `webf.d.ts` in polyfill type generation for accessing core types like `EventTarget`
Applied to files:
bridge/webf_bridge.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/core/**/*.d.ts : Create a `webf` namespace containing all type exports from merged bridge core types
Applied to files:
bridge/webf_bridge.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:
integration_tests/lib/playground.dartwebf/lib/src/bridge/binding_object.dartbridge/include/webf_bridge.h
📚 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: Use widget unit tests to verify rendering changes with: cd webf && flutter test test/src/rendering/
Applied to files:
integration_tests/lib/playground.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/rendering/**/*.dart : Use RenderBoxModel as base class for layout in Dart rendering code
Applied to files:
webf/lib/src/bridge/binding_object.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} : 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:
webf/lib/src/bridge/binding_object.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/bridge/binding_object.dartwebf/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 : Free FFI allocated memory in finally blocks to ensure cleanup on exceptions
Applied to files:
webf/lib/src/bridge/binding_object.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-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 : Implement ARIA attributes in WidgetElement when applicable for accessibility
Applied to files:
webf/lib/src/dom/element_widget_adapter.dart
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Batch DOM operations to minimize reflows
Applied to files:
bridge/polyfill/src/test/index.jsintegration_tests/specs/blink/css1/box_properties/padding_left.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Use `snapshot()` for visual regression tests to capture current rendering
Applied to files:
bridge/polyfill/src/test/index.js
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Measure operations using `performance.now()` for timing in performance-critical tests
Applied to files:
bridge/polyfill/src/test/index.js
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Test behavior, not implementation
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.ts
📚 Learning: 2025-12-08T23:27:27.888Z
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Remove created elements after tests (test cleanup)
Applied to files:
integration_tests/specs/blink/css1/box_properties/padding_left.ts
🧬 Code graph analysis (6)
bridge/core/dom/element.cc (1)
bridge/core/dom/element.h (1)
GetElementData(365-365)
bridge/core/dom/document.cc (1)
bridge/core/dom/document.h (1)
update_style_for_this_document_in_progress_(277-289)
bridge/core/page.h (1)
bridge/core/dom/document.h (2)
update_style_for_this_document_in_progress_(277-289)bool(278-279)
bridge/core/dom/element.h (1)
bridge/core/dom/element.cc (2)
Attributes(76-81)Attributes(76-76)
bridge/core/css/style_engine.cc (3)
bridge/core/dom/element.cc (2)
id(304-306)id(304-304)bridge/core/dom/node.h (1)
String(109-116)bridge/foundation/string/wtf_string.h (2)
String(23-160)String(42-42)
bridge/include/webf_bridge.h (1)
bridge/webf_bridge.cc (2)
updateStyleForThisDocument(309-363)updateStyleForThisDocument(309-309)
⏰ 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). (4)
- GitHub Check: build_windows_bridge (Debug)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (29)
bridge/polyfill/src/test/index.js (4)
9-13: LGTM!The generation counter pattern for invalidating RAF callbacks is a clean approach. The optional chaining with
.bind(global)correctly handles environments whererequestAnimationFramemay not exist while preserving the properthiscontext.
25-29: LGTM!Incrementing the generation counter effectively invalidates all pending RAF callbacks from the previous spec without needing to track individual RAF IDs. The comment clearly documents the intent.
58-72: LGTM!The wrapper correctly:
- Captures the generation synchronously to compare against at callback time
- Flushes the sync buffer defensively (with type check) before scheduling
- Returns the native RAF ID for
cancelAnimationFramecompatibility- Prevents stale callbacks from executing after spec teardown
The synchronous buffer flush before scheduling aligns well with the PR's goal of making style calculations reliable at RAF boundaries.
82-89: LGTM!The placement of
clearAllAnimationFrames()beforeresetDocumentElement()is correct—pending RAF callbacks from the previous spec are invalidated before the document is rebuilt, preventing them from interacting with the new DOM structure.bridge/core/html/parser/html_parser.cc (6)
35-37: LGTM!Clean wrapper function for document parsing. The null return case is properly handled by the caller.
39-44: LGTM!Proper fragment parsing setup. The hardcoded HTML namespace is appropriate since SVG fragments are handled separately by
parseSVG().
274-280: LGTM!Clean conditional logic for fragment vs. document parsing. The fragment context is properly derived from the root node.
412-442: LGTM!Correct implementation of fragment root flattening. The lambda properly unwraps Gumbo's synthetic
<html>/<head>/<body>wrappers to expose actual fragment content forinnerHTMLbehavior, while maintaining error state across all children.
444-444: LGTM!Correct conditional dispatch between fragment and document traversal modes.
46-60: GUMBO_TAG_BODY is the appropriate default for all fallback cases.The function correctly derives the context tag from
root_nodefor recognized element types (including table, select, colgroup). The defaults toGUMBO_TAG_BODYonly apply when context derivation fails—the node is null, the local name is empty, or the tag is unrecognized. For these edge cases,GUMBO_TAG_BODYis a reasonable and standard fallback in HTML fragment parsing.bridge/core/dom/frame_request_callback_collection.cc (1)
24-25: LGTM: Idle state management consistent with callback execution pattern.Setting the context to non-idle before firing the frame callback aligns with the broader idle-state management introduced in this PR. The subsequent
DrainMicrotasks()call (line 33) will properly manage microtask execution after the callback completes.webf/lib/src/dom/element_widget_adapter.dart (3)
62-62: LGTM: Onscreen state tracking added.The
_isOnScreenflag enables proper deduplication of onscreen/offscreen events, preventing redundant event dispatches during style-driven rebuilds.
107-121: LGTM: Event deduplication logic correctly implemented.The deduplication logic properly prevents redundant onscreen/offscreen events:
- Line 114: Early return if already onscreen when enqueuing an onscreen event
- Line 119: Early return if not onscreen when enqueuing an offscreen event
This aligns with the framework expectation that onscreen/offscreen are one-shot lifecycle notifications per visibility transition.
137-141: LGTM: State transitions occur before event dispatch.Setting
_isOnScreenbefore dispatching the event is correct—the flag tracks the state being entered, not the completion of dispatch. This enables subsequentenqueueScreenEventcalls to correctly deduplicate before the async dispatch completes.bridge/core/dom/element.h (2)
233-238: LGTM: Display-none invalidation flag for style optimization.The new flag enables the style engine to skip descendant invalidations for
display:nonesubtrees, mirroring Blink's optimization pattern. The inline documentation clearly explains the purpose and usage.
423-428: Dual-storage pattern is intentional for legacy compatibility.The fallback check in
hasAttributes()correctly handles the dual-storage design:element_data_(modern C++ storage) andattributes_(legacy ElementAttributes wrapper, marked for refactoring). These maintain independent storage (ElementAttributeshas an internalstd::unordered_map), so both must be checked to accurately reflect the element's attribute state. Methods likesetAttribute(),getAttribute(), andremoveAttribute()useEnsureElementAttributes()independently, which can populate the legacy storage without necessarily creatingelement_data_. The divergence is expected and essential for the legacy/modern migration path. Consider adding a code comment documenting this dual-storage relationship if not already present elsewhere.bridge/bindings/qjs/qjs_function.cc (2)
14-14: LGTM: ExecutingContext header added for idle-state management.
85-86: LGTM: Idle state management for function invocation.Setting the context to non-idle before
JS_Call(line 88) aligns with the broader idle-state management pattern. The subsequentDrainMicrotasks()call (line 90) properly handles microtask execution after the function completes.integration_tests/lib/webf_tester.dart (1)
49-49: Test context API updated to use opaque pointer.The test framework API now uses
Pointer<Void> testContextinstead ofdouble contextId, matching the change inplayground.dart. The non-null assertion is safe assuminginitTestFrameworkalways succeeds (line 48).bridge/core/dom/document.cc (1)
35-35: Reentrancy guard forUpdateStyleForThisDocumentlooks correctUsing
update_style_for_this_document_in_progress_withAutoReset<bool>gives a simple, robust guard against recursive style updates, and the early return keeps the fast path cheap. No issues from a single-threaded Document usage standpoint.Also applies to: 618-622
bridge/include/webf_bridge.h (1)
139-145: FFI surface forupdateStyleForThisDocumentis well‑shapedExport uses
WEBF_EXPORT_C,int8_tmatches DartInt8, and the documented 1/0 contract aligns with the C++ implementation and Dart wrapper.bridge/webf_bridge.cc (1)
8-8:updateStyleForThisDocumentorchestration looks good; confirm callback semanticsThe new bridge function correctly gates style updates with
TryBeginUpdateStyleForThisDocument, validates context/dispatcher, and on the JS thread rechecks context/Blink/idle/document before callingDocument::UpdateStyleForThisDocument()underMemberMutationScope, finishing with akFinishRecordingCommand. The in‑progress flag is only cleared in the completion lambda passed toPostToJsAndCallback, so please confirm thatDispatcher::PostToJsAndCallbackalways runs that completion callback even when the JS‑thread lambda early‑returns due to failed guards; otherwise the page‑level in‑progress flag could remain stucktrueand block future style updates.Also applies to: 10-10, 309-363
bridge/core/page.h (1)
147-160: Per‑page style‑update in‑progress tracking is implemented correctlyThe atomic
update_style_for_this_document_in_progress_plusTryBegin…/End…/Is…accessors give a clear and thread‑safe guard around per‑page style updates, with appropriate acquire/release semantics on the flag.Also applies to: 171-171
webf/lib/src/bridge/to_native.dart (1)
295-306: Dart FFI binding forupdateStyleForThisDocumentmatches the native APIThe typedefs line up with
int8_t updateStyleForThisDocument(void* page)and thebool updateStyleForThisDocument(Pointer<Void> page)wrapper provides a simple, idiomatic Dart interface.bridge/core/css/style_engine.cc (1)
1165-1171: Display‑none tracking and size‑dependent media‑query gating look soundThe new
display_none_for_invalidationcomputation andSetDisplayNoneForStyleInvalidationcalls in both subtree and element‑only recalc paths give the element a clear signal for “display:none” when determining invalidation behavior. The media‑query size‑change handling inMediaQueryAffectingValueChanged(TreeScope overload) correctly builds a deterministic snapshot of size‑dependent queries, compares it against the cached results, and only forces a full style/invalidation pass when the set or match results actually change, which should materially reduce unnecessary style work on pure resizes.Also applies to: 1451-1509, 1954-2090
bridge/core/dom/element.cc (4)
117-124: LGTM! Pre-binding style update pattern is correct.The addition of Blink-enabled style updates before geometry queries ensures layout is up-to-date. The null checks for context and document are appropriate, and the
MemberMutationScopeguard properly tracks mutations during the update.
140-147: LGTM! Consistent with getBoundingClientRect pattern.The same pre-binding style update logic is correctly applied here.
624-624: LGTM! Correct return value.Returning the actual
local_name_instead of a placeholder is the correct implementation for selector matching.
1082-1082: LGTM! Intentional removal of baseHref dependency.Setting
payload->hreftonullptrconsistently simplifies the inline style handling and removes the baseHref dependency as described in the PR objectives.
| if (local_name.IsNull() || local_name.empty()) { | ||
| return false; | ||
| } | ||
| for (const Attribute& attribute : GetElementData()->Attributes()) { | ||
| if (hint == attribute.LocalName()) { | ||
| return true; | ||
|
|
||
| if (HasElementData()) { | ||
| for (const Attribute& attribute : GetElementData()->Attributes()) { | ||
| if (attribute.LocalName() == local_name) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| */ | ||
|
|
||
| if (attributes_ != nullptr) { | ||
| ExceptionState exception_state; | ||
| return attributes_->hasAttribute(local_name, exception_state); | ||
| } | ||
|
|
||
| return false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for methods that access both element_data_ and attributes_
rg -n 'element_data_|attributes_' bridge/core/dom/element.cc | head -30Repository: openwebf/webf
Length of output: 1838
🏁 Script executed:
# Check for attribute synchronization or delegation patterns
rg -B3 -A3 'attributes_->has|HasElementData.*attributes_|element_data_.*Attributes' bridge/core/dom/element.ccRepository: openwebf/webf
Length of output: 213
🏁 Script executed:
# Look for the initialization and lifecycle of both sources
rg -n 'attributes_ =' bridge/core/dom/element.ccRepository: openwebf/webf
Length of output: 167
🏁 Script executed:
# Check element.h for how these two members are declared and documented
rg -B2 -A2 'ElementAttributes.*attributes_|ElementData.*element_data_' bridge/core/dom/element.hRepository: openwebf/webf
Length of output: 668
Dual attribute source pattern is intentional; ensure all code paths maintain synchronization.
The dual-check pattern in HasAttributeIgnoringNamespace is confirmed to be intentional. The method checks element_data_ (primary source) first, then attributes_ (lazily-initialized secondary source) as a fallback. Code like CloneDataFromElement() explicitly synchronizes both sources together via EnsureElementAttributes().CopyWith(), confirming they are meant to coexist.
However, the relationship between these two attribute sources would benefit from explicit comments documenting when and why both exist, and clarifying that all code paths that mutate attributes must keep both sources synchronized to avoid inconsistency.
🤖 Prompt for AI Agents
In bridge/core/dom/element.cc around lines 628 to 645, the dual attribute-source
pattern (element_data_ primary and attributes_ fallback) is intentional but
lacks explanatory comments and a clear instruction for mutators to keep both
sources synchronized; add a concise comment above HasAttributeIgnoringNamespace
(and/or at the Element class attribute section) explaining why both sources
exist, when attributes_ may be null, and that any code path that mutates
attributes must update both sources (use EnsureElementAttributes().CopyWith() /
CloneDataFromElement() or equivalent helper) to maintain consistency; also add a
note referencing the canonical synchronization helpers to guide future changes.
| Pointer<Void> testContext = initTestFramework(contextId); | ||
| registerDartTestMethodsToCpp(contextId); | ||
| registerDartTestMethodsToCpp(testContext); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n integration_tests/lib/playground.dart | sed -n '110,125p'Repository: openwebf/webf
Length of output: 817
🏁 Script executed:
rg -n 'initTestFramework|registerDartTestMethodsToCpp|testContext' integration_tests/lib/playground.dart -B2 -A5Repository: openwebf/webf
Length of output: 614
🏁 Script executed:
# Find initTestFramework and registerDartTestMethodsToCpp definitions
rg -n 'initTestFramework|registerDartTestMethodsToCpp' --type=dart -B2 -A8Repository: openwebf/webf
Length of output: 4629
🏁 Script executed:
# Check for FFI handle management patterns
rg -n 'Dart_NewPersistentHandle|Dart_DeletePersistentHandle|Pointer<Void>' webf --type=dart --type cpp -A3 -B1 | head -100Repository: openwebf/webf
Length of output: 8475
🏁 Script executed:
# Check the native implementation of initTestFramework and how it uses the returned pointer
rg -n 'initTestFramework' bridge --type=cpp --type=cc -B5 -A15 | head -80Repository: openwebf/webf
Length of output: 83
🏁 Script executed:
# Check if testContext is used in callbacks or async operations
rg -n 'testContext|registerDartTestMethodsToCpp' bridge --type=cpp --type=cc -B2 -A5 | head -100Repository: openwebf/webf
Length of output: 83
🏁 Script executed:
# Look at the broader lifecycle in webf_tester.dart to see if testContext is freed anywhere
rg -n 'testContext|dispose|cleanup' integration_tests/lib/webf_tester.dart -B2 -A3Repository: openwebf/webf
Length of output: 964
🏁 Script executed:
# Search for initTestFramework in C++ files (correct extension)
find bridge -type f \( -name "*.cpp" -o -name "*.h" -o -name "*.hpp" \) | head -20Repository: openwebf/webf
Length of output: 833
🏁 Script executed:
# Search for initTestFramework implementation
rg -n 'initTestFramework' bridge -B5 -A15 --type-listRepository: openwebf/webf
Length of output: 446
🏁 Script executed:
# Try broader search
rg -n 'initTestFramework' bridge -B5 -A15 2>/dev/null | head -100Repository: openwebf/webf
Length of output: 5949
🏁 Script executed:
# Check if testContext/test framework pointers are freed anywhere
rg -n 'free.*testContext|delete.*testContext|testContext.*free|testContext.*delete' integration_tests --type=dartRepository: openwebf/webf
Length of output: 39
🏁 Script executed:
# Check if dispose/cleanup in WebFController or view cleans up testContext
rg -n 'dispose|cleanup|testContext' webf/lib/src/launcher/view_controller.dart -B2 -A5 | head -60Repository: openwebf/webf
Length of output: 2084
🏁 Script executed:
# Check if there's a cleanup function for test framework in the FFI bindings
rg -n 'freeTestFramework|deleteTestContext|cleanup.*Test|Test.*cleanup' integration_tests/lib/bridge -B2 -A3Repository: openwebf/webf
Length of output: 39
🏁 Script executed:
# Check if testContext cleanup is handled elsewhere
rg -n 'testContext|WebFTestContext' integration_tests --type=dart -B1 -A2Repository: openwebf/webf
Length of output: 3907
🏁 Script executed:
# Check the full lifecycle in webf_tester.dart dispose method
rg -n 'dispose|deactivate' integration_tests/lib/webf_tester.dart -B3 -A10Repository: openwebf/webf
Length of output: 631
🏁 Script executed:
# Check if WebFTestContext has a destructor that might be called automatically
rg -n 'WebFTestContext|~WebFTestContext' bridge -B2 -A5Repository: openwebf/webf
Length of output: 10202
🏁 Script executed:
# Look for any delete/free functions for test context
fd -e h -e hpp bridge | xargs rg -l 'testContext|TestContext' | head -5Repository: openwebf/webf
Length of output: 90
🏁 Script executed:
# Verify the disposeCallback is called when controller.dispose() is invoked
rg -n 'disposeCallback|dispose' webf/lib/src/launcher/controller.dart -B2 -A3 | head -80Repository: openwebf/webf
Length of output: 2735
Add cleanup for testContext in playground.dart.
The testContext pointer allocated in initTestFramework must be cleaned up. In playground.dart, the pointer is allocated and registerDartTestMethodsToCpp is called, but executeTest is commented out and there's no corresponding controller.dispose() to trigger cleanup via the disposeCallback. Either uncomment executeTest or ensure the controller is disposed to properly release the test context (the native destructor is called automatically when the WebFPage is destroyed during controller disposal).
🤖 Prompt for AI Agents
In integration_tests/lib/playground.dart around lines 115 to 116, testContext
returned from initTestFramework is not being cleaned up because executeTest is
commented out and controller.dispose() is never called; either uncomment the
executeTest call so the test flow runs and disposes the controller, or
explicitly call controller.dispose() (preferably in a finally block or after
test completion) to trigger the native disposeCallback that frees the
testContext pointer and ensure no native memory leak.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bridge/core/dom/element.cc (1)
117-124: Consider extracting duplicate pre-binding style update logic.The same pattern (get context → check Blink enabled → get document → create mutation scope → update style) is duplicated in
getClientRects(lines 140-147). This could be extracted into a helper method to reduce duplication and ensure consistent behavior across geometry-querying methods.🔎 Suggested helper extraction
+void Element::EnsureStyleUpdatedForGeometryQuery() { + ExecutingContext* context = GetExecutingContext(); + if (context && context->isBlinkEnabled()) { + Document* doc = context->document(); + if (doc) { + MemberMutationScope mutation_scope{context}; + doc->UpdateStyleForThisDocument(); + } + } +} + BoundingClientRect* Element::getBoundingClientRect(ExceptionState& exception_state) { - ExecutingContext* context = GetExecutingContext(); - if (context && context->isBlinkEnabled()) { - Document* doc = context->document(); - if (doc) { - MemberMutationScope mutation_scope{context}; - doc->UpdateStyleForThisDocument(); - } - } + EnsureStyleUpdatedForGeometryQuery(); NativeValue result = InvokeBindingMethod(
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bridge/core/css/style_engine.ccbridge/core/dom/element.ccuse_cases/src/pages/FeatureCatalogPage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- bridge/core/css/style_engine.cc
🧰 Additional context used
📓 Path-based instructions (5)
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/core/dom/element.cc
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/dom/element.cc
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/core/dom/element.cc
{bridge/**/*.{cc,h},webf/**/*.dart}
📄 CodeRabbit inference engine (CLAUDE.md)
Document memory ownership clearly in FFI implementations
Files:
bridge/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/dom/element.cc
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: openwebf/webf PR: 0
File: integration_tests/CLAUDE.md:0-0
Timestamp: 2025-12-08T23:27:27.888Z
Learning: Applies to integration_tests/specs/**/*.ts : Batch DOM operations to minimize reflows
📚 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/core/dom/element.cc
⏰ 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). (4)
- GitHub Check: build_windows_bridge (Debug)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Analyze (rust)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
use_cases/src/pages/FeatureCatalogPage.tsx (1)
47-47: Catalog item structure and routing are correct.The
/responsiveroute is properly defined in the router configuration with the corresponding ResponsivePage component, so this catalog entry will navigate correctly.bridge/core/dom/element.cc (4)
140-147: LGTM - Style update before geometry query.The pre-binding style update ensures styles are recalculated before querying client rects, which aligns with the PR's goal of vsync-driven style calculations. The duplicate pattern was noted in the previous comment.
623-625: LGTM - Critical fix for selector matching.Returning the actual
local_name_instead of an empty placeholder is essential for CSS selector matching to work correctly with tag name selectors.
1067-1074: LGTM - CSS variable empty value normalization.The code correctly handles empty CSS variable values by normalizing them to a single space. The previous critical issue (using
value_stringbefore declaration) has been properly addressed - the variable is now declared at line 1068 before the check at line 1072.
1080-1084: No issue found.The removal of conditional
base_hreflogic is safe. Inline styles resolve relative URLs through the fallback mechanism (baseHref ?? controller.url) on the Dart side, which correctly uses the document's base URL—the appropriate base for inline style properties in thestyleattribute.
| if (attributes_ != nullptr) { | ||
| ExceptionState exception_state; | ||
| return attributes_->hasAttribute(local_name, exception_state); | ||
| } |
There was a problem hiding this comment.
Exception state is created but not checked after use.
The exception_state created at line 641 is passed to hasAttribute but never inspected afterward. If an exception occurs, it will be silently ignored. Consider either checking and handling the exception or using ASSERT_NO_EXCEPTION() if exceptions are not expected here.
🔎 Proposed fix
if (attributes_ != nullptr) {
- ExceptionState exception_state;
- return attributes_->hasAttribute(local_name, exception_state);
+ return attributes_->hasAttribute(local_name, ASSERT_NO_EXCEPTION());
}Or if exceptions should be handled:
if (attributes_ != nullptr) {
ExceptionState exception_state;
- return attributes_->hasAttribute(local_name, exception_state);
+ bool result = attributes_->hasAttribute(local_name, exception_state);
+ if (exception_state.HasException()) {
+ return false;
+ }
+ return result;
}🤖 Prompt for AI Agents
In bridge/core/dom/element.cc around lines 640 to 643, an ExceptionState is
created and passed into attributes_->hasAttribute(local_name, exception_state)
but the exception_state is never inspected; update the call site to either check
exception_state after the call and handle/report any exception (e.g., return a
sensible default or propagate the error) or, if exceptions are not expected
here, replace the no-op check with ASSERT_NO_EXCEPTION() immediately after the
hasAttribute call to document and enforce that no exception occurred.
This PR changes how we drive style calculations. Before this PR, the style calculation was unstable and could be unexpectedly erratic.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.