New issue
Advanced search Search tips

Issue 743050 link

Starred by 1 user
Status: Fixed
Owner:
msw@chromium.org
Closed: Jul 2017
Cc:
mustash-bugs@google.com
jonr...@chromium.org
jamescook@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug



Sign in to add a comment

ShelfModel::AddAt Failure

Project Member Reported by jonr...@chromium.org, Jul 14 2017 
Hey msw@,

When running mash_browser_tests there are some errors occurring in ShelfModel. They don't seem tied to a particular test case.

Example with the failure, even though the test suite is for some reason marked as a success: https://build.chromium.org/p/chromium.fyi/builders/Mojo%20ChromiumOS/builds/19946

[0714/071557.480959:FATAL:shelf_model.cc(120)] Check failed: ItemIndexByID(item.id) == -1 (2 vs. -1) The id is not unique: app_id:mgndgikekgjfcpckkfioiadnlibdjbkf, launch_id:
#0 0x0000037b325c base::debug::StackTrace::StackTrace()
#1 0x0000037cb421 logging::LogMessage::~LogMessage()
#2 0x0000062f95c0 ash::ShelfModel::AddAt()
#3 0x0000061fcccb ash::ShelfController::AddShelfItem()
#4 0x000002b4d285 ash::mojom::ShelfControllerStubDispatch::Accept()
#5 0x000004d64a58 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#6 0x000004d646f6 mojo::FilterChain::Accept()
#7 0x000004d65bf5 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#8 0x000004d6cc5c mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#9 0x000004d6c44f mojo::internal::MultiplexRouter::Accept()
#10 0x000004d646f6 mojo::FilterChain::Accept()
#11 0x000004d639e5 mojo::Connector::ReadSingleMessage()
#12 0x000004d642e1 mojo::Connector::ReadAllAvailableMessages()
#13 0x000004d64199 mojo::Connector::OnHandleReadyInternal()
#14 0x0000019f41c5 chromeos::file_system_provider::Int64ToIntCompletionCallback()
#15 0x000004d7abe4 mojo::SimpleWatcher::OnHandleReady()
#16 0x000004d7b073 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKSt5tupleIJSB_ijS5_EEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#17 0x00000385fcb0 base::debug::TaskAnnotator::RunTask()
#18 0x0000037d2749 base::MessageLoop::RunTask()
#19 0x0000037d29fb base::MessageLoop::DeferOrRunPendingTask()
#20 0x0000037d2e04 base::MessageLoop::DoWork()
#21 0x0000037d52c9 base::MessagePumpLibevent::Run()
#22 0x0000037d237b base::MessageLoop::Run()
#23 0x0000037fc6aa base::RunLoop::Run()
#24 0x0000037a4142 (anonymous namespace)::StartEmbeddedService()
#25 0x0000037a497d _ZN4base8internal7InvokerINS0_9BindStateIPFvN4mojo16InterfaceRequestIN15service_manager5mojom7ServiceEEEEJEEES9_E3RunEPNS0_13BindStateBaseEOS8_
#26 0x0000021b4a42 service_manager::RunStandaloneService()
#27 0x0000037a3d6e RunMashBrowserTests()
#28 0x0000037a3bc7 main
#29 0x7f52d009ff45 __libc_start_main
#30 0x000000627b3e <unknown>

Received signal 6
#0 0x0000037b325c base::debug::StackTrace::StackTrace()
#1 0x0000037b2dd1 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#2 0x7f52d3788330 <unknown>
#3 0x7f52d00b4c37 gsignal
#4 0x7f52d00b8028 abort
#5 0x0000037b1ab5 base::debug::BreakDebugger()
#6 0x0000037cb7e7 logging::LogMessage::~LogMessage()
#7 0x0000062f95c0 ash::ShelfModel::AddAt()
#8 0x0000061fcccb ash::ShelfController::AddShelfItem()
#9 0x000002b4d285 ash::mojom::ShelfControllerStubDispatch::Accept()
#10 0x000004d64a58 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#11 0x000004d646f6 mojo::FilterChain::Accept()
#12 0x000004d65bf5 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#13 0x000004d6cc5c [0714/071557.740358:ERROR:shell_delegate_mus.cc(84)] Not implemented reached in virtual void ash::ShellDelegateMus::ShelfInit()
mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#14 0x000004d6c44f mojo::internal::MultiplexRouter::Accept()
#15 0x000004d646f6 mojo::FilterChain::Accept()
#16 0x000004d639e5 mojo::Connector::ReadSingleMessage()
#17 0x000004d642e1 mojo::Connector::ReadAllAvailableMessages()
#18 0x000004d64199 mojo::Connector::OnHandleReadyInternal()
#19 0x0000019f41c5 chromeos::file_system_provider::Int64ToIntCompletionCallback()
#20 0x000004d7abe4 mojo::SimpleWatcher::OnHandleReady()
#21 0x000004d7b073 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKSt5tupleIJSB_ijS5_EEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#22 0x00000385fcb0 base::debug::TaskAnnotator::RunTask()
#23 0x0000037d2749 base::MessageLoop::RunTask()
#24 0x0000037d29fb base::MessageLoop::DeferOrRunPendingTask()
#25 0x0000037d2e04 base::MessageLoop::DoWork()
#26 0x0000037d52c9 base::MessagePumpLibevent::Run()
#27 0x0000037d237b base::MessageLoop::Run()
#28 0x0000037fc6aa base::RunLoop::Run()
#29 0x0000037a4142 (anonymous namespace)::StartEmbeddedService()
#30 0x0000037a497d [0714/071557.612374:FATAL:shelf_model.cc(120)] Check failed: ItemIndexByID(item.id) == -1 (2 vs. -1) The id is not unique: app_id:mgndgikekgjfcpckkfioiadnlibdjbkf, launch_id:
#0 0x0000037b325c base::debug::StackTrace::StackTrace()
#1 0x0000037cb421 logging::LogMessage::~LogMessage()
#2 0x0000062f95c0 ash::ShelfModel::AddAt()
#3 0x0000061fcccb ash::ShelfController::AddShelfItem()
#4 0x000002b4d285 ash::mojom::ShelfControllerStubDispatch::Accept()
#5 0x000004d64a58 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#6 0x000004d646f6 mojo::FilterChain::Accept()
#7 0x000004d65bf5 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#8 0x000004d6cc5c mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#9 0x000004d6c44f mojo::internal::MultiplexRouter::Accept()
#10 0x000004d646f6 mojo::FilterChain::Accept()
#11 0x000004d639e5 mojo::Connector::ReadSingleMessage()
#12 0x000004d642e1 mojo::Connector::ReadAllAvailableMessages()
#13 0x000004d64199 mojo::Connector::OnHandleReadyInternal()
#14 0x0000019f41c5 chromeos::file_system_provider::Int64ToIntCompletionCallback()
#15 0x000004d7abe4 mojo::SimpleWatcher::OnHandleReady()
#16 0x000004d7b073 _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKSt5tupleIJSB_ijS5_EEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NS_13IndexSequenceIJXspT1_EEEE
#17 0x00000385fcb0 base::debug::TaskAnnotator::RunTask()
#18 0x0000037d2749 base::MessageLoop::RunTask()
#19 0x0000037d29fb base::MessageLoop::DeferOrRunPendingTask()
#20 0x0000037d2e04 base::MessageLoop::DoWork()
#21 0x0000037d52c9 base::MessagePumpLibevent::Run()
#22 0x0000037d237b base::MessageLoop::Run()
#23 0x0000037fc6aa base::RunLoop::Run()
#24 0x0000037a4142 (anonymous namespace)::StartEmbeddedService()
#25 0x0000037a497d _ZN4base8internal7InvokerINS0_9BindStateIPFvN4mojo16InterfaceRequestIN15service_manager5mojom7ServiceEEEEJEEES9_E3RunEPNS0_13BindStateBaseEOS8_
#26 0x0000021b4a42 service_manager::RunStandaloneService()
#27 0x0000037a3d6e RunMashBrowserTests()
#28 0x0000037a3bc7 main
#29 0x7fb321958f45 __libc_start_main
#30 0x000000627b3e <unknown>

Comment 1 by jonr...@chromium.org, Jul 19 2017

Labels: -Pri-3 Pri-0
The most recent run of mash_browser_tests on the FYI bot had this fail for each test.

We won't be able to get the tests re-enabled on the waterfall as long as this happens:

https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.fyi%2FMojo_ChromiumOS%2F20107%2F%2B%2Frecipes%2Fsteps%2Fmash_browser_tests%2F0%2Fstdout

It is flaky. Locally it repros 50% of the time with:

./out/${OUT_DIR}/browser_tests --override-use-software-gl-for-tests --run-in-mash --gtest_filter=BrowserTest.Title

Comment 2 by msw@chromium.org, Jul 19 2017 

Okay, I'll take a look today, thanks!
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 20 2017 

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

commit 78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84
Author: Michael Wasserman <msw@chromium.org>
Date: Thu Jul 20 19:51:20 2017

mash: Add a browser shortcut placeholder in ShelfModel.

Add the placeholder in ShelfModel's ctor, like the app list item.
Only call OnShelfItemDelegateChanged for items with delegates.
Rename CLC::InitBrowserShortcutItem, update expectations/behavior.
Bail early in CLC::OnShelfItemAdded for the browser shortcut too.

This should make mash_browser_tests run more reliably.
(avoids a race between ChromeLauncherController adding the shortcut and ShelfWindowWatcher observing the first chrome window)

Bug:  743050 
Test: No ChromeOS shelf behavior changes; more mash_browser_tests pass.
Change-Id: I96b3adb4fc3eff35f720d8b22048523dc6f1a6a3
Reviewed-on: https://chromium-review.googlesource.com/578547
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488364}
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/metrics/user_metrics_recorder_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/public/cpp/shelf_model.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/public/cpp/shelf_model_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/shelf/shelf_controller.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/shelf/shelf_controller_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/shelf/shelf_view.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/ash/wm/panels/panel_window_resizer_unittest.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/78b6f3ea50d1ae1d5603b17ab7d3470eecfd4c84/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

Comment 4 by jonr...@chromium.org, Jul 20 2017

Status: Fixed (was: Assigned)
The tests cycled green after your fix!!! 

https://build.chromium.org/p/chromium.fyi/builders/Mojo%20ChromiumOS/builds/20162

Thanks for working on this so quickly!

Comment 5 by msw@chromium.org, Jul 20 2017 

Great! Thanks for reporting the issue and checking the results!
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 23 2017 

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

commit 717cd5747ed764a9268c6b9538a9c51f8d73ab93
Author: Mike Wasserman <msw@chromium.org>
Date: Sat Sep 23 17:01:40 2017

mash: Refine shelf handling of browser windows and the shortcut item.

Restore browser shortcut item creation to ChromeLauncherController.
(Ash began adding it as a lame workaround in crrev.com/c/578547)

Set the shelf item type for browser windows on initialization.
(matches Chrome app window behavior added in crrev.com/c/644849)

ShelfWindowWatcher will eventually handle windows with unknown types.
For now it just handles windows with the panel/dialog type property.
Marking the shelf item type property on init avoids a race condition.
(ShelfWindow watcher creates items when adding windows to containers)

mash: Restore browser shortcut creation to Chrome.
Bug:  743050 ,  557406 
Test: No ChromeOS [--mash] shelf browser item behavior changes.
Change-Id: I1f7403a4c3e6f44f047296d61066d413c7504c1a
Reviewed-on: https://chromium-review.googlesource.com/679483
Commit-Queue: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503942}
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/metrics/user_metrics_recorder_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/public/cpp/shelf_model.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/public/cpp/shelf_model_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/shelf/shelf_controller_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/shelf/shelf_view_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/ash/shelf/shelf_window_watcher_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/717cd5747ed764a9268c6b9538a9c51f8d73ab93/chrome/browser/ui/views/frame/browser_frame_mus.cc

Comment 7 by lafo...@chromium.org, Feb 26 2018

Components: -MUS Internals>Services>WindowService
►

Sign in to add a comment