Commit 49454dc9 authored by Fady Samuel's avatar Fady Samuel Committed by Commit Bot

Reland "Surface synchronization: Add diagnostic CHECK"

This reverts commit fc9d1583.

Reason for revert: Now that we've branched for M68, I'm relanding this.

Original change's description:
> Revert "Surface synchronization: Add diagnostic CHECK"
> 
> This reverts commit 3fb6e543.
> 
> Reason for revert: This is blocking branch cut. We know we hit this case and we've collected some crash logs to sift through now.
> 
> Original change's description:
> > 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/989851
> > Reviewed-by: enne <enne@chromium.org>
> > Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#547457}
> 
> TBR=enne@chromium.org,fsamuel@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 672962
> Change-Id: Id1ad5997e12b7f966df5f7b034bd5a30baa5ae60
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
> Reviewed-on: https://chromium-review.googlesource.com/1008642
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Commit-Queue: Fady Samuel <fsamuel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#550175}

TBR=enne@chromium.org,fsamuel@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 672962
Change-Id: I7e460b4177d7690cc324d0feadeed7aacbcb24ec
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1012407Reviewed-by: default avatarFady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551126}
parent 83134b01
......@@ -1379,7 +1379,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_) {
......
......@@ -180,7 +180,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 {}
......@@ -242,7 +242,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(
......@@ -3532,7 +3532,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();
......@@ -9020,7 +9020,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) {
......
......@@ -299,9 +299,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