New issue
Advanced search Search tips

Issue 624584 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 624201



Sign in to add a comment

Various Kiosk browser tests DCHECKing while trying to show shelf

Project Member Reported by glevin@chromium.org, Jun 29 2016

Issue description

On 6/24, a number of Kiosk browser tests started DCHECKing and failing on Linux ChromiumOS Tests (dbg).  The DCHECK is
    DCHECK(!visible_ || layer()->GetTargetOpacity() > 0.0f);
in Window::Show():
https://cs.chromium.org/chromium/src/ui/aura/window.cc?q=Window::Show&sq=package:chromium&dr=CSs&l=235

See recent failures in
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29?numbuilds=200

Some tests observed to have failed in this specific way are:

KioskUpdateTest.IncompliantPlatformFirstInstall
KioskUpdateTest.LaunchAppWithSecondaryArcLikeAppAndExtension
KioskUpdateTest.LaunchAppWithSharedModuleAndSecondaryApp
KioskUpdateTest.LaunchKioskAppWithSecondaryExtension
KioskUpdateTest.PRE_LaunchAppWithUpdatedModule
KioskUpdateTest.PRE_UpdateMultiAppKioskAddOneApp
KioskUpdateTest.PRE_UpdateMultiAppKioskRemoveOneApp
KioskEnterpriseTest.PrivateStore

... and so forth.  The problem always happens when the test calls WaitForAppLaunchWithOptions().  The message loop runs StartupAppLauncher::OnReadyToLaunch(), which leads to Shell::ShowShelf(), and Widget::Show() on the StatusAreaWidget; then the NativeWidgetAura calls window_->Show().

When I traced through this, the StatusAreaWidget's Show() function was called 47 times. Regarding the DCHECK conditions: sometimes the window_ was visible and opacity was 1; sometimes it wasn't visible, and opacity was 1.  On the Show() call right before the DCHECK, it wasn't visible, and opacity was 0.  When it DCHECKED, of course, it was now visible, but opacity was still 0.  Note that this this doesn't always happen... sometimes these tests pass.  So whatever is showing and hiding this window and setting its opacity is doing it in a non-deterministic order.

I don't know if this is related to  Issue 612688 .  Those aren't running on (dbg), and the errors seem different.

Here's the call stack:

#1 logging::LogMessage::~LogMessage()
#2 aura::Window::Show()
#3 views::NativeWidgetAura::ShowWithWindowState()
#4 views::Widget::Show()
#5 ash::RootWindowController::ShowShelf()
#6 ash::Shell::ShowShelf()
#7 ChromeShellDelegate::Observe()
#8 content::NotificationServiceImpl::Notify()
#9 chromeos::ChromeUserManagerImpl::SessionStarted()
#10 chromeos::StartupAppLauncher::LaunchApp()
#11 chromeos::AppLaunchController::OnReadyToLaunch()
#12 chromeos::StartupAppLauncher::OnReadyToLaunch()
#13 base*internal*RunnableAdapter*CancelableCallback*Run*WeakPtr*
#14 base*internal*InvokeHelper*MakeItSo*RunnableAdapter*CancelableCallback*WeakPtr*
#15 base*internal*Invoker*IndexSequence*BindState*RunnableAdapter*chromeos*StartupAppLauncher*WeakPtr*BindStateBase*
#16 base::Callback<>::Run()
#17 base::debug::TaskAnnotator::RunTask()
#18 base::MessageLoop::RunTask()
#19 base::MessageLoop::DeferOrRunPendingTask()
#20 base::MessageLoop::DoWork()
#21 base::MessagePumpGlib::Run()
#22 base::MessageLoop::RunHandler()
#23 base::RunLoop::Run()
#24 content::RunThisRunLoop()
#25 content::MessageLoopRunner::Run()
#26 content::WindowedNotificationObserver::Wait()
#27 chromeos::KioskTest::WaitForAppLaunchWithOptions()
#28 chromeos::KioskEnterpriseTest_PrivateStore_Test::RunTestOnMainThread() (for example)

... more to come...
 

Comment 1 by glevin@chromium.org, Jun 30 2016

Cc: glevin@chromium.org
Labels: -Pri-2 Pri-1
Owner: xiy...@chromium.org
Status: Assigned (was: Available)
xiyuan@, could you take a look at this?  Multiple KioskUpdateTest.s are failing in every run of Linux ChromiumOS Tests (dbg):
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29?numbuilds=200

There are multiple failures in every run (can search logs for "WaitForAppLaunchWithOptions").  The only reason we have some green is that sometimes tests pass on subsequent retry.  As far as I can see, this particular failure never happens in KioskTest.s, just in KioskUpdateTest.s and KioskEnterpriseTest.s.

Comment 2 by xiy...@chromium.org, Jun 30 2016

Cc: xiy...@chromium.org
Owner: afakhry@chromium.org
Pass to afakhry@ who is helping to look at those failures. I'll dup the other one into this since glevin@ put more context here.

Comment 3 by xiy...@chromium.org, Jun 30 2016

Cc: afakhry@chromium.org
 Issue 624710  has been merged into this issue.
Blocking: 624201

Comment 5 by xiy...@chromium.org, Jun 30 2016

The browser tests start with clean state, so we would load login screen as oobe, which would show the bottom bar after 300ms [1]. The handler of that would mark status area as visible. The status area is made visible by toggling the visibility flag with an opacity animation [3].

Now the fun part, if the above happens right before we finish kiosk app launch that calls ChromeUserManagerImpl::SessionStarted(). We end up trying to show the status area widget with visible_ true but opacity 0, hence the DCHECK.

Not sure about what could be a proper fix though. :(

[1]: https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/login_shared.js?rcl=0&l=98
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/chromeos/login/core_oobe_handler.cc?rcl=0&l=470
[3] https://cs.chromium.org/chromium/src/ash/common/system/tray/tray_background_view.cc?rcl=1467300303&l=296
Components: -Test>Android Tests
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 1 2016

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

commit 1828054aceaf6bc19e5ac5c5bc49689c47930df3
Author: afakhry <afakhry@chromium.org>
Date: Fri Jul 01 18:33:03 2016

Fix flaky Kiosk browser_tests failing on wrong widget opacity.

Ash was trying to show the shelf with its widget opacity set to 0.0 causing
a DCHECK crash in the tests. This makes sure the opacity of the system tray's
widget is set properly when it's shown or hidden.

Also re-enables a test that was disabled previously since it's passing now due
to this CL.

BUG= 624584 
TEST=browser_tests --gtest_filter=KioskEnterpriseTest.PrivateStore
TEST=browser_tests --gtest_filter=KioskUpdateTest.IncompliantPlatformFirstInstall

Review-Url: https://codereview.chromium.org/2119913002
Cr-Commit-Position: refs/heads/master@{#403489}

[modify] https://crrev.com/1828054aceaf6bc19e5ac5c5bc49689c47930df3/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://crrev.com/1828054aceaf6bc19e5ac5c5bc49689c47930df3/chrome/browser/chromeos/login/ui/webui_login_view.cc

Status: Fixed (was: Assigned)
Labels: VerifyIn-54
Status: Verified (was: Fixed)
bulk verified

Sign in to add a comment