Merge pull request #3518 from staylor/investigate/drops-watchdog-stall

Avoid spurious mutations for unchanged CSS declarations
This commit is contained in:
Karl Seguin authored and GitHub committed 2026-09-16 08:13:43 +08:00
commit 3864c1be3e
2 files changed
+188 -16

No files matched your search

@@ -0,0 +1,170 @@
<!DOCTYPE html>
<script src="../testing.js"></script>
<script id=absent_properties_do_not_mutate_style>
{
for (const initial of [null, '', 'color:red']) {
const element = document.createElement('div');
if (initial !== null) element.setAttribute('style', initial);
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true });
testing.expectEqual('', element.style.removeProperty('display'));
element.style.display = '';
element.style.setProperty('display', '');
element.style.cssFloat = '';
element.style.removeProperty('--absent');
testing.expectEqual(0, observer.takeRecords().length);
testing.expectEqual(initial, element.getAttribute('style'));
observer.disconnect();
}
}
</script>
<script id=identical_declarations_preserve_raw_style>
{
const element = document.createElement('div');
const raw = 'color:red; margin-top:0px; float:left; --token:x';
element.setAttribute('style', raw);
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true });
element.style.color = 'red';
element.style.setProperty('COLOR', 'red');
element.style.marginTop = '0';
element.style.cssFloat = 'left';
element.style.setProperty('--token', 'x');
testing.expectEqual(0, observer.takeRecords().length);
testing.expectEqual(raw, element.getAttribute('style'));
observer.disconnect();
}
</script>
<script id=priority_changes_and_empty_values>
{
const element = document.createElement('div');
element.style.setProperty('color', 'red', 'important');
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true, attributeOldValue: true });
element.style.setProperty('color', 'red', 'IMPORTANT');
testing.expectEqual(0, observer.takeRecords().length);
element.style.setProperty('color', 'blue', 'invalid');
testing.expectEqual(0, observer.takeRecords().length);
testing.expectEqual('red', element.style.color);
element.style.setProperty('color', 'red');
const priorityRecords = observer.takeRecords();
testing.expectEqual(1, priorityRecords.length);
testing.expectEqual('color: red !important;', priorityRecords[0].oldValue);
testing.expectEqual('', element.style.getPropertyPriority('color'));
element.style.setProperty('color', '', 'important');
const removed = observer.takeRecords();
testing.expectEqual(1, removed.length);
testing.expectEqual('color: red;', removed[0].oldValue);
testing.expectEqual('', element.getAttribute('style'));
element.style.setProperty('color', '', 'important');
testing.expectEqual(0, observer.takeRecords().length);
observer.disconnect();
}
</script>
<script id=real_changes_and_explicit_assignments_still_notify>
{
const element = document.createElement('div');
element.style.color = 'red';
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true, attributeOldValue: true });
element.style.color = 'blue';
let records = observer.takeRecords();
testing.expectEqual(1, records.length);
testing.expectEqual('style', records[0].attributeName);
testing.expectEqual('color: red;', records[0].oldValue);
testing.expectEqual('blue', element.style.color);
element.style.cssText = element.style.cssText;
testing.expectEqual(1, observer.takeRecords().length);
element.setAttribute('style', element.getAttribute('style'));
testing.expectEqual(1, observer.takeRecords().length);
testing.expectEqual('blue', element.style.removeProperty('COLOR'));
records = observer.takeRecords();
testing.expectEqual(1, records.length);
testing.expectEqual('color: blue;', records[0].oldValue);
testing.expectEqual('', element.style.removeProperty('color'));
testing.expectEqual(0, observer.takeRecords().length);
element.style.cssText = '';
testing.expectEqual(1, observer.takeRecords().length);
observer.disconnect();
}
</script>
<script id=css_float_priority_changes_notify>
{
const element = document.createElement('div');
element.style.setProperty('float', 'left', 'important');
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true });
element.style.cssFloat = 'left';
testing.expectEqual(1, observer.takeRecords().length);
testing.expectEqual('', element.style.getPropertyPriority('float'));
element.style.cssFloat = 'left';
testing.expectEqual(0, observer.takeRecords().length);
observer.disconnect();
}
</script>
<script id=custom_property_case_is_significant>
{
const element = document.createElement('div');
element.style.setProperty('--Token', 'x');
const observer = new MutationObserver(() => {});
observer.observe(element, { attributes: true });
element.style.setProperty('--token', 'x');
testing.expectEqual(1, observer.takeRecords().length);
element.style.setProperty('--Token', 'y');
testing.expectEqual(1, observer.takeRecords().length);
element.style.setProperty('--Token', 'y');
testing.expectEqual(0, observer.takeRecords().length);
testing.expectEqual('x', element.style.getPropertyValue('--token'));
testing.expectEqual('y', element.style.getPropertyValue('--Token'));
observer.disconnect();
}
</script>
<script id=observer_reapplying_style_converges>
(async () => {
const state = await testing.async();
const element = document.createElement('div');
let calls = 0;
const observer = new MutationObserver(() => {
if (++calls >= 4) observer.disconnect();
element.style.color = 'red';
element.style.removeProperty('display');
element.style.cssFloat = '';
});
observer.observe(element, { attributes: true });
element.style.color = 'red';
await new Promise(resolve => setTimeout(resolve, 0));
observer.disconnect();
state.resolve();
await state.done(() => {
testing.expectEqual(1, calls);
testing.expectEqual('red', element.style.color);
});
})();
</script>
<script id=healthy_batches_are_not_disconnected>
(async () => {
const state = await testing.async();
const element = document.createElement('div');
let delivered = 0;
const observers = Array.from({ length: 64 }, () => {
const observer = new MutationObserver(() => delivered++);
observer.observe(element, { attributes: true });
return observer;
});
for (let round = 0; round < 32; round++) {
element.setAttribute('data-round', String(round));
await new Promise(resolve => setTimeout(resolve, 0));
}
observers.forEach(observer => observer.disconnect());
state.resolve();
await state.done(() => testing.expectEqual(2048, delivered));
})();
</script>
+18 -16
View File
@@ -166,9 +166,9 @@ pub fn setProperty(self: *CSSStyleDeclaration, property_name: []const u8, value:
break :blk true;
} else false;
try self.setPropertyImpl(property_name, value, important, frame);
try self.syncStyleAttribute(frame);
if (try self.setPropertyImpl(property_name, value, important, frame)) {
try self.syncStyleAttribute(frame);
}
}
/// Apply one declaration parsed from a `style=` block. Unlike the imperative
@@ -181,7 +181,7 @@ fn applyParsedDeclaration(self: *CSSStyleDeclaration, declaration: CssParser.Dec
if (existing._important) return;
}
}
try self.setPropertyImpl(declaration.name, declaration.value, declaration.important, frame);
_ = try self.setPropertyImpl(declaration.name, declaration.value, declaration.important, frame);
}
fn initOwnedString(allocator: Allocator, value: []const u8) !String {
@@ -190,10 +190,9 @@ fn initOwnedString(allocator: Allocator, value: []const u8) !String {
return String.wrap(try allocator.dupe(u8, value));
}
fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: []const u8, important: bool, frame: *Frame) !void {
fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: []const u8, important: bool, frame: *Frame) !bool {
if (value.len == 0) {
_ = try self.removePropertyImpl(property_name, frame);
return;
return (try self.removePropertyImpl(property_name, frame)) != null;
}
const normalized = normalizePropertyName(property_name, &frame.buf);
@@ -203,12 +202,13 @@ fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value:
// Find existing property
if (self.findProperty(.wrap(normalized))) |existing| {
if (existing._value.eql(.wrap(normalized_value)) and existing._important == important) return false;
const allocator = frame._factory.storageAllocator();
const new_value = try initOwnedString(allocator, normalized_value);
existing._value.deinit(allocator);
existing._value = new_value;
existing._important = important;
return;
return true;
}
// Create new property
@@ -219,20 +219,21 @@ fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value:
._important = important,
});
self._properties.append(&prop._node);
return true;
}
pub fn removeProperty(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) ![]const u8 {
if (self._is_computed) {
return error.NoModificationAllowed;
}
const result = try self.removePropertyImpl(property_name, frame);
const result = (try self.removePropertyImpl(property_name, frame)) orelse return "";
try self.syncStyleAttribute(frame);
return result;
}
fn removePropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) ![]const u8 {
fn removePropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) !?[]const u8 {
const normalized = normalizePropertyName(property_name, &frame.buf);
const prop = self.findProperty(.wrap(normalized)) orelse return "";
const prop = self.findProperty(.wrap(normalized)) orelse return null;
// the value might not be on the heap (it could be inlined in the small string
// optimization), so we need to dupe it.
@@ -287,8 +288,9 @@ fn setFloat(self: *CSSStyleDeclaration, value_: ?[]const u8, frame: *Frame) !voi
if (self._is_computed) {
return error.NoModificationAllowed;
}
try self.setPropertyImpl("float", value_ orelse "", false, frame);
try self.syncStyleAttribute(frame);
if (try self.setPropertyImpl("float", value_ orelse "", false, frame)) {
try self.syncStyleAttribute(frame);
}
}
fn getCssText(self: *const CSSStyleDeclaration, frame: *Frame) ![]const u8 {
@@ -990,10 +992,10 @@ test "CSS property value storage is reused" {
var style = CSSStyleDeclaration{};
defer style.clearProperties(frame);
try style.setPropertyImpl("transform", "translate3d(1px,0,0)", false, frame);
try testing.expect(try style.setPropertyImpl("transform", "translate3d(1px,0,0)", false, frame));
const first_ptr = style.findProperty(comptime .wrap("transform")).?._value.suffix.ptr;
try style.setPropertyImpl("transform", "translate3d(2px,0,0)", false, frame);
try style.setPropertyImpl("transform", "translate3d(3px,0,0)", false, frame);
try testing.expect(try style.setPropertyImpl("transform", "translate3d(2px,0,0)", false, frame));
try testing.expect(try style.setPropertyImpl("transform", "translate3d(3px,0,0)", false, frame));
const property = style.findProperty(comptime .wrap("transform")).?;
try testing.expectEqual(first_ptr, property._value.suffix.ptr);