New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 854798 link

Starred by 1 user
Status: Fixed
Owner:
kylec...@chromium.org
Closed: Jun 2018
Cc:
samans@chromium.org
█ fsam...@chromium.org
jonr...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 794961



Sign in to add a comment

Viz: TabCaptureApiPixelTest Timing out in extensions::ResultCatcher::GetNextResult()

Project Member Reported by jonr...@chromium.org, Jun 20 2018 
OS = Linux
Test Suite = browser_testes
Tests = TabCaptureApiPixelTest.OffscreenTabEndToEnd
        TabCaptureApiPixelTest.OffscreenTabEvilTests

These tests seem to timeout while running with --enable-features=VizDisplayCompositor while calling extensions::ResultCatcher::GetNextResult()

Example failure:
BrowserTestBase received signal: Terminated. Backtrace:
#0 0x7fa9cbeeb0bc base::debug::StackTrace::StackTrace()
#1 0x000003244325 content::(anonymous namespace)::DumpStackTraceSignalHandler()
#2 0x7fa9be337030 <unknown>
#3 0x7fa9be3e363d __poll
#4 0x7fa9bff730b9 <unknown>
#5 0x7fa9bff731cc g_main_context_iteration
#6 0x7fa9cbe47c12 base::MessagePumpGlib::Run()
#7 0x7fa9cbe44581 base::MessageLoop::Run()
#8 0x7fa9cbe78266 Still waiting for the following processes to finish:
	./out/lviz/browser_tests --enable-features=VizDisplayCompositor --gtest_also_run_disabled_tests --gtest_filter=TabCaptureApiPixelTest.OffscreenTabEndToEnd --single_process --test-launcher-output=/tmp/.org.chromium.Chromium.ZuchIl/resultswote6o/test_results.xml --user-data-dir=/tmp/.org.chromium.Chromium.ZuchIl/devYepn
base::RunLoop::Run()
#9 0x000003270c3f content::RunThisRunLoop()
#10 0x000004787664 extensions::ResultCatcher::GetNextResult()
#11 0x000002055e5f extensions::ExtensionApiTest::RunExtensionTestImpl()
#12 0x0000020561a3 extensions::ExtensionApiTest::RunExtensionSubtestWithArgAndFlags()
#13 0x000001f3c662 extensions::(anonymous namespace)::TabCaptureApiPixelTest_OffscreenTabEndToEnd_Test::RunTestOnMainThread()
#14 0x00000324385e content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#15 0x000002d07560 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#16 0x000002d0629a ChromeBrowserMainParts::PreMainMessageLoopRun()
#17 0x7fa9c7b5ef41 content::BrowserMainLoop::PreMainMessageLoopRun()
#18 0x7fa9c8056485 content::StartupTaskRunner::RunAllTasksNow()
#19 0x7fa9c7b5d63c content::BrowserMainLoop::CreateStartupTasks()
#20 0x7fa9c7b61ea5 content::BrowserMainRunnerImpl::Initialize()
#21 0x7fa9c7b5afe5 content::BrowserMain()
#22 0x7fa9c862f1e5 content::RunBrowserProcessMain()
#23 0x7fa9c86301c1 content::ContentMainRunnerImpl::Run()
#24 0x7fa9c5be16bf service_manager::Main()
#25 0x7fa9c862e154 content::ContentMain()
#26 0x0000032433c2 content::BrowserTestBase::SetUp()
#27 0x000002c9d272 InProcessBrowserTest::SetUp()
#28 0x00000269455d testing::Test::Run()
#29 0x0000026951c0 testing::TestInfo::Run()
#30 0x0000026956d7 testing::TestCase::Run()
#31 0x0000026a10c7 testing::internal::UnitTestImpl::RunAllTests()
#32 0x0000026a0c3d testing::UnitTest::Run()
#33 0x000002cb0a61 base::TestSuite::Run()
#34 0x000002c6f552 ChromeTestSuiteRunner::RunTestSuite()
#35 0x00000326cf90 content::LaunchTests()
#36 0x000002c6f9f3 LaunchChromeTests()
#37 0x000002c6f4de main
#38 0x7fa9be3242b1 __libc_start_main
#39 0x00000126802a _start
[1/1] TabCaptureApiPixelTest.OffscreenTabEndToEnd (TIMED OUT)

Comment 1 by jonr...@chromium.org, Jun 21 2018 

Reproduced today ahead of triage

Comment 2 by jonr...@chromium.org, Jun 22 2018

Labels: -Pri-3 Pri-1
Owner: kylec...@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by kylec...@chromium.org, Jun 22 2018

Blockedon: 843290
Cc: -m...@chromium.org fsam...@chromium.org
There is some kind of race/bug where we set the fallback SurfaceId for a SurfaceLayer in the browser but the browser doesn't submit another CompositorFrame after that. The renderer Surface never gets embedded and the test times out waiting for it to show up screen.

The SurfaceLayer gets as far as a commit with the fallback SurfaceId. LayerTreeHostImpl::DrawLayers() runs and early outs because there is no damage. The problem is likely in DamageTracker::UpdateDamageTracking() or something related to that.

Comment 4 by kylec...@chromium.org, Jun 25 2018 

Here are events that contribute to there being no damage for the SurfaceLayer.

1. Fallback SurfaceId is set on the SurfaceLayer.
2. Commit happens for the SurfaceLayer.
3. |skip_draw_properties_computation| is false at [1].
4. It's false because effect_node->is_drawn is false at [2] and LayerShouldBeSkippedInternal() returns true.
5. DamageTracker::UpdateDamageTracking() skips the SurfaceLayerImpl and there is no damage from anything else. No CompositorFrame is submitted as a result.

[1] https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host_common.cc?l=376&rcl=e26e064dd3ad6ca86361fbbcd262339da58c1345
[2] https://cs.chromium.org/chromium/src/cc/trees/draw_property_utils.cc?l=464&rcl=e26e064dd3ad6ca86361fbbcd262339da58c1345
[3] https://cs.chromium.org/chromium/src/cc/trees/damage_tracker.cc?l=116&rcl=e26e064dd3ad6ca86361fbbcd262339da58c1345

Comment 5 by kylec...@chromium.org, Jun 25 2018 

I semi figured out what is going on, the test uses the tabCapture.captureOffscreenTab() API and creates an offscreen tab that's casted into a visible tab. The offscreen tab itself is hidden. I think that explains why the offscreen tab SurfaceLayer has is_drawn=false in the corresponding PropertyTree EffectNode.

The problem that needs to be fixed here is that we want to submit a CompositorFrame if CompositorFrameMetadata::referenced_surfaces has changed. This is similar to how we want to submit a CompositorFrame if the LocalSurfaceId has changed. 

Changing LayerTreeHostImpl::HasDamage() to check this should be straightforward.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 25 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1be23e1d749f5c86682169d36ac631697bb044de

commit 1be23e1d749f5c86682169d36ac631697bb044de
Author: kylechar <kylechar@chromium.org>
Date: Mon Jun 25 21:04:34 2018

Always update surface references.

This CL ensures that if the set of referenced surfaces from the active
tree changes then a new CompositorFrame will be submitted. Before this
CL LayerTreeHostImpl didn't look at referenced surfaces in HasDamage()
so if there is no other damage then it will skip submitting a
CompositorFrame.

For this to happen the SurfaceLayer can't be visible, as updating the
fallback SurfaceId will otherwise contribute to the damage rect.

Bug:  854798 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id3f196bc151d6118e31c51f2bedc62fc773ee65f
Reviewed-on: https://chromium-review.googlesource.com/1114066
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570177}
[modify] https://crrev.com/1be23e1d749f5c86682169d36ac631697bb044de/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/1be23e1d749f5c86682169d36ac631697bb044de/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/1be23e1d749f5c86682169d36ac631697bb044de/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/1be23e1d749f5c86682169d36ac631697bb044de/testing/buildbot/filters/viz.browser_tests.filter

Comment 7 by kylec...@chromium.org, Jun 26 2018

Status: Fixed (was: Assigned)

Comment 8 by fsam...@chromium.org, Jun 26 2018

Blockedon: -843290
►

Sign in to add a comment