feat: implement CSSOM support for CSSStyleSheet and CSSRuleList #783

CGQAQ · 2026-01-02T13:15:49Z

Summary:

Implement CSSOM support for CSSStyleSheet and CSSRuleList.

Test Plan:

(not run locally)

Notes:

Please review API surface and behavior parity.

Summary by CodeRabbit

New Features
- Exposed document.styleSheets to access all stylesheets in a document
- Added insertRule() and deleteRule() methods to CSSStyleSheet for dynamic CSS rule management
- Added cssRules property to access rule collections from stylesheets
- Made CSSRuleList indexable with numeric keys and added item() method for rule access
- Added type and cssText properties to CSSRule for rule metadata

_{✏️ Tip: You can customize this high-level summary in your review settings.}

- Added document.styleSheets and wired it to return the document-order list of <style>/<link> sheets (bridge/core/dom/ document.cc:305, bridge/core/dom/document.d.ts:22). - Exposed the needed CSSOM surface for tests: CSSStyleSheet.cssRules/insertRule/deleteRule and CSSRuleList.length/item/ [i] (bridge/core/css/css_style_sheet.d.ts:4, bridge/core/css/css_rule_list.d.ts:3). - Implemented rule insertion/deletion + mutation invalidation so insertRule() actually updates the underlying sheet (bridge/core/css/style_sheet_contents.cc:267, bridge/core/css/css_style_sheet.cc:165) and fixed StyleEngine::UnregisterAuthorSheet to not drop shared contents (bridge/core/css/style_engine.h:342). - Added + ran a Blink-enabled unit test: HTMLStyleElement.documentStyleSheetsInsertRule (bridge/core/html/ html_style_element_test.cc:50) via ./bridge/cmake-build-debug-webf2/webf_unit_test --gtest_filter=HTMLStyleElement.documentStyleSheetsInsertRule.

vercel · 2026-01-02T13:15:53Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
use-case	Ready	Preview, Comment	Jan 2, 2026 9:28pm

coderabbitai · 2026-01-02T13:16:08Z

📝 Walkthrough

Walkthrough

The PR expands CSS Object Model (CSSOM) support by adding rule manipulation methods to CSSStyleSheet, exposing stylesheet access via Document.styleSheets, implementing named property access for CSSRuleList, and refactoring method signatures to remove unnecessary ExceptionState parameters. Includes style engine integration for rule insertion/deletion and comprehensive tests.

Changes

Cohort / File(s)	Summary
CSSRule Type Definitions `bridge/core/css/css_rule.d.ts`, `bridge/core/css/css_rule.h`	Added `readonly type: double` and `readonly cssText: string` properties to CSSRule interface; introduced `ImplType` type alias for C++ binding consistency.
CSSRuleList Implementation `bridge/core/css/css_rule_list.d.ts`, `bridge/core/css/css_rule_list.h`, `bridge/core/css/css_rule_list.cc`	Expanded public API with `length` property, `item(index)` method, and numeric index signature; implemented `NamedPropertyQuery` and `NamedPropertyEnumerator` for named property access and enumeration.
CSSStyleSheet Refactoring `bridge/core/css/css_style_sheet.d.ts`, `bridge/core/css/css_style_sheet.h`, `bridge/core/css/css_style_sheet.cc`	Removed ExceptionState parameters from `cssRules()` and `rules()` methods; added `insertRule` and `deleteRule` public API; enhanced mutation handling with StyleEngine integration for CSSOM wrapper management and rule-set cache invalidation.
Style Engine Updates `bridge/core/css/style_engine.h`, `bridge/core/css/style_sheet_contents.cc`	Implemented `WrapperInsertRule` and `WrapperDeleteRule` with full validation and import/child rule separation logic; enhanced `UnregisterAuthorSheet` with conditional erasure based on remaining sheet references.
Document API Expansion `bridge/core/dom/document.d.ts`, `bridge/core/dom/document.h`, `bridge/core/dom/document.cc`	Added `styleSheets()` accessor that traverses document tree to collect all CSSStyleSheet instances from style and link elements.
Tests & Integration `bridge/core/html/html_style_element_test.cc`, `integration_tests/specs/cssom/document-styleSheets.ts`	Added unit tests covering insertRule behavior, CSSOM cache safety, async microtask paths, and computed style stability; integration tests validating document.styleSheets updates, rule insertion/deletion, and cascading order.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant CSSStyleSheet
    participant StyleSheetContents
    participant StyleEngine
    participant Document

    Client->>CSSStyleSheet: insertRule(rule, index)
    activate CSSStyleSheet
    CSSStyleSheet->>StyleSheetContents: WrapperInsertRule(rule, index)
    activate StyleSheetContents
    StyleSheetContents->>StyleSheetContents: Validate index & rule type
    Note over StyleSheetContents: Import rules stay in import section<br/>Non-import rules after imports
    StyleSheetContents->>StyleSheetContents: Update import/child rule vectors
    StyleSheetContents->>StyleSheetContents: ClearRuleSet()
    StyleSheetContents-->>CSSStyleSheet: Success/Failure
    deactivate StyleSheetContents
    
    CSSStyleSheet->>StyleEngine: Mark style dirty (via WillMutate/DidMutate)
    activate StyleEngine
    StyleEngine->>StyleEngine: Clear cached wrappers
    StyleEngine->>StyleEngine: Temporarily unregister author sheet
    StyleEngine->>StyleEngine: Re-register author sheet
    StyleEngine->>StyleEngine: Schedule full recomputation
    StyleEngine-->>CSSStyleSheet: Acknowledged
    deactivate StyleEngine
    
    CSSStyleSheet-->>Client: Return insertion status
    deactivate CSSStyleSheet

sequenceDiagram
    participant Client
    participant Document
    participant TreeWalker
    participant HTMLStyleElement
    participant HTMLLinkElement
    participant CSSStyleSheet

    Client->>Document: styleSheets()
    activate Document
    Document->>TreeWalker: Traverse document tree
    activate TreeWalker
    loop For each Element node
        TreeWalker->>HTMLStyleElement: Is <style> element?
        alt Yes
            HTMLStyleElement-->>Document: Extract CSSStyleSheet
        end
        TreeWalker->>HTMLLinkElement: Is <link rel=stylesheet> element?
        alt Yes
            HTMLLinkElement-->>Document: Extract CSSStyleSheet
        end
    end
    deactivate TreeWalker
    
    Document-->>Client: std::vector<CSSStyleSheet*>
    deactivate Document

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

The review requires careful analysis of rule insertion/deletion validation logic, style engine integration with copy-on-write semantics, and interaction across multiple subsystems (Document, StyleEngine, StyleSheetContents, CSSStyleSheet). The heterogeneous changes span type definitions, C++ implementation with complex control flow, signature modifications, and test coverage additions.

Possibly Related PRs

feat: Use vsync to drive style calculations #777: Overlaps with CSS style subsystem modifications including StyleEngine and Document style-update/invalidation paths; both PRs refactor style recalculation workflows and expand bridge APIs for stylesheet access.

Poem

🐰 Hop, hop, hop—new CSS rules spring to life!
StyleSheets dance through documents, free from strife,
Insert, delete, query with named property delight,
The CSSOM grows complete—a web developer's delight!
🎨✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.57% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately summarizes the main change: implementing CSSOM support for CSSStyleSheet and CSSRuleList, which matches the primary objective of the pull request.

✨ Finishing touches

📝 Generate docstrings

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

bridge/core/css/css_style_sheet.cc (1)
379-383: Unreachable code after early return.

The exception_state.ThrowException on lines 381-382 is never executed because return 0 is on line 380.
🔎 Proposed fix
   if (!rule) {
-    return 0;
     exception_state.ThrowException(ctx(), ErrorType::InternalError,
                                    "Failed to parse the rule '" + rule_string.ToUTF8String() + "'.");
+    return 0;
   }

🧹 Nitpick comments (6)

bridge/core/css/css_rule_list.d.ts (1)
4-6: Consider type consistency for index parameters.

There's a minor inconsistency: length and item() use double for numeric types (lines 4-5), while the indexed signature uses number (line 6). While functionally equivalent, using number consistently would align better with TypeScript conventions:
-  readonly length: double;
-  item(index: double): CSSRule | null;
+  readonly length: number;
+  item(index: number): CSSRule | null;
   readonly [key: number]: CSSRule | null;
If double is a deliberate IDL-to-TypeScript mapping, this is acceptable but worth documenting.
integration_tests/specs/cssom/document-styleSheets.ts (2)
86-86: Replace fixed sleep with deterministic synchronization.

Using sleep(0.02) introduces timing-dependent flakiness. The style updates should be reflected after waitForFrame(), which already handles frame synchronization. Consider:

Relying solely on waitForFrame() if it's sufficient

Adding a helper that polls getComputedStyle() until the expected value appears

Using a more robust synchronization mechanism
Example polling helper
async function waitForStyle(element: Element, property: string, expectedValue: string, timeout = 1000) {
  const start = performance.now();
  while (performance.now() - start < timeout) {
    const value = getComputedStyle(element)[property];
    if (value.includes(expectedValue)) return true;
    await sleep(0.01);
  }
  throw new Error(`Timeout waiting for ${property} to be ${expectedValue}`);
}

// Usage:
await waitForStyle(target, 'color', '0, 128, 0');
Also applies to: 96-96

64-106: Test may be brittle due to baseline length assumption.

The test assumes baseLength remains stable, but if previous tests fail to clean up their style elements, this test's expectations would break. Consider:

Using a dedicated test container that's cleared before each test

Querying for specifically-marked test stylesheets rather than relying on count

Adding a beforeEach hook to ensure clean state

This is a recommended improvement for test suite robustness, though the current approach works if all tests properly clean up.
bridge/core/dom/document.cc (1)

305-328: Consider caching for performance optimization.

The implementation performs a full DOM traversal on every styleSheets() call. Evidence shows it's called multiple times in quick succession within the style update logic (style_node_manager.dart lines 95-97), making this a potential bottleneck. Since invalidation points already exist through InsertedInto() and RemovedFrom() hooks on style/link elements, a cache mechanism is feasible. However, given WebF's mobile constraints and typical document sizes, the current approach may be acceptable.

The logic correctly handles:

Empty documents (early return)

Safe type casting with DynamicTo<T>

Collection from both <style> and <link> elements
bridge/core/css/style_sheet_contents.cc (1)
28-30: Remove duplicate include.

<utility> is included twice.
🔎 Proposed fix
 #include <utility>
 
-#include <utility>
 #include "core/css/css_style_sheet.h"
bridge/core/html/html_style_element_test.cc (1)
94-133: Unused variable context.

The context variable on line 105 is declared but not used. The test only uses env->page()->evaluateScript and TEST_runLoop(context).
🔎 Proposed fix
-  auto* context = env->page()->executingContext();
   const char* code = R"(
   // Two empty <style> elements share the same cached StyleSheetContents. This
   // forces CSSStyleSheet::insertRule to take the copy-on-write path.
   ...
   )";

   env->page()->evaluateScript(code, strlen(code), "vm://", 0);
-  TEST_runLoop(context);
+  TEST_runLoop(env->page()->executingContext());

   EXPECT_EQ(errorCalled, false);

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f390b2c and f64100e.

📒 Files selected for processing (15)

bridge/core/css/css_rule.d.ts
bridge/core/css/css_rule.h
bridge/core/css/css_rule_list.cc
bridge/core/css/css_rule_list.d.ts
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.cc
bridge/core/css/css_style_sheet.d.ts
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/css/style_sheet_contents.cc
bridge/core/dom/document.cc
bridge/core/dom/document.d.ts
bridge/core/dom/document.h
bridge/core/html/html_style_element_test.cc
integration_tests/specs/cssom/document-styleSheets.ts

🧰 Additional context used

📓 Path-based instructions (7)

bridge/core/**

📄 CodeRabbit inference engine (bridge/AGENTS.md)

Core bridge code should be located in bridge/core/ directory

Files:

bridge/core/dom/document.d.ts
bridge/core/dom/document.cc
bridge/core/css/css_style_sheet.d.ts
bridge/core/css/css_rule.h
bridge/core/css/style_sheet_contents.cc
bridge/core/css/css_rule_list.d.ts
bridge/core/css/css_rule_list.cc
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/html/html_style_element_test.cc
bridge/core/dom/document.h
bridge/core/css/css_rule.d.ts
bridge/core/css/css_style_sheet.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
Use WEBF_EXPORT_C macro for exporting C functions to Dart FFI
In FFI contexts, use Dart_Handle instead of Handle for type compatibility
For C++ FFI function naming: use GetObjectPropertiesFromDart for C++ exports, NativeGetObjectPropertiesFunc for Dart typedefs, and GetObjectPropertiesFunc for 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/document.cc
bridge/core/css/css_rule.h
bridge/core/css/style_sheet_contents.cc
bridge/core/css/css_rule_list.cc
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/html/html_style_element_test.cc
bridge/core/dom/document.h
bridge/core/css/css_style_sheet.cc

bridge/**/*.{cc,cpp}

📄 CodeRabbit inference engine (bridge/CLAUDE.md)

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
For string handling in FFI, copy strings that might be freed using std::string(const_char_ptr), and use toNativeUtf8() 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 using std::string to ensure string lifetime, and consider error callbacks separate from result callbacks
Avoid PostToJsSync when 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/document.cc
bridge/core/css/style_sheet_contents.cc
bridge/core/css/css_rule_list.cc
bridge/core/html/html_style_element_test.cc
bridge/core/css/css_style_sheet.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
Use PostToJs for executing operations on the JS thread in FFI
Use PostToDart for returning results to Dart isolate
Avoid PostToJsSync synchronous execution when possible

C++ bridge code must use 2-space indent, 120 column line limit, and follow Chromium style via .clang-format

Files:

bridge/core/dom/document.cc
bridge/core/css/css_rule.h
bridge/core/css/style_sheet_contents.cc
bridge/core/css/css_rule_list.cc
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/html/html_style_element_test.cc
bridge/core/dom/document.h
bridge/core/css/css_style_sheet.cc

{bridge/**/*.{cc,h},webf/**/*.dart}

📄 CodeRabbit inference engine (CLAUDE.md)

Document memory ownership clearly in FFI implementations

Files:

bridge/core/dom/document.cc
bridge/core/css/css_rule.h
bridge/core/css/style_sheet_contents.cc
bridge/core/css/css_rule_list.cc
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/html/html_style_element_test.cc
bridge/core/dom/document.h
bridge/core/css/css_style_sheet.cc

integration_tests/specs/**/*.ts

📄 CodeRabbit inference engine (integration_tests/CLAUDE.md)

integration_tests/specs/**/*.ts: Place tests in appropriate directories under specs/ (css/, dom/, or window/)
Use TypeScript (.ts extension) for test files
Use done() callback for async tests
Use snapshot() for visual regression tests to capture current rendering
Use simulateClick with coordinates for hit testing tests
Test assets should reference files in assets/ directory
Use fdescribe() instead of describe() to run only specific test specs (Jasmine feature)
Use fit() instead of it() 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 using performance.now() for timing in performance-critical tests

Files:

integration_tests/specs/cssom/document-styleSheets.ts

bridge/**/*.{h,hpp}

📄 CodeRabbit inference engine (bridge/CLAUDE.md)

bridge/**/*.{h,hpp}: Ring buffer command structure should use enum Type : uint8_t for command types with union for type-specific data to ensure type-safe and cache-friendly command handling
Ring buffer implementation should use alignas(64) for atomic head and tail pointers, std::atomic<size_t> for thread-safe synchronization, and power-of-2 buffer sizes enforced with static_assert

Files:

bridge/core/css/css_rule.h
bridge/core/css/css_rule_list.h
bridge/core/css/css_style_sheet.h
bridge/core/css/style_engine.h
bridge/core/dom/document.h

🧠 Learnings (18)

📓 Common learnings

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/

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