Commit c618b6f3 authored by Saman Sami's avatar Saman Sami Committed by Commit Bot

Reland "viz: Fix FindLatestSurfaceUpTo when there is conflict"

This is a reland of ae629163

Original change's description:
> viz: Fix FindLatestSurfaceUpTo when there is conflict
>
> The condition used in the binary search was wrong. Fixed it and added
> a unit test.
>
> Bug: 1000868
> Change-Id: Iaf4af1bdfdb88db1082bfae89a3783faed860934
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1800743
> Auto-Submit: Saman Sami <samans@chromium.org>
> Commit-Queue: Jonathan Ross <jonross@chromium.org>
> Reviewed-by: Jonathan Ross <jonross@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#696170}

TBR=jonross@chromium.org,samans@chromium.org

Bug: 1000868, 1015120
Change-Id: I2b793035cd689551217e23de5e43d0fd855cbf37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2017945Reviewed-by: default avatarNick Burris <nburris@chromium.org>
Commit-Queue: Nick Burris <nburris@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734663}
parent 8fc3cd9c
...@@ -3067,6 +3067,22 @@ TEST_F(SurfaceSynchronizationTest, ...@@ -3067,6 +3067,22 @@ TEST_F(SurfaceSynchronizationTest,
EXPECT_EQ(parent_id2, parent_support().last_activated_surface_id()); EXPECT_EQ(parent_id2, parent_support().last_activated_surface_id());
} }
// Regression test for https://crbug.com/1000868. Verify that the output of
// GetLatestInFlightSurface is correct when there is a conflict between the last
// surface in the group and the queried SurfaceRange.
TEST_F(SurfaceSynchronizationTest, LatestInFlightSurfaceConflict) {
const SurfaceId id1 = MakeSurfaceId(kParentFrameSink, 1, 1);
const SurfaceId id2 = MakeSurfaceId(kParentFrameSink, 2, 2);
const SurfaceId id3 = MakeSurfaceId(kParentFrameSink, 1, 3);
parent_support().SubmitCompositorFrame(id1.local_surface_id(),
MakeDefaultCompositorFrame());
parent_support().SubmitCompositorFrame(id2.local_surface_id(),
MakeDefaultCompositorFrame());
EXPECT_EQ(GetSurfaceForId(id1),
GetLatestInFlightSurface(SurfaceRange(base::nullopt, id3)));
}
// Check that if two different SurfaceIds with the same embed token are // Check that if two different SurfaceIds with the same embed token are
// embedded, viz doesn't crash. https://crbug.com/1001143 // embedded, viz doesn't crash. https://crbug.com/1001143
TEST_F(SurfaceSynchronizationTest, TEST_F(SurfaceSynchronizationTest,
......
...@@ -188,7 +188,7 @@ SurfaceAllocationGroup::FindLatestSurfaceUpTo( ...@@ -188,7 +188,7 @@ SurfaceAllocationGroup::FindLatestSurfaceUpTo(
// If even the first surface is newer than |surface_id|, we can't find a // If even the first surface is newer than |surface_id|, we can't find a
// surface that is older than or equal to |surface_id|. // surface that is older than or equal to |surface_id|.
if (surfaces_[0]->surface_id().IsNewerThan(surface_id)) if (!surface_id.IsSameOrNewerThan(surfaces_[0]->surface_id()))
return surfaces_.end(); return surfaces_.end();
// Perform a binary search the find the latest surface that is older than or // Perform a binary search the find the latest surface that is older than or
...@@ -197,12 +197,13 @@ SurfaceAllocationGroup::FindLatestSurfaceUpTo( ...@@ -197,12 +197,13 @@ SurfaceAllocationGroup::FindLatestSurfaceUpTo(
int end = surfaces_.size(); int end = surfaces_.size();
while (end - begin > 1) { while (end - begin > 1) {
int avg = (begin + end) / 2; int avg = (begin + end) / 2;
if (surfaces_[avg]->surface_id().IsNewerThan(surface_id)) if (!surface_id.IsSameOrNewerThan(surfaces_[avg]->surface_id()))
end = avg; end = avg;
else else
begin = avg; begin = avg;
} }
DCHECK(surface_id.IsSameOrNewerThan(surfaces_[begin]->surface_id()));
return surfaces_.begin() + begin; return surfaces_.begin() + begin;
} }
......
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