Commit 3fb6e543 authored by Fady Samuel's avatar Fady Samuel Committed by Commit Bot

Surface synchronization: Add diagnostic CHECK

Current evidence points to a race in cc that is causing surface invariants
violations (No invariants violations detected in LayerTreeHost but detected
in ClientLayerTreeFrameSink).

My suspicion is viewport is updated out of sync with LocalSurfaceId and
device scale factor.

This CL adds a diagnostic CHECK between the pending and active LayerImpls to
attempt to detect the race and produce a better stack trace. The invariant
we want to test here is if active tree has an invalid viewport (viewport has
changed), then the LocalSurfaceId MUST change.

Bug: 672962
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I975c9dfb9c86c292d7aac052bac01510e6efb1c2
Reviewed-on: https://chromium-review.googlesource.com/989851Reviewed-by: default avatarenne <enne@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547457}
parent 54c9e082
......@@ -1380,7 +1380,7 @@ void LayerTreeHost::PushLayerTreePropertiesTo(LayerTreeImpl* tree_impl) {
tree_impl->set_content_source_id(content_source_id_);
tree_impl->set_local_surface_id(local_surface_id_);
tree_impl->SetLocalSurfaceId(local_surface_id_);
has_pushed_local_surface_id_ = true;
if (pending_page_scale_animation_) {
......
......@@ -179,7 +179,7 @@ class LayerTreeHostImplTest : public testing::Test,
}
void DidActivateSyncTree() override {
// Make sure the active tree always has a valid LocalSurfaceId.
host_impl_->active_tree()->set_local_surface_id(
host_impl_->active_tree()->SetLocalSurfaceId(
viz::LocalSurfaceId(1, base::UnguessableToken::Deserialize(2u, 3u)));
}
void WillPrepareTiles() override {}
......@@ -241,7 +241,7 @@ class LayerTreeHostImplTest : public testing::Test,
bool init = host_impl_->InitializeRenderer(layer_tree_frame_sink_.get());
host_impl_->SetViewportSize(gfx::Size(10, 10));
host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, 1.f, 1.f);
host_impl_->active_tree()->set_local_surface_id(
host_impl_->active_tree()->SetLocalSurfaceId(
viz::LocalSurfaceId(1, base::UnguessableToken::Deserialize(2u, 3u)));
// Set the viz::BeginFrameArgs so that methods which use it are able to.
host_impl_->WillBeginImplFrame(viz::CreateBeginFrameArgsForTesting(
......@@ -3503,7 +3503,7 @@ class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest {
host_impl_->active_tree()->BuildPropertyTreesForTesting();
host_impl_->active_tree()->DidBecomeActive();
host_impl_->active_tree()->HandleScrollbarShowRequestsFromMain();
host_impl_->active_tree()->set_local_surface_id(
host_impl_->active_tree()->SetLocalSurfaceId(
viz::LocalSurfaceId(1, base::UnguessableToken::Deserialize(2u, 3u)));
DrawFrame();
......@@ -8991,7 +8991,7 @@ TEST_F(LayerTreeHostImplTest, PartialSwapReceivesDamageRect) {
root->test_properties()->AddChild(std::move(child));
layer_tree_host_impl->active_tree()->SetRootLayerForTesting(std::move(root));
layer_tree_host_impl->active_tree()->BuildPropertyTreesForTesting();
layer_tree_host_impl->active_tree()->set_local_surface_id(
layer_tree_host_impl->active_tree()->SetLocalSurfaceId(
viz::LocalSurfaceId(1, base::UnguessableToken::Deserialize(2u, 3u)));
TestFrameData frame;
......
......@@ -469,7 +469,7 @@ void LayerTreeImpl::PushPropertiesTo(LayerTreeImpl* target_tree) {
target_tree->set_content_source_id(content_source_id());
target_tree->set_local_surface_id(local_surface_id());
target_tree->SetLocalSurfaceId(local_surface_id());
target_tree->pending_page_scale_animation_ =
std::move(pending_page_scale_animation_);
......@@ -963,6 +963,13 @@ void LayerTreeImpl::SetDeviceScaleFactor(float device_scale_factor) {
host_impl_->SetNeedUpdateGpuRasterizationStatus();
}
void LayerTreeImpl::SetLocalSurfaceId(
const viz::LocalSurfaceId& local_surface_id) {
CHECK(!settings().enable_surface_synchronization || !viewport_size_invalid_ ||
local_surface_id_ != local_surface_id);
local_surface_id_ = local_surface_id;
}
void LayerTreeImpl::SetRasterColorSpace(
int raster_color_space_id,
const gfx::ColorSpace& raster_color_space) {
......
......@@ -300,9 +300,7 @@ class CC_EXPORT LayerTreeImpl {
void set_content_source_id(uint32_t id) { content_source_id_ = id; }
uint32_t content_source_id() { return content_source_id_; }
void set_local_surface_id(const viz::LocalSurfaceId& id) {
local_surface_id_ = id;
}
void SetLocalSurfaceId(const viz::LocalSurfaceId& id);
const viz::LocalSurfaceId& local_surface_id() const {
return local_surface_id_;
}
......
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