New issue
Advanced search Search tips

Issue 804707 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Regression



Sign in to add a comment

viz_content_browsertests failing on chromium.win/Win7 Tests (1)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Jan 23 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of tasak@google.com

viz_content_browsertests failing on chromium.win/Win7 Tests (1)
- ReloadCacheControlBrowserTest.NavigateToSame
- ReloadCacheControlBrowserTest.BypassingReload
- ...

Builders failed on: 
- Win7 Tests (1): 
  https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29

FYI:
[5804:5688:0122/215006.763:FATAL:host_frame_sink_manager.cc(107)] Check failed: !data.HasCompositorFrameSinkData().
Backtrace:
	base::debug::StackTrace::StackTrace [0x036B88A8+104]
	base::debug::StackTrace::StackTrace [0x036B7463+35]
	logging::LogMessage::~LogMessage [0x0372FFD7+151]
	viz::HostFrameSinkManager::CreateRootCompositorFrameSink [0x0D711FCB+603]
	content::VizProcessTransportFactory::OnEstablishedGpuChannel [0x143EAD3B+1931]


 

Comment 1 by tasak@google.com, Jan 23 2018

Labels: OS-Windows Type-Bug-Regression
Owner: kylec...@chromium.org
Suspect:
https://chromium-review.googlesource.com/874813

Because of "fix: use-after-free", I did't revert the CL.

Comment 2 by tasak@google.com, Jan 23 2018

Not sure. https://chromium-review.googlesource.com/879261 ?
Because the CL touched host_frame_sink_manager.
 
Labels: -Pri-2 Pri-1
Status: Assigned (was: Available)
sheriff: raising priority and marking assigned - this test is failing almost every build on win7 tests dbg.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 23 2018

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

commit 8ea6162ada4d5ba0d4dda7a91b1681d2a7620f09
Author: kylechar <kylechar@chromium.org>
Date: Tue Jan 23 14:46:48 2018

Revert "viz: Fix FrameSinkManagerImpl use-after-free."

This reverts commit a3d549d3bc5847ef57a3f5ada156f61f3b389c27.

Reason for revert:  https://crbug.com/804707  

Original change's description:
> viz: Fix FrameSinkManagerImpl use-after-free.
> 
> If a [Root]CompositorFrameSinkImpl in FrameSinkManagerImpls
> |compositor_frame_sinks_| wasn't deleted before FrameSinkManagerImpl
> is destroyed then we can end up running the CompositorFrameSinkSupport
> destructor twice. It looks like flat_map::clear() runs the
> CompositorFrameSinkSupport destructor before removing the items from the
> map, so when CompositorFrameSinkSupport calls back into
> FrameSinkManagerImpl::UnregisterCompositorFrameSinkSupport() the entry
> is still in the map and gets removed, running the
> CompositorFrameSinkSupport destructor a second time.
> 
> Add a DCHECK that all CompositorFrameSinkSupports are unregistered (and
> thus destroyed) before FrameSinkManagerImpl. Also refactor the code to
> avoid use-after-free without DCHECK.
> 
> Bug: 803405
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
> Change-Id: Ib5500d70284d61a8007497543ad5e745eb642d3f
> Reviewed-on: https://chromium-review.googlesource.com/874813
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#530923}

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

Change-Id: I2d80d21488007a03fb10a682a3a0c3dddc57a9c0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 803405,  804707 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/881361
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531231}
[modify] https://crrev.com/8ea6162ada4d5ba0d4dda7a91b1681d2a7620f09/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/8ea6162ada4d5ba0d4dda7a91b1681d2a7620f09/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/8ea6162ada4d5ba0d4dda7a91b1681d2a7620f09/components/viz/service/frame_sinks/frame_sink_manager_unittest.cc

I'm going to try reverting the CL. The original problem wasn't causing test failures.
Status: Fixed (was: Assigned)
Tests are passing again.

Sign in to add a comment