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

Issue 725667 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

desktopui_MusLogin CHECK failure in ImageSkia::MakeThreadSafe from ash::WallpaperController::SetWallpaperImage

Project Member Reported by jamescook@chromium.org, May 23 2017

Issue description

This happens immediately after the ui::MusThreadProxy::GpuRefreshNativeDisplays crash in  issue 725659 , so it might be related to that.

https://wmatrix.googleplex.com/testrun/unfiltered?test_ids=486204748

I'm not sure what's going on with the UserImageRequest::OnDecodeImageFailed() part of the stack -- why would a user image problem lead to trying to set the wallpaper?

Crash reason:  SIGTRAP
Crash address: 0x0
Process uptime: not available

Thread 0 (crashed)
 0  chrome!gfx::ImageSkia::MakeThreadSafe() + 0x98
 1  chrome!ash::WallpaperController::SetWallpaperImage(gfx::ImageSkia const&, wallpaper::WallpaperLayout) + 0x18b
 2  chrome!chromeos::(anonymous namespace)::SetWallpaper(gfx::ImageSkia const&, wallpaper::WallpaperLayout) + 0xf1
 3  chrome!void base::internal::Invoker<base::internal::BindState<void (chromeos::WallpaperManager::*)(base::FilePath const&, wallpaper::WallpaperLayout, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >*, std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> >, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >), base::WeakPtr<chromeos::WallpaperManager>, base::FilePath, wallpaper::WallpaperLayout, base::internal::UnretainedWrapper<std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> > >, base::internal::PassedWrapper<std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> > > >, void (std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >)>::RunImpl<void (chromeos::WallpaperManager::* const&)(base::FilePath const&, wallpaper::WallpaperLayout, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >*, std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> >, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >), std::tuple<base::WeakPtr<chromeos::WallpaperManager>, base::FilePath, wallpaper::WallpaperLayout, base::internal::UnretainedWrapper<std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> > >, base::internal::PassedWrapper<std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> > > > const&, 0ul, 1ul, 2ul, 3ul, 4ul>(void (chromeos::WallpaperManager::* const&)(base::FilePath const&, wallpaper::WallpaperLayout, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >*, std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> >, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >), std::tuple<base::WeakPtr<chromeos::WallpaperManager>, base::FilePath, wallpaper::WallpaperLayout, base::internal::UnretainedWrapper<std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> > >, base::internal::PassedWrapper<std::unique_ptr<wallpaper::MovableOnDestroyCallback, std::default_delete<wallpaper::MovableOnDestroyCallback> > > > const&, base::IndexSequence<0ul, 1ul, 2ul, 3ul, 4ul>, std::unique_ptr<user_manager::UserImage, std::default_delete<user_manager::UserImage> >&&) + 0xc8
 4  chrome!chromeos::user_image_loader::(anonymous namespace)::UserImageRequest::OnDecodeImageFailed() + 0x3f
 5  chrome!(anonymous namespace)::OnDecodeImageDone(base::Callback<void (int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (SkBitmap const&, int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, int, SkBitmap const&) + 0x46
 6  chrome!base::internal::Invoker<base::internal::BindState<void (*)(base::Callback<void (int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (SkBitmap const&, int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, int, SkBitmap const&), base::Callback<void (int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (SkBitmap const&, int), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, int>, void (SkBitmap const&)>::Run(base::internal::BindStateBase*, SkBitmap const&) + 0x57
 7  chrome!base::internal::Invoker<base::internal::BindState<base::Callback<void (SkBitmap const&), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, SkBitmap>, void ()>::RunOnce(base::internal::BindStateBase*) + 0x36
 8  chrome!base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) + 0x101
 9  chrome!base::MessageLoop::RunTask(base::PendingTask*) + 0x176
10  chrome!base::MessageLoop::DoWork() + 0x3e9
11  chrome!base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) + 0x18c
12  chrome!base::RunLoop::Run() + 0x40
13  chrome!ChromeBrowserMainParts::MainMessageLoopRun(int*) + 0xc2
14  chrome!content::BrowserMainLoop::RunMainMessageLoopParts() + 0x44
15  chrome!content::BrowserMainRunnerImpl::Run() + 0x12
16  chrome!content::BrowserMain(content::MainFunctionParams const&) + 0x8c
17  chrome!content::ContentMainRunnerImpl::Run() + 0x184
18  chrome!service_manager::Main(service_manager::MainParams const&) + 0x6e0
19  chrome!content::ContentMain(content::ContentMainParams const&) + 0x51
20  chrome!ChromeMain + 0x96

Ideas anyone?

 
stack1.txt
37.6 KB View Download

Comment 1 by msw@chromium.org, May 23 2017

Cc: alemate@chromium.org hashimoto@chromium.org achuith@chromium.org satorux@chromium.org
It does seem odd that UserImageRequest::OnDecodeImageFailed calls the same image_info_.loaded_cb as UserImageRequest::OnImageFinalized, but that seems to be a very old pattern:
https://chromium.googlesource.com/chromium/src/+blame/8c98ad1cd5bf430ed2beb606316ee236d45dc389/chrome/browser/chromeos/login/user_image_loader.cc

CC'ing some other owners and contributors, I can look if we just want WallpaperController to bail on null/invalid images.

Comment 2 by xiy...@chromium.org, May 24 2017

I guess user wallpaper is considered as a user image thus user image loader code is used here. 

The API convention seems to be that the loaded_cb will be called when the loading finishes regardless of whether the load succeeds or not.

There are 4 user_image_loader::StartWithFilePath calls in WallpaperManager. The crash signature suggests that the crash is in WallpaperManager::OnDefaultWallpaperDecoded. The code probably assumes that default wallpaper load should never fail. The other 3 call sites all check the load result.

Comment 3 by msw@chromium.org, May 26 2017

Cc: bshe@chromium.org
Owner: x...@chromium.org
Status: Assigned (was: Untriaged)
Yeah, maybe OnDefaultWallpaperDecoded (or the local SetWallpaper helper function) should just bail if user_image->image().isNull()? Assigning to xdai@ for wallpaper OWNERS insight, but lmk if you want help making that simple workaround fix.

Comment 4 by x...@chromium.org, Jun 9 2017

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 14 2017

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

commit 26f7ab8456c7e21751bd55ef1112a063dfc25203
Author: xdai <xdai@chromium.org>
Date: Wed Jun 14 20:33:28 2017

Fix the crash that happens if the default wallpaper image decoding failed.

BUG= 725667 

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

[modify] https://crrev.com/26f7ab8456c7e21751bd55ef1112a063dfc25203/chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc

 Issue 730322  has been merged into this issue.
Components: -Internals>MUS Internals>Services>WindowService
Labels: -Proj-Mustash-Mus Proj-Mustash
Migrating Proj-Mustash-Mus to components Internals>Services>WindowService and Internals>Services>Ash

Labels: -Proj-Mustash Proj-Mash
Status: WontFix (was: Started)
I'm closing this out, assuming it's fixed. If it's still an issue, please reopen.

Sign in to add a comment