Commit dca5bff4 authored by Eric Karl's avatar Eric Karl Committed by Commit Bot

Effective revert of "Allow one undrawn frame in ShouldSendBeginFrame"

This effectively reverts commit
7e8de286.

Reason for revert: Testing whether this change was responsible for
the latency regression seen in crbug.com/959048.

This isn't a true revert, as that CL no longer reverts cleanly, but this
restores our previous behavior.

Bug: 935630, 959048
Change-Id: Iefb4bd614b5802cfa24a8cb459e62842cfa6d4e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1698629
Auto-Submit: Eric Karl <ericrk@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683450}
parent 9174baca
...@@ -3409,9 +3409,12 @@ TEST_F(DisplayTest, BeginFrameThrottling) { ...@@ -3409,9 +3409,12 @@ TEST_F(DisplayTest, BeginFrameThrottling) {
EXPECT_TRUE(ShouldSendBeginFrame(support_.get(), frame_time)); EXPECT_TRUE(ShouldSendBeginFrame(support_.get(), frame_time));
UpdateBeginFrameTime(support_.get(), frame_time); UpdateBeginFrameTime(support_.get(), frame_time);
submit_frame(); submit_frame();
// Immediately after submitting frame, because there is presentation // If we've submitted more than one frame without drawing, there will
// feedback queued up, ShouldSendBeginFrame should always return true. // always be presentation feedback so ShouldSendBeginFrame should always
EXPECT_TRUE(ShouldSendBeginFrame(support_.get(), frame_time)); // return true.
if (i > 0)
EXPECT_TRUE(ShouldSendBeginFrame(support_.get(), frame_time));
// Clear the presentation feedbacks. // Clear the presentation feedbacks.
UpdateBeginFrameTime(support_.get(), frame_time); UpdateBeginFrameTime(support_.get(), frame_time);
} }
...@@ -3564,9 +3567,11 @@ TEST_F(DisplayTest, DontThrottleWhenParentBlocked) { ...@@ -3564,9 +3567,11 @@ TEST_F(DisplayTest, DontThrottleWhenParentBlocked) {
UpdateBeginFrameTime(sub_support.get(), frame_time); UpdateBeginFrameTime(sub_support.get(), frame_time);
sub_support->SubmitCompositorFrame(sub_local_surface_id, sub_support->SubmitCompositorFrame(sub_local_surface_id,
MakeDefaultCompositorFrame()); MakeDefaultCompositorFrame());
// Immediately after submitting frame, because there is presentation // If we've submitted more than one frame without drawing, there will
// feedback queued up, ShouldSendBeginFrame should always return true. // always be presentation feedback so ShouldSendBeginFrame should always
EXPECT_TRUE(ShouldSendBeginFrame(sub_support.get(), frame_time)); // return true.
if (i > 0)
EXPECT_TRUE(ShouldSendBeginFrame(support_.get(), frame_time));
// Clear the presentation feedbacks. // Clear the presentation feedbacks.
UpdateBeginFrameTime(sub_support.get(), frame_time); UpdateBeginFrameTime(sub_support.get(), frame_time);
} }
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h"
#include "components/viz/common/frame_sinks/begin_frame_source.h" #include "components/viz/common/frame_sinks/begin_frame_source.h"
#include "components/viz/common/frame_timing_details_map.h" #include "components/viz/common/frame_timing_details_map.h"
#include "components/viz/common/quads/compositor_frame.h" #include "components/viz/common/quads/compositor_frame.h"
...@@ -67,7 +68,13 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport ...@@ -67,7 +68,13 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
// exceeded, we throttle sBeginFrames to 1 per second. Limit must be at least // exceeded, we throttle sBeginFrames to 1 per second. Limit must be at least
// 1, as the relative ordering of renderer / browser frame submissions allows // 1, as the relative ordering of renderer / browser frame submissions allows
// us to have one outstanding undrawn frame under normal operation. // us to have one outstanding undrawn frame under normal operation.
#if defined(OS_ANDROID)
// TODO(ericrk): Revert this. Setting to 0 temporarily to test the impact of
// this change on a latency regression. See https://crbug.com/959048
static constexpr uint32_t kUndrawnFrameLimit = 0;
#else
static constexpr uint32_t kUndrawnFrameLimit = 3; static constexpr uint32_t kUndrawnFrameLimit = 3;
#endif
CompositorFrameSinkSupport(mojom::CompositorFrameSinkClient* client, CompositorFrameSinkSupport(mojom::CompositorFrameSinkClient* client,
FrameSinkManagerImpl* frame_sink_manager, FrameSinkManagerImpl* frame_sink_manager,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment