ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad is failing in viz_browser_tests |
||||||||||||
Issue descriptionSince after around r559360:559337 Example failures: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20(dbg)(1)/72063 https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20(dbg)(1)/72064 ../../chrome/browser/thumbnails/thumbnail_browsertest.cc:336: Failure Actual function call count doesn't match EXPECT_CALL(*thumbnail_service(), SetPageThumbnail(Field(&ThumbnailingContext::url, red_url), ImageColorIs(SK_ColorRED)))... Expected: to be called once Actual: never called - unsatisfied and active Stack trace: #0 0x0000040f4880 StackTraceGetter::CurrentStackTrace() #1 0x000004115e37 testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop() #2 0x00000411529d testing::internal::AssertHelper::operator=() #3 0x0000040e3f88 testing::internal::GoogleTestFailureReporter::ReportFailure() #4 0x000001ee5178 testing::internal::Expect() #5 0x0000040ed83d testing::internal::UntypedFunctionMockerBase::VerifyAndClearExpectationsLocked() #6 0x0000029ead41 testing::internal::FunctionMockerBase<>::~FunctionMockerBase() #7 0x0000029ea735 testing::internal::FunctionMocker<>::~FunctionMocker() #8 0x0000029e6015 thumbnails::(anonymous namespace)::MockThumbnailService::~MockThumbnailService() #9 0x0000029e5d81 testing::NiceMock<>::~NiceMock() #10 0x0000029e5da9 testing::NiceMock<>::~NiceMock() #11 0x7f671cd54a2f impl::RefcountedKeyedServiceTraits::Destruct() #12 0x000001f9238c base::RefCountedThreadSafe<>::Release() #13 0x00000203bcc9 scoped_refptr<>::Release() #14 0x000002031f6a scoped_refptr<>::~scoped_refptr() #15 0x0000067f00d1 ThumbnailSource::~ThumbnailSource() #16 0x0000067f0109 ThumbnailSource::~ThumbnailSource() #17 0x7f6724188680 content::URLDataSourceImpl::~URLDataSourceImpl() #18 0x7f67241886c9 content::URLDataSourceImpl::~URLDataSourceImpl() #19 0x7f6724179e20 content::URLDataManager::DeleteDataSources() #20 0x7f672334b77b content::BrowserMainLoop::ShutdownThreadsAndCleanUp() #21 0x7f6723351b6b content::BrowserMainRunnerImpl::Shutdown() #22 0x7f672333db2f content::BrowserMain() #23 0x7f67250a72c7 content::RunNamedProcessTypeMain() #24 0x7f67250aae74 content::ContentMainRunnerImpl::Run() #25 0x7f672509dfb5 content::ContentServiceManagerMainDelegate::RunEmbedderProcess() #26 0x7f671dd99dbc service_manager::Main() #27 0x7f67250a41c5 content::ContentMain() #28 0x000006fa190c content::BrowserTestBase::SetUp() #29 0x000005f43481 InProcessBrowserTest::SetUp() #30 0x0000029e368b thumbnails::(anonymous namespace)::ThumbnailTest::SetUp()
,
May 17 2018
That test contains a timeout, maybe in this particular situation (viz+dbg) things are simply too slow? I guess let's just increase the timeout a bit and see if that helps. I don't actually know what viz is, though, or how viz_browser_tests are different from regular browser_tests.
,
May 17 2018
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecf635df450228efeaf3b407c5995e73c9b65b5f commit ecf635df450228efeaf3b407c5995e73c9b65b5f Author: Kinuko Yasuda <kinuko@chromium.org> Date: Thu May 17 09:01:06 2018 Viz suppression for ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad TBR=treib@chromium.org Bug: 843921 Change-Id: I49f76869bbc7cd0e8abd5ac889ddec5e0ac1282d No-Tree-Checks: True No-Try: True Reviewed-on: https://chromium-review.googlesource.com/1063526 Commit-Queue: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#559462} [modify] https://crrev.com/ecf635df450228efeaf3b407c5995e73c9b65b5f/testing/buildbot/filters/viz.browser_tests.filter
,
May 22 2018
,
May 23 2018
+samans@ for any comments on Thumbnail Service in Viz. Viz is a project which relocates the display compositor from the browser process to the gpu process. This will change the timings and can expose race-conditions. In general having sleeps in tests is dangerous. Since it looks like the test is wanting to wait on frames you could consider RenderFrameSubmissionObserver in content/public/test/browser_test_utils.h viz_browser_tests can be run by: ./out/Debug/browser_tests --enable-features=VizDisplayCompositor
,
May 23 2018
Agreed that sleeping in the test is a poor hack. RenderFrameSubmissionObserver looks promising, but I don't know enough about the whole thing to judge whether it'll actually work. For example, how can I make sure that I got the frame after the navigation, as opposed to some previous frame that happened to still be pending?
,
Jun 11 2018
When you call ui_test_utils::NavigateToURL the backing of WebContents is recreated. If you instantiate a RenderFrameSubmissionObserver after calling that, and call WaitForMetadataChange you will be notified of the next frame submitted. Or if one has already been submitted you'll be sent the metadata for it. Frames from the previous WebContents will have been submitted/canceled already. And you won't be attaching the observer to it anyways.
,
Jun 12 2018
Thanks for the explanation! This should be a fairly straightforward fix then. Setting to Available.
,
Jun 12 2018
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e20ce45ac3506780001cffcb899328521aa72a60 commit e20ce45ac3506780001cffcb899328521aa72a60 Author: jonross <jonross@chromium.org> Date: Wed Jun 13 12:31:40 2018 Update ThumbnailTest Frame Waiting ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad was using a time based wait, to simulate awaiting a frame submission. However the timing on viz debug builds on Linux64 was leading to incorrect results. This updates the test to use RenderFrameSubmissionObserver to wait directly upon frame generation. TEST=ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad Bug: 843921 Change-Id: I71cd8acdc528a3b220c8ccbbbece585ed5cf03bd Reviewed-on: https://chromium-review.googlesource.com/1097216 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Jonathan Ross <jonross@chromium.org> Cr-Commit-Position: refs/heads/master@{#566807} [modify] https://crrev.com/e20ce45ac3506780001cffcb899328521aa72a60/chrome/browser/thumbnails/thumbnail_browsertest.cc [modify] https://crrev.com/e20ce45ac3506780001cffcb899328521aa72a60/testing/buildbot/filters/viz.browser_tests.filter
,
Jun 13 2018
,
Jun 13 2018
So I'm re-opening this issue. The change I made in #11 seems to have revealed a similar race on Windows under the --enable-features=NetworkService I'll revert the change in #11 until I've had a chance to look further at this. But I'm not sure what differences the NetworkService entails. I had one repro locally, but trying to debug further has made it go away. I'm wondering if we are capturing a screenshot from a frame before the red is applied. Any thoughts?
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24f3b882d3a3adde15189d050deeccc36c476341 commit 24f3b882d3a3adde15189d050deeccc36c476341 Author: Jonathan Ross <jonross@chromium.org> Date: Wed Jun 13 21:20:08 2018 Revert "Update ThumbnailTest Frame Waiting" This reverts commit e20ce45ac3506780001cffcb899328521aa72a60. Reason for revert: This has exposed a race in NetworkService feature tests. https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy2AELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKhAWNocm9taXVtLndpbi9XaW4xMCBUZXN0cyB4NjQvMjQ2MDkvbmV0d29ya19zZXJ2aWNlX2Jyb3dzZXJfdGVzdHMgb24gV2luZG93cy0xMC0xNTA2My9WR2gxYldKdVlXbHNWR1Z6ZEM1VGFHOTFiR1JEWVhCMGRYSmxUMjVPWVhacFoyRjBhVzVuUVhkaGVWTnNiM2RRWVdkbFRHOWhaQT09DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw Original change's description: > Update ThumbnailTest Frame Waiting > > ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad was using a time based > wait, to simulate awaiting a frame submission. However the timing on viz debug > builds on Linux64 was leading to incorrect results. > > This updates the test to use RenderFrameSubmissionObserver to wait directly upon > frame generation. > > TEST=ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad > > Bug: 843921 > Change-Id: I71cd8acdc528a3b220c8ccbbbece585ed5cf03bd > Reviewed-on: https://chromium-review.googlesource.com/1097216 > Reviewed-by: Marc Treib <treib@chromium.org> > Commit-Queue: Jonathan Ross <jonross@chromium.org> > Cr-Commit-Position: refs/heads/master@{#566807} TBR=jonross@chromium.org,treib@chromium.org Change-Id: If60f494b1a21aaec2ca84572b5734544ec7d786f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 843921 Reviewed-on: https://chromium-review.googlesource.com/1099955 Reviewed-by: Jonathan Ross <jonross@chromium.org> Commit-Queue: Jonathan Ross <jonross@chromium.org> Cr-Commit-Position: refs/heads/master@{#566988} [modify] https://crrev.com/24f3b882d3a3adde15189d050deeccc36c476341/chrome/browser/thumbnails/thumbnail_browsertest.cc [modify] https://crrev.com/24f3b882d3a3adde15189d050deeccc36c476341/testing/buildbot/filters/viz.browser_tests.filter
,
Jun 14 2018
"I'm wondering if we are capturing a screenshot from a frame before the red is applied." That's exactly what the test is for: Make sure that *doesn't* happen :) The red background is specified in the <head> of the test html, which means the very first paint of this page must already be red. If it's not, then either a) there's a bug in the paint layer somewhere (which we did have before, see bug 21798 ), or b) we're taking a screenshot of the wrong page after all, maybe some race condition between navigation and rendering?
,
Jun 14 2018
Assigning to samans@ who have recently looked at screenshots/thumbnails. Since this is failing in both VizDisplayCompositor and NetworkService, along with the comments in #15, this sounds like a bug in the thumbnailing itself. samans@ could you help take a look or help triage this to the correct owner?
,
Jun 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b7532ee34edcea2849ce2580aff291e2628044b7 commit b7532ee34edcea2849ce2580aff291e2628044b7 Author: Saman Sami <samans@chromium.org> Date: Fri Jun 15 12:50:39 2018 Fix flakiness of ThumbnailTest.ShouldCaptureOnNavigatingAwaySlowPageLoad Use NavigationAndFirstPaintWaiter to ensure we wait for the first paint. Bug: 843921 Change-Id: Ia469d2460adf71f7151e2874bed034d6d3acbf10 Reviewed-on: https://chromium-review.googlesource.com/1101641 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Saman Sami <samans@chromium.org> Cr-Commit-Position: refs/heads/master@{#567616} [modify] https://crrev.com/b7532ee34edcea2849ce2580aff291e2628044b7/chrome/browser/thumbnails/thumbnail_browsertest.cc [modify] https://crrev.com/b7532ee34edcea2849ce2580aff291e2628044b7/testing/buildbot/filters/viz.browser_tests.filter
,
Jun 15 2018
Please reopen if not fixed. |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by kinuko@chromium.org
, May 17 2018