diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index c637295b0288b..6d059da2be08c 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/lib/ui/platform_dispatcher.dart @@ -308,8 +308,9 @@ class PlatformDispatcher { _invoke(onMetricsChanged, _onMetricsChangedZone); } - // The [FlutterView]s for which [FlutterView.render] has already been called - // during the current [onBeginFrame]/[onDrawFrame] callback sequence. + // A debug-only variable that stores the [FlutterView]s for which + // [FlutterView.render] has already been called during the current + // [onBeginFrame]/[onDrawFrame] callback sequence. // // It is null outside the scope of those callbacks indicating that calls to // [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView] @@ -319,9 +320,16 @@ class PlatformDispatcher { // Between [onBeginFrame] and [onDrawFrame] the properties value is // temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives // the gap between the two callbacks. - Set? _renderedViews; - // The `_renderedViews` value between `_beginFrame` and `_drawFrame`. - Set? _renderedViewsBetweenCallbacks; + // + // In release build, this variable is null, and therefore the calling rule is + // not enforced. This is because the check might hurt cold startup delay; + // see https://github.com/flutter/engine/pull/46919. + Set? _debugRenderedViews; + // A debug-only variable that temporarily stores the `_renderedViews` value + // between `_beginFrame` and `_drawFrame`. + // + // In release build, this variable is null. + Set? _debugRenderedViewsBetweenCallbacks; /// A callback invoked when any view begins a frame. /// @@ -343,9 +351,12 @@ class PlatformDispatcher { // Called from the engine, via hooks.dart void _beginFrame(int microseconds) { - assert(_renderedViews == null); - assert(_renderedViewsBetweenCallbacks == null); - _renderedViews = {}; + assert(_debugRenderedViews == null); + assert(_debugRenderedViewsBetweenCallbacks == null); + assert(() { + _debugRenderedViews = {}; + return true; + }()); _invoke1( onBeginFrame, @@ -353,10 +364,13 @@ class PlatformDispatcher { Duration(microseconds: microseconds), ); - assert(_renderedViews != null); - assert(_renderedViewsBetweenCallbacks == null); - _renderedViewsBetweenCallbacks = _renderedViews; - _renderedViews = null; + assert(_debugRenderedViews != null); + assert(_debugRenderedViewsBetweenCallbacks == null); + assert(() { + _debugRenderedViewsBetweenCallbacks = _debugRenderedViews; + _debugRenderedViews = null; + return true; + }()); } /// A callback that is invoked for each frame after [onBeginFrame] has @@ -374,16 +388,22 @@ class PlatformDispatcher { // Called from the engine, via hooks.dart void _drawFrame() { - assert(_renderedViews == null); - assert(_renderedViewsBetweenCallbacks != null); - _renderedViews = _renderedViewsBetweenCallbacks; - _renderedViewsBetweenCallbacks = null; + assert(_debugRenderedViews == null); + assert(_debugRenderedViewsBetweenCallbacks != null); + assert(() { + _debugRenderedViews = _debugRenderedViewsBetweenCallbacks; + _debugRenderedViewsBetweenCallbacks = null; + return true; + }()); _invoke(onDrawFrame, _onDrawFrameZone); - assert(_renderedViews != null); - assert(_renderedViewsBetweenCallbacks == null); - _renderedViews = null; + assert(_debugRenderedViews != null); + assert(_debugRenderedViewsBetweenCallbacks == null); + assert(() { + _debugRenderedViews = null; + return true; + }()); } /// A callback that is invoked when pointer data is available. diff --git a/lib/ui/window.dart b/lib/ui/window.dart index c70a855da0f34..9475dc10ab2c9 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -373,8 +373,14 @@ class FlutterView { /// painting. void render(Scene scene, {Size? size}) { // Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated - // by _renderedViews being null) are ignored. See _renderedViews. - final bool validRender = platformDispatcher._renderedViews?.add(this) ?? false; + // by _debugRenderedViews being null) are ignored. See _debugRenderedViews. + // TODO(dkwingsmt): We should change this skip into an assertion. + // https://github.com/flutter/flutter/issues/137073 + bool validRender = true; + assert(() { + validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false; + return true; + }()); if (validRender) { _render(scene as _NativeScene, size?.width ?? physicalSize.width, size?.height ?? physicalSize.height); } diff --git a/shell/common/animator.cc b/shell/common/animator.cc index a16374651cc2b..3dd925cee6213 100644 --- a/shell/common/animator.cc +++ b/shell/common/animator.cc @@ -143,13 +143,16 @@ void Animator::BeginFrame( void Animator::Render(std::unique_ptr layer_tree, float device_pixel_ratio) { - // Animator::Render should be called after BeginFrame, which is indicated by - // frame_timings_recorder_ being non-null. Otherwise, this call is ignored. - if (frame_timings_recorder_ == nullptr) { - return; - } has_rendered_ = true; + if (!frame_timings_recorder_) { + // Framework can directly call render with a built scene. + frame_timings_recorder_ = std::make_unique(); + const fml::TimePoint placeholder_time = fml::TimePoint::Now(); + frame_timings_recorder_->RecordVsync(placeholder_time, placeholder_time); + frame_timings_recorder_->RecordBuildStart(placeholder_time); + } + TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder_, "flutter", "Animator::Render", /*flow_id_count=*/0, /*flow_ids=*/nullptr); diff --git a/shell/common/animator.h b/shell/common/animator.h index dae86d0c9dd35..be15a76b765b1 100644 --- a/shell/common/animator.h +++ b/shell/common/animator.h @@ -58,8 +58,7 @@ class Animator final { /// /// This method must be called during a vsync callback, or /// technically, between Animator::BeginFrame and Animator::EndFrame - /// (both private methods). Otherwise, an assertion will be - /// triggered. + /// (both private methods). Otherwise, this call will be ignored. /// void Render(std::unique_ptr layer_tree, float device_pixel_ratio); diff --git a/shell/common/animator_unittests.cc b/shell/common/animator_unittests.cc index 37cda29bb648f..19a851a864c74 100644 --- a/shell/common/animator_unittests.cc +++ b/shell/common/animator_unittests.cc @@ -158,30 +158,20 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyIdleBeforeRender) { latch.Wait(); ASSERT_FALSE(delegate.notify_idle_called_); - fml::AutoResetWaitableEvent render_latch; // Validate it has not notified idle and try to render. task_runners.GetUITaskRunner()->PostDelayedTask( [&] { ASSERT_FALSE(delegate.notify_idle_called_); - EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { - auto layer_tree = std::make_unique( - LayerTree::Config(), SkISize::Make(600, 800)); - animator->Render(std::move(layer_tree), 1.0); - render_latch.Signal(); - }); - // Request a frame that builds a layer tree and renders a frame. - // When the frame is rendered, render_latch will be signaled. - animator->RequestFrame(true); + auto layer_tree = std::make_unique(LayerTree::Config(), + SkISize::Make(600, 800)); + animator->Render(std::move(layer_tree), 1.0); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }, // See kNotifyIdleTaskWaitTime in animator.cc. fml::TimeDelta::FromMilliseconds(60)); latch.Wait(); - render_latch.Wait(); - // A frame has been rendered, and the next frame request will notify idle. - // But at the moment there isn't another frame request, therefore it still - // hasn't notified idle. + // Still hasn't notified idle because there has been no frame request. task_runners.GetUITaskRunner()->PostTask([&] { ASSERT_FALSE(delegate.notify_idle_called_); // False to avoid getting cals to BeginFrame that will request more frames @@ -249,16 +239,16 @@ TEST_F(ShellTest, AnimatorDoesNotNotifyDelegateIfPipelineIsNotEmpty) { for (int i = 0; i < 2; i++) { task_runners.GetUITaskRunner()->PostTask([&] { - EXPECT_CALL(delegate, OnAnimatorBeginFrame).WillOnce([&] { - auto layer_tree = std::make_unique(LayerTree::Config(), - SkISize::Make(600, 800)); - animator->Render(std::move(layer_tree), 1.0); - begin_frame_latch.Signal(); - }); animator->RequestFrame(); task_runners.GetPlatformTaskRunner()->PostTask(flush_vsync_task); }); begin_frame_latch.Wait(); + + PostTaskSync(task_runners.GetUITaskRunner(), [&] { + auto layer_tree = std::make_unique(LayerTree::Config(), + SkISize::Make(600, 800)); + animator->Render(std::move(layer_tree), 1.0); + }); } PostTaskSync(task_runners.GetUITaskRunner(), [&] { animator.reset(); });