New issue
Advanced search Search tips

Issue 750530 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

app_shell_unittests: ShellNativeAppWindowAuraTest.Bounds flaky

Project Member Reported by michae...@chromium.org, Jul 30 2017

Issue description

Flakiness dashboard: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=app_shell_unittests&tests=ShellNativeAppWindowAuraTest.Bounds

The error is:
[132356:132356:0730/163547.507100:8623171005042:FATAL:dependency_manager.cc(108)] Check failed: false. Attempted to access a context that was ShutDown(). This is most likely a heap smasher in progress. After KeyedService::Shutdown() completes, your service MUST NOT refer to depended services again.


The crash is more reliable when the tests run in a certain order or in single-process mode, eg:

./testing/xvfb.py out_linux/rel/app_shell_unittests --single-process-tests --gtest_shuffle --gtest_random_seed=20

Additional test environment:
    CHROME_DEVEL_SANDBOX=/usr/local/sbin/chrome-devel-sandbox
    LANG=en_US.UTF-8
Command: out_linux/rel/app_shell_unittests --single-process-tests --gtest_shuffle --gtest_random_seed=20

Note: Randomizing tests' orders with a seed of 20 .
[==========] Running 10 tests from 7 test cases.
[----------] Global test environment set-up.
[----------] 1 test from ShellScreenTest
[ RUN      ] ShellScreenTest.ShellScreen
Xlib:  extension "RANDR" missing on display ":103".
Xlib:  extension "RANDR" missing on display ":103".
[       OK ] ShellScreenTest.ShellScreen (54 ms)
[----------] 1 test from ShellScreenTest (54 ms total)

[----------] 1 test from ShellContentClientTest
[ RUN      ] ShellContentClientTest.UserAgentFormat
[       OK ] ShellContentClientTest.UserAgentFormat (0 ms)
[----------] 1 test from ShellContentClientTest (0 ms total)

[----------] 1 test from ShellOAuth2TokenServiceTest
[ RUN      ] ShellOAuth2TokenServiceTest.SetRefreshToken
[       OK ] ShellOAuth2TokenServiceTest.SetRefreshToken (2 ms)
[----------] 1 test from ShellOAuth2TokenServiceTest (2 ms total)

[----------] 1 test from ShellDesktopControllerAuraTest
[ RUN      ] ShellDesktopControllerAuraTest.InputEvents
Xlib:  extension "RANDR" missing on display ":103".
Xlib:  extension "RANDR" missing on display ":103".
[       OK ] ShellDesktopControllerAuraTest.InputEvents (59 ms)
[----------] 1 test from ShellDesktopControllerAuraTest (59 ms total)

[----------] 3 tests from IdentityApiTest
[ RUN      ] IdentityApiTest.GetAuthToken
[       OK ] IdentityApiTest.GetAuthToken (981 ms)
[ RUN      ] IdentityApiTest.RemoveCachedAuthToken
[       OK ] IdentityApiTest.RemoveCachedAuthToken (3 ms)
[ RUN      ] IdentityApiTest.GetAuthTokenNoRefreshToken
[       OK ] IdentityApiTest.GetAuthTokenNoRefreshToken (2 ms)
[----------] 3 tests from IdentityApiTest (986 ms total)

[----------] 2 tests from ShellPrefsTest
[ RUN      ] ShellPrefsTest.CreateLocalState
[       OK ] ShellPrefsTest.CreateLocalState (2 ms)
[ RUN      ] ShellPrefsTest.CreateUserPrefService
[       OK ] ShellPrefsTest.CreateUserPrefService (1 ms)
[----------] 2 tests from ShellPrefsTest (3 ms total)

[----------] 1 test from ShellNativeAppWindowAuraTest
[ RUN      ] ShellNativeAppWindowAuraTest.Bounds
[136608:136608:0730/164626.103192:8623809601118:FATAL:dependency_manager.cc(108)] Check failed: false. Attempted to access a context that was ShutDown(). This is most likely a heap smasher in progress. After KeyedService::Shutdown() completes, your service MUST NOT refer to depended services again.
#0 0x7f1bb25f9f37 base::debug::StackTrace::StackTrace()
#1 0x7f1bb2620f71 logging::LogMessage::~LogMessage()
#2 0x7f1bb2b74375 DependencyManager::AssertContextWasntDestroyed()
#3 0x7f1bb2b81b7f BrowserContextKeyedServiceFactory::GetContextToUse()
#4 0x7f1bb2b75057 KeyedServiceFactory::GetServiceForContext()
#5 0x0000004b5177 extensions::AppWindow::OnNativeClose()
#6 0x0000004aca24 extensions::ShellNativeAppWindowAuraTest_Bounds_Test::TestBody()
#7 0x0000004f7b66 testing::Test::Run()
#8 0x0000004f8330 testing::TestInfo::Run()
#9 0x0000004f8817 testing::TestCase::Run()
#10 0x0000004feb47 testing::internal::UnitTestImpl::RunAllTests()
#11 0x0000004fe7d3 testing::UnitTest::Run()
#12 0x000000514eb4 base::TestSuite::Run()
#13 0x0000005163aa base::(anonymous namespace)::LaunchUnitTestsInternal()
#14 0x00000051626a base::LaunchUnitTests()
#15 0x0000004a6a4e main
#16 0x7f1ba9abaf45 __libc_start_main
#17 0x0000004a68e9 <unknown>

At first glance, this doesn't appear related to changes I've been making to app_shell.
 
One thing I've noticed: AppWindowClient::Set is called multiple times with different values, and the later values are ignored because multiple calls "can happen in unit tests, where the utility thread runs in-process."

https://cs.chromium.org/chromium/src/extensions/browser/app_window/app_window_client.cc?type=cs&q=appwindowclient::set+file:app_window_client%5C.cc&sq=package:chromium&l=20

That wouldn't explain why this test fails in single-process mode, but it's something we should fix because the value of |g_client| might not point to what a test is expecting. ShellDesktopControllerAuraTest.InputEvents  and ShellNativeAppWindowAuraTest.Bounds each causes a different AppWindowClient to be set globally, and at least ShellDesktopControllerAuraTest.InputEvents's AppWindowClient gets freed at some point.
Blockedon: 720073
Cc: -osh...@chromium.org -benwells@chromium.org michae...@chromium.org blundell@chromium.org
Owner: ----
Status: Available (was: Started)
Disabling the IdentityApiTests locally seems to remove the flakiness. I suspect this is related to issue 720073, where the Identity API is causing the same AssertContextWasntDestroyed error.

The AppWindowClient stuff is a red herring; changing how that works didn't fix the tests.
Blockedon: -720073
Owner: michae...@chromium.org
Status: Started (was: Available)
OK, I actually think the problem is in the test itself. I will upload a CL shortly.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 2 2017

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

commit 93ca0f8e1defd6a2aad2ae93fac9bf7e9c78409e
Author: Michael Giuffrida <michaelpg@chromium.org>
Date: Wed Aug 02 01:41:06 2017

Fix flaky ShellNativeAppWindowAuraTest.Bounds

This test creates its own TestBrowserContext, but it derives from
ExtensionsTest, which also creates one. Making the test use the
provided context instead of creating its own fixes the flake. The
flake is fairly rare but becomes more frequent as we add new tests
to app_shell_unittests. Read on for details on how this failure
occurs, why it was flaky and why more tests make it fail more often.

When run in combination with other tests, once in a while the new
context happened to be created at the same address as a context used
in a previous test, just by chance.

The process-wide DependencyManager keeps track of the pointers to
contexts that have already been shut down. ExtensionsTest is smart
enough to register its own context as "live", so this flake doesn't
normally happen, but this test didn't bother.

A new context at an old address would be fine *if* the context was
marked "live" again. When it isn't, accessing KeyedService with the new
context will cause DependencyManager to identify it as a "dead" context
(as seen in a previous test) and assert.

If we really needed a separate TestBrowserContext, we could mark it
live, but why bother?

Bug:  750530 
Change-Id: I094be7ea4c715a8316547fb2b9f06c081f963483
Reviewed-on: https://chromium-review.googlesource.com/592546
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Elliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491178}
[modify] https://crrev.com/93ca0f8e1defd6a2aad2ae93fac9bf7e9c78409e/extensions/shell/browser/shell_native_app_window_aura_unittest.cc

Status: Fixed (was: Started)
Flakiness looks fixed, as expected.

Sign in to add a comment