Skip to content

Commit 002515b

Browse files
authored
fix(blink): synchronize first-paint styles before UICommand flush (#827)
## Summary Fixes a Blink-first-paint “unstyled/incorrect style” window by deferring UICommand package flushing until a synchronous style update has run for the newly-mounted visual subtree. ## Problem When the Blink CSS engine is enabled, DOM mutations can enqueue/flush UICommand packages before Blink has performed the first `Document::UpdateStyleForThisDocument()`. This can lead to: - an unstyled first frame (style commands arrive in a later batch) - `getComputedStyle()` returning incorrect defaults during early mount - RouterLink (hybrid router) subpage mounts being especially prone to the issue ## Solution - Add a “first-paint style sync” barrier on `ExecutingContext`: - `needs_first_paint_style_sync_` gates UICommand package emission - `first_paint_committed_` prevents re-gating the initial document paint - Arm the barrier on structural DOM insertions: - `ContainerNode::{AppendChildCommon,InsertBeforeCommon}` call `ExecutingContext::MaybeBeginFirstPaintStyleSync(...)` - Special-case RouterLink: - do not block insertion of the RouterLink container into `<body>` (avoids hybrid-router deadlock) - re-arm when the RouterLink subtree (route contents) mounts its first child - Ensure the barrier clears deterministically: - `SharedUICommand::AddCommand` calls `MaybeUpdateStyleForFirstPaint()` on `kFinishRecordingCommand` to force a synchronous style update when required - `Document::UpdateStyleForThisDocument()` calls `MaybeCommitFirstPaintStyleSync()` after updating styles to finalize the barrier state - Defer UICommand packages while the barrier is active: - `UICommandPackageRingBuffer::PushPackage()` diverts packages into a deferred queue when gated - `FlushDeferredPackages()` pushes deferred packages once the barrier is cleared - `Reset/Clear` updated to account for deferred packages and lock ordering ## Tests - Bridge unit test: `bridge/foundation/blink_first_paint_style_sync_test.cc` - Verifies style commands (`kClearStyle`, `kSetStyleById`) appear in the first flushed batch. - Integration test: `integration_tests/specs/cssom/first_paint_barrier_sync_flush.ts` - Verifies `getComputedStyle()` is unblocked and returns expected values during RouterLink subtree mount. - Stabilize Blob constructor spec by waiting for a frame before `toBlob()`. ## How to test - `node scripts/run_bridge_unit_test.js` - `cd integration_tests && npm run integration` ## Notes / Risk - Changes only take effect when the Blink engine is enabled. - RouterLink handling is explicitly designed to avoid blocking hybrid-router navigation while still gating the first painted subtree.
2 parents a2925d5 + e9623b0 commit 002515b

File tree

12 files changed

+238
-6
lines changed

12 files changed

+238
-6
lines changed

bridge/core/dom/container_node.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,10 @@ void ContainerNode::InsertBeforeCommon(Node& next_child, Node& new_child) {
723723
assert(!new_child.nextSibling());
724724
assert(!new_child.previousSibling());
725725

726+
if (auto* context = GetExecutingContext()) {
727+
context->MaybeBeginFirstPaintStyleSync(*this, new_child, !hasChildren());
728+
}
729+
726730
Node* prev = next_child.previousSibling();
727731
assert(last_child_ != prev);
728732
next_child.SetPreviousSibling(&new_child);
@@ -744,6 +748,11 @@ void ContainerNode::InsertBeforeCommon(Node& next_child, Node& new_child) {
744748
}
745749

746750
void ContainerNode::AppendChildCommon(Node& child) {
751+
const bool was_empty = !hasChildren();
752+
if (auto* context = GetExecutingContext()) {
753+
context->MaybeBeginFirstPaintStyleSync(*this, child, was_empty);
754+
}
755+
747756
child.SetParentOrShadowHostNode(this);
748757
if (last_child_) {
749758
child.SetPreviousSibling(last_child_);

bridge/core/dom/document.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,7 @@ void Document::UpdateStyleForThisDocument() {
684684
style_engine.InvalidateEnvDependentStylesIfNeeded();
685685
UpdateStyleInvalidationIfNeeded();
686686
UpdateStyle();
687+
context->MaybeCommitFirstPaintStyleSync();
687688

688689
auto duration_ms =
689690
std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - start_time).count();

bridge/core/executing_context.cc

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "core/dom/mutation_observer.h"
2626
#include "core/css/style_engine.h"
2727
#include "core/events/error_event.h"
28+
#include "core/html/html_body_element.h"
2829
#include "core/html/html_script_element.h"
2930
#include "core/events/promise_rejection_event.h"
3031
#include "core/js_function_ref.h"
@@ -827,10 +828,64 @@ void ExecutingContext::SetWidgetElementShape(NativeWidgetElementShape* native_wi
827828
void ExecutingContext::EnableBlinkEngine() {
828829
WEBF_LOG(INFO) << "=== The Blink engine is enabled ===";
829830

831+
if (enable_blink_engine_) {
832+
return;
833+
}
834+
830835
enable_blink_engine_ = true;
831836
document()->EnsureStyleEngine();
832837
}
833838

839+
void ExecutingContext::MaybeBeginFirstPaintStyleSync(const ContainerNode& parent,
840+
const Node& child,
841+
bool parent_was_empty) {
842+
if (!enable_blink_engine_ || needs_first_paint_style_sync_) {
843+
return;
844+
}
845+
846+
// Re-arm the barrier for the first mount of a RouterLink subtree (subpage).
847+
if (parent.IsRouterLinkElement() && parent_was_empty) {
848+
needs_first_paint_style_sync_ = true;
849+
return;
850+
}
851+
852+
// Only gate the very first document paint once.
853+
if (first_paint_committed_) {
854+
return;
855+
}
856+
857+
// Avoid deadlocking hybrid-router navigation by *not* blocking the initial
858+
// insertion of RouterLink containers into <body>. Route children are mounted
859+
// later (into the RouterLink element), where we can safely gate.
860+
if (IsA<HTMLBodyElement>(parent) && !child.IsRouterLinkElement()) {
861+
needs_first_paint_style_sync_ = true;
862+
}
863+
}
864+
865+
void ExecutingContext::MaybeUpdateStyleForFirstPaint() {
866+
if (!enable_blink_engine_ || !needs_first_paint_style_sync_) {
867+
return;
868+
}
869+
870+
if (!document_) {
871+
return;
872+
}
873+
874+
MemberMutationScope mutation_scope{this};
875+
document_->UpdateStyleForThisDocument();
876+
needs_first_paint_style_sync_ = false;
877+
first_paint_committed_ = true;
878+
}
879+
880+
void ExecutingContext::MaybeCommitFirstPaintStyleSync() {
881+
if (!enable_blink_engine_ || !needs_first_paint_style_sync_) {
882+
return;
883+
}
884+
885+
needs_first_paint_style_sync_ = false;
886+
first_paint_committed_ = true;
887+
}
888+
834889
void ExecutingContext::FlushUICommand(const BindingObject* self, uint32_t reason) {
835890
std::vector<NativeBindingObject*> deps;
836891
FlushUICommand(self, reason, deps);

bridge/core/executing_context.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ class ScriptWrappable;
6666
class NativeByteDataFinalizerContext;
6767
class ScriptPromiseResolver;
6868
class HTMLScriptElement;
69+
class ContainerNode;
70+
class Node;
6971
struct NativeJSFunctionRef;
7072

7173
using JSExceptionHandler = std::function<void(ExecutingContext* context, const char* message)>;
@@ -214,6 +216,10 @@ class ExecutingContext {
214216
FORCE_INLINE bool isIdle() const { return is_idle_; }
215217
FORCE_INLINE void SetIsIdle(bool is_idle) { is_idle_ = is_idle; }
216218
FORCE_INLINE void MarkNeedsStyleUpdateInMicrotask() { is_needs_update_styles_in_microtask_ = true; }
219+
// If a "first-paint style sync" barrier is active, clear it after we've
220+
// performed a synchronous style update so deferred UICommand packages can be
221+
// flushed to Dart (e.g. for getComputedStyle()).
222+
void MaybeCommitFirstPaintStyleSync();
217223

218224
// Cached media/viewport values pushed from Dart (e.g., during resize). These
219225
// avoid synchronous GetBindingProperty calls (which may flush layout) during
@@ -272,6 +278,10 @@ class ExecutingContext {
272278
void InstallDocument();
273279
void InstallPerformance();
274280
void InstallNativeLoader();
281+
void MaybeBeginFirstPaintStyleSync(const ContainerNode& parent,
282+
const Node& child,
283+
bool parent_was_empty);
284+
void MaybeUpdateStyleForFirstPaint();
275285

276286
void DrainPendingPromiseJobs();
277287

@@ -323,6 +333,12 @@ class ExecutingContext {
323333
bool is_dedicated_;
324334
std::unique_ptr<RemoteObjectRegistry> remote_object_registry_;
325335
bool enable_blink_engine_ = false;
336+
// When Blink CSS is enabled, defer UICommand packages until we've run at
337+
// least one `Document::UpdateStyleForThisDocument()` for a newly-mounted
338+
// visual subtree, so the first visible frame ships with both DOM + styles.
339+
// This barrier can be re-armed (e.g., first mount of a RouterLink subtree).
340+
bool needs_first_paint_style_sync_{false};
341+
bool first_paint_committed_{false};
326342
bool is_idle_{true};
327343
bool is_needs_update_styles_in_microtask_ {false};
328344
std::optional<double> cached_viewport_width_;
@@ -335,6 +351,10 @@ class ExecutingContext {
335351

336352
// Native library metadata
337353
std::vector<NativeLibraryMetaData*> native_library_meta_data_contaner_;
354+
355+
friend class ContainerNode;
356+
friend class SharedUICommand;
357+
friend class UICommandPackageRingBuffer;
338358
};
339359

340360
class ObjectProperty {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#include "gtest/gtest.h"
2+
3+
#include <cstring>
4+
5+
#include "foundation/ui_command_buffer.h"
6+
#include "webf_test_env.h"
7+
8+
using namespace webf;
9+
10+
namespace {
11+
12+
bool HasCommand(const UICommandItem* items, int64_t length, UICommand command) {
13+
for (int64_t i = 0; i < length; ++i) {
14+
if (items[i].type == static_cast<int32_t>(command)) {
15+
return true;
16+
}
17+
}
18+
return false;
19+
}
20+
21+
} // namespace
22+
23+
TEST(BlinkFirstPaintStyleSync, EmitsSheetStyleCommandsInFirstBatch) {
24+
auto env = TEST_init(nullptr, nullptr, 0, /*enable_blink=*/1);
25+
auto* context = env->page()->executingContext();
26+
27+
// Flush bootstrap microtasks and drop any initial commands.
28+
TEST_runLoop(context);
29+
context->uiCommandBuffer()->clear();
30+
31+
const char* setup = R"JS(
32+
const style = document.createElement('style');
33+
style.textContent = '.box { color: red; }';
34+
document.body.appendChild(style);
35+
36+
const div = document.createElement('div');
37+
div.className = 'box';
38+
div.textContent = 'hi';
39+
document.body.appendChild(div);
40+
)JS";
41+
env->page()->evaluateScript(setup, strlen(setup), "vm://", 0);
42+
43+
context->uiCommandBuffer()->SyncAllPackages();
44+
auto* pack = static_cast<UICommandBufferPack*>(context->uiCommandBuffer()->data());
45+
auto* items = static_cast<UICommandItem*>(pack->data);
46+
47+
EXPECT_TRUE(HasCommand(items, pack->length, UICommand::kClearStyle));
48+
EXPECT_TRUE(HasCommand(items, pack->length, UICommand::kSetStyleById));
49+
}
50+

bridge/foundation/shared_ui_command.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ void SharedUICommand::AddCommand(UICommand type,
3636
void* native_binding_object,
3737
void* nativePtr2,
3838
bool request_ui_update) {
39+
if (type == UICommand::kFinishRecordingCommand) {
40+
context_->MaybeUpdateStyleForFirstPaint();
41+
}
42+
3943
// For non-dedicated contexts, add directly to read buffer
4044
if (!context_->isDedicated()) {
4145
std::lock_guard<std::mutex> lock(read_buffer_mutex_);
@@ -52,7 +56,12 @@ void SharedUICommand::AddCommand(UICommand type,
5256
if (type == UICommand::kFinishRecordingCommand) {
5357
// Calculate if we should request batch update based on waiting commands and ring buffer state
5458
bool should_request_batch_update =
55-
ui_command_sync_strategy_->GetWaitingCommandsCount() > 0 || package_buffer_->HasUnflushedCommands();
59+
ui_command_sync_strategy_->GetWaitingCommandsCount() > 0 || package_buffer_->HasUnflushedCommands() ||
60+
package_buffer_->HasDeferredPackages() || package_buffer_->PackageCount() > 0;
61+
62+
if (!context_->needs_first_paint_style_sync_) {
63+
package_buffer_->FlushDeferredPackages();
64+
}
5665

5766
// Flush all waiting ui commands to ring buffer
5867
ui_command_sync_strategy_->FlushWaitingCommands();
@@ -143,6 +152,9 @@ void SharedUICommand::SyncAllPackages() {
143152
// First flush waiting commands from UICommandStrategy to ring buffer
144153
ui_command_sync_strategy_->FlushWaitingCommands();
145154
package_buffer_->FlushCurrentPackage();
155+
if (!context_->needs_first_paint_style_sync_) {
156+
package_buffer_->FlushDeferredPackages();
157+
}
146158
}
147159

148160
void SharedUICommand::FillReadBuffer() {

bridge/foundation/ui_command_ring_buffer.cc

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,32 @@ void UICommandPackageRingBuffer::FlushCurrentPackage() {
297297
PushPackage(std::move(package));
298298
}
299299

300+
void UICommandPackageRingBuffer::FlushDeferredPackages() {
301+
if (context_ && context_->needs_first_paint_style_sync_) {
302+
return;
303+
}
304+
305+
std::vector<std::unique_ptr<UICommandPackage>> deferred;
306+
{
307+
std::lock_guard<std::mutex> lock(deferred_mutex_);
308+
if (deferred_packages_.empty()) {
309+
return;
310+
}
311+
deferred.swap(deferred_packages_);
312+
}
313+
314+
for (auto& package : deferred) {
315+
PushPackage(std::move(package));
316+
}
317+
}
318+
300319
void UICommandPackageRingBuffer::PushPackage(std::unique_ptr<UICommandPackage> package) {
320+
if (context_ && context_->needs_first_paint_style_sync_) {
321+
std::lock_guard<std::mutex> lock(deferred_mutex_);
322+
deferred_packages_.push_back(std::move(package));
323+
return;
324+
}
325+
301326
size_t write_idx = write_index_.load(std::memory_order_relaxed);
302327
size_t next_write_idx = (write_idx + 1) & capacity_mask_;
303328

@@ -357,6 +382,13 @@ size_t UICommandPackageRingBuffer::PackageCount() const {
357382
}
358383

359384
bool UICommandPackageRingBuffer::Empty() const {
385+
{
386+
std::lock_guard<std::mutex> lock(const_cast<std::mutex&>(deferred_mutex_));
387+
if (!deferred_packages_.empty()) {
388+
return false;
389+
}
390+
}
391+
360392
// Check if there are any packages in the ring buffer or overflow
361393
if (PackageCount() > 0) {
362394
return false;
@@ -372,6 +404,11 @@ bool UICommandPackageRingBuffer::HasUnflushedCommands() const {
372404
return !current_package_->commands.empty();
373405
}
374406

407+
bool UICommandPackageRingBuffer::HasDeferredPackages() const {
408+
std::lock_guard<std::mutex> lock(const_cast<std::mutex&>(deferred_mutex_));
409+
return !deferred_packages_.empty();
410+
}
411+
375412
void UICommandPackageRingBuffer::Clear() {
376413
write_index_.store(0, std::memory_order_relaxed);
377414
read_index_.store(0, std::memory_order_relaxed);
@@ -381,11 +418,20 @@ void UICommandPackageRingBuffer::Clear() {
381418
packages_[i].package.reset();
382419
}
383420

384-
std::lock_guard<std::mutex> lock(overflow_mutex_);
385-
overflow_packages_.clear();
421+
{
422+
std::lock_guard<std::mutex> pkg_lock(current_package_mutex_);
423+
current_package_->Clear();
424+
}
386425

387-
std::lock_guard<std::mutex> pkg_lock(current_package_mutex_);
388-
current_package_->Clear();
426+
{
427+
std::lock_guard<std::mutex> deferred_lock(deferred_mutex_);
428+
deferred_packages_.clear();
429+
}
430+
431+
{
432+
std::lock_guard<std::mutex> overflow_lock(overflow_mutex_);
433+
overflow_packages_.clear();
434+
}
389435
}
390436

391437
bool UICommandPackageRingBuffer::ShouldCreateNewPackage(UICommand command) const {

bridge/foundation/ui_command_ring_buffer.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ class UICommandPackageRingBuffer {
108108
bool request_ui_update = true);
109109
void AddCommandItem(const UICommandItem& item, UICommand type, bool request_ui_update = true);
110110
void FlushCurrentPackage();
111+
// Push any deferred packages to the ring buffer (no-op while a first-paint
112+
// barrier is active).
113+
void FlushDeferredPackages();
114+
bool HasDeferredPackages() const;
111115

112116
// Consumer operations
113117
std::unique_ptr<UICommandPackage> PopPackage();
@@ -139,6 +143,10 @@ class UICommandPackageRingBuffer {
139143
std::atomic<size_t> write_index_{0};
140144
std::atomic<size_t> read_index_{0};
141145

146+
// Deferred packages (first-paint barrier) held on the producer thread.
147+
std::mutex deferred_mutex_;
148+
std::vector<std::unique_ptr<UICommandPackage>> deferred_packages_;
149+
142150
// Overflow handling
143151
std::mutex overflow_mutex_;
144152
std::vector<std::unique_ptr<UICommandPackage>> overflow_packages_;

bridge/test/test.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ list(APPEND WEBF_TEST_SOURCE
3636
./core/html/html_link_element_rel_list_test.cc
3737
./core/timing/performance_test.cc
3838
./foundation/shared_ui_command_test.cc
39+
./foundation/blink_first_paint_style_sync_test.cc
3940
./foundation/ui_command_ring_buffer_test.cc
4041
./foundation/ui_command_strategy_test.cc
4142
./foundation/string/string_impl_unittest.cc

integration_tests/specs/blob/constructor.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ describe('Blob API', () => {
4545
div.style.border = '1px solid #000';
4646
document.body.appendChild(div);
4747

48+
await waitForFrame();
49+
4850
const blob = await div.toBlob(1);
4951
// @ts-ignore
5052
const base64 = await blob.base64();

0 commit comments

Comments
 (0)