Issue metadata
Sign in to add a comment
|
NavigationURLLoaderImplTest.Redirect303Tests flaked on ScopedTaskEnvironment::MainThreadHasPendingTask() check |
||||||||||||||||||||||
Issue descriptionOn the Fuchsia/x64 FYI bot content_unittests flaked with: [ RUN ] NavigationURLLoaderImplTest.Redirect303Tests [471189:1679449663:1130/092222.175656:210433019:FATAL:test_browser_thread_bundle.cc(75)] Check failed: !scoped_task_environment_->MainThreadHasPendingTask(). #00: base::debug::StackTrace::StackTrace(unsigned long) at stack_trace_fuchsia.cc:? #01: logging::LogMessage::~LogMessage() at logging.cc:? #02: content::TestBrowserThreadBundle::~TestBrowserThreadBundle() at test_browser_thread_bundle.cc:? #03: content::NavigationURLLoaderImplTest::~NavigationURLLoaderImplTest() at navigation_url_loader_impl_unittest.cc:? #04: content::NavigationURLLoaderImplTest_RequestPriority_Test::~NavigationURLLoaderImplTest_RequestPriority_Test() at navigation_url_loader_impl_unittest.cc:? #05: testing::TestInfo::Run() at gtest.cc:? #06: testing::TestCase::Run() at gtest.cc:? #07: testing::internal::UnitTestImpl::RunAllTests() at gtest.cc:? #08: testing::UnitTest::Run() at gtest.cc:? #09: base::TestSuite::Run() at test_suite.cc:? #10: long base::internal::Invoker<base::internal::BindState<long (content::RenderFrameHostDelegate::*)() const, base::internal::UnretainedWrapper<content::RenderFrameHostDelegate> >, long ()>::RunImpl<long (content::RenderFrameHostDelegate::* const&)() const, std::__1::tuple<base::internal::UnretainedWrapper<content::RenderFrameHostDelegate> > const&, 0ul>(long (content::RenderFrameHostDelegate::* const&)() const, std::__1::tuple<base::internal::UnretainedWrapper<content::RenderFrameHostDelegate> > const&, std::__1::integer_sequence<unsigned long, 0ul>) at render_frame_host_impl.cc:? #11: base::OnceCallback<bool ()>::Run() && at rtc_rtp_sender_unittest.cc:? #12: base::(anonymous namespace)::LaunchUnitTestsInternal(base::OnceCallback<int ()>, unsigned long, int, bool, base::OnceCallback<void ()>) at unit_test_launcher.cc:? #13: base::LaunchUnitTests(int, char**, base::OnceCallback<int ()>) at unit_test_launcher.cc:? #14: main at run_all_unittests.cc:? #15: pc 0x5ff55dd80c48 (libc.so,0x19c48) [00210.706] pkgsvr: pkgfs:unsupported(/packages/content_unittests/0): dir unlink "content_unittests__exec.log" Suspect that this may be related to issue 910029 , which is a race-condition between operations on the main thread, and the SimpleBackendImpl sequence. Assigning to alexclarke@, since this is likely caused by the reland of https://chromium-review.googlesource.com/c/chromium/src/+/1354919 yesterday.
,
Nov 30
CC'ing today's Gardener for Fuchsia bots.
,
Nov 30
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe commit 1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe Author: Wez <wez@chromium.org> Date: Fri Nov 30 19:59:38 2018 Revert "Feature to run Blink main thread using SequenceManager" This reverts commit 66fbaed95264740edc1ba7c51ab7b5401a31aeb2. Reason for revert: Revert required to allow https://chromium-review.googlesource.com/c/chromium/src/+/1354919 to be reverted cleanly for issue 910645 . Original change's description: > Feature to run Blink main thread using SequenceManager > > This will enable us to start trialing the MessageLoop replacement by > SequenceManager. > > We only do this in the Blink main thread for now. There is some work > needed around SequenceManager ownership to be done before this can be > enabled in worker threads. > > > Bug: 891670 > Change-Id: I1e59784ff35c87883a66d9b315435bf47cd0f809 > Reviewed-on: https://chromium-review.googlesource.com/c/1349316 > Reviewed-by: Alex Clarke <alexclarke@chromium.org> > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Reviewed-by: Alexander Timin <altimin@chromium.org> > Reviewed-by: Sami Kyöstilä <skyostil@chromium.org> > Commit-Queue: Carlos Caballero <carlscab@google.com> > Cr-Commit-Position: refs/heads/master@{#612185} TBR=dcheng@chromium.org,skyostil@chromium.org,alexclarke@chromium.org,altimin@chromium.org,carlscab@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 910645 , 891670 Change-Id: I3529309984cd61dfef8a10e49129d364efabcf1f Reviewed-on: https://chromium-review.googlesource.com/c/1357016 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#612737} [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/base/task/sequence_manager/sequence_manager.h [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/base/task/sequence_manager/sequence_manager_impl.cc [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/base/test/scoped_task_environment.cc [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/content/renderer/renderer_main.cc [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/third_party/blink/public/platform/scheduler/DEPS [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/third_party/blink/public/platform/scheduler/web_thread_scheduler.h [modify] https://crrev.com/1ef457d1b9ce1c2372cc8b37725e6e424c5c4efe/third_party/blink/renderer/platform/scheduler/common/web_thread_scheduler.cc
,
Nov 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b54920b5ce0b28a83cf343b94d78c672d12a2645 commit b54920b5ce0b28a83cf343b94d78c672d12a2645 Author: Wez <wez@chromium.org> Date: Fri Nov 30 20:04:13 2018 Revert "[Reland] Use the SequenceManager in ScopedTaskEnvironment" This reverts commit 49c8774b77a9c530447f805dbb5358092cfa28e1. Reason for revert: CL seems likely cause of a new MainThreadHasPendingTasks() flake observed on the Fuchsia builders (see crbug.com/910645 ). Original change's description: > [Reland] Use the SequenceManager in ScopedTaskEnvironment > > A reland of https://crrev.com/c/1324391 > > This is necessary because we want content::TestBrowserThreadBundle to > own a BrowserUIThreadScheduler, but that also owns a ScopedTaskEnvironment > and you can't have two SequenceManagers on the same thread. > > This patch allows ScopedTaskEnvironment to optionally work with an > externally owned SequenceManager solving the problem. > > This implements https://docs.google.com/document/d/1y08C6JQ9Yta3EQXzwIqqIIKHq9500WV6CWFZzZfDx7I/edit?usp=drivesdk, > > We now have the ability to mock time on the UI thread. > > TBR=asvitkine@chromium.org,miu@chromium.org,gab@chromium.org,fdoray@chromium.org > > Bug: 863341, 891670, 708584 > Change-Id: Ia4409b885deb9935e6e5b6d99f4598f164d350db > Reviewed-on: https://chromium-review.googlesource.com/c/1354919 > Reviewed-by: Alex Clarke <alexclarke@chromium.org> > Commit-Queue: Alex Clarke <alexclarke@chromium.org> > Cr-Commit-Position: refs/heads/master@{#612122} TBR=gab@chromium.org,miu@chromium.org,fdoray@chromium.org,asvitkine@chromium.org,alexclarke@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 910645 , 863341, 891670, 708584 Change-Id: I395322743c110f9b1dd424434756f05b2301a45b Reviewed-on: https://chromium-review.googlesource.com/c/1357014 Commit-Queue: Wez <wez@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#612741} [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/base/message_loop/message_pump_mac.mm [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/base/task/sequence_manager/thread_controller_with_message_pump_impl.cc [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/base/test/scoped_task_environment.cc [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/base/test/scoped_task_environment.h [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/base/test/scoped_task_environment_unittest.cc [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm [modify] https://crrev.com/b54920b5ce0b28a83cf343b94d78c672d12a2645/content/renderer/media/stream/webmediaplayer_ms_unittest.cc
,
Nov 30
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1dc884dc9ff9b74841192d420479197f80c3ac9c commit 1dc884dc9ff9b74841192d420479197f80c3ac9c Author: Alex Clarke <alexclarke@chromium.org> Date: Thu Dec 06 13:22:23 2018 SequenceManagerImpl::IsIdleForTesting to match MessageLoop behavior The MessageLoop version of IsIdleForTesting does not take Now() into account, but the SequenceManager version was. This is a mistake because its flaky when there are delayed tasks that are just about to run. TBR=fdoray@chromium.org Bug: 910645 , 863341, 891670 Change-Id: Ieaf5e534df0f553046b76fa844f61e4a268082f9 Reviewed-on: https://chromium-review.googlesource.com/c/1365233 Reviewed-by: Alex Clarke <alexclarke@chromium.org> Reviewed-by: Alexander Timin <altimin@chromium.org> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org> Commit-Queue: Alex Clarke <alexclarke@chromium.org> Cr-Commit-Position: refs/heads/master@{#614330} [modify] https://crrev.com/1dc884dc9ff9b74841192d420479197f80c3ac9c/base/message_loop/message_loop_unittest.cc [modify] https://crrev.com/1dc884dc9ff9b74841192d420479197f80c3ac9c/base/task/sequence_manager/sequence_manager.h [modify] https://crrev.com/1dc884dc9ff9b74841192d420479197f80c3ac9c/base/task/sequence_manager/sequence_manager_impl.cc |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by w...@chromium.org
, Nov 30