New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 872070 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 9
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 866708



Sign in to add a comment

LockScreenAppStateWebUiLockTest failures with Mojo changes

Project Member Reported by roc...@chromium.org, Aug 8

Issue description

The failing test is (in unit_tests):

  LockScreenAppStateWebUiLockTest
    .LaunchActionWhenStylusRemoved_ActionClosedBeforeAnimationDone

See blocked bug for details and motivation regarding what's changing in Mojo. Likely the failures are caused by incorrect task ordering assumptions.
 
Labels: OS-Chrome
Cc: jdufault@chromium.org
Labels: -Pri-3 Pri-1
+jdufault - any chance you have cycles to help look into this?
And just to clarify, these failures appear when applying this CL[1]. The CL does not break any guarantees made by Mojo bindings, but it can affect the timing of arbitrary events. The failures therefore strongly imply that incorrect ordering assumptions being made somewhere, and this has been the case for all other blocking bugs which have been fixed so far.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1145692
Cc: -jdufault@chromium.org tbarzic@chromium.org
tbarzic@ worked on the stylus-related lock screen features

Webui-based lock screen hasn't been used in a few milestones and we're planning to remove it from the codebase soon (m71ish), so it may be fine to just disable this test.
Cc: jdufault@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 9

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

commit 722ad8e096ad99eb0104be5c6d5c3851e316f6e0
Author: Toni Barzic <tbarzic@google.com>
Date: Thu Aug 09 22:24:59 2018

In LockScreenAppState tests flush mojo after new note requests

Make sure every call to SendNewNoteRequest is followed by call to
FlushTrayActionForTesting, which should flush mojo pipes.
For most instances this was already the case, but it was missing in few
places - these were followed up with ExpectObservedStatesMatch, which
did fluch of its own (to ensure the mojo pipe is flushed in other
direction), which flushed the new note request as well, or were
expecting mojo call to have no effect.

With CL:1145692, this does not seem to be enough anymore, as flush in
ExpectObservedStatesMatch ends up handling only a single mojo message.

While here, replace checks for empty observed state changed with calls
to ExpectObservedStatesMatch.

TEST=run tets with CL:1145692 applied
BUG= 872070 

Change-Id: I58447ffe8315ddcd5917f0ce901ea5b2624b4e2a
Reviewed-on: https://chromium-review.googlesource.com/1170026
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581917}
[modify] https://crrev.com/722ad8e096ad99eb0104be5c6d5c3851e316f6e0/chrome/browser/chromeos/lock_screen_apps/state_controller_unittest.cc

Cc: -tbarzic@chromium.org roc...@chromium.org
Owner: tbarzic@chromium.org
Status: Fixed (was: Started)
Thanks!

Sign in to add a comment