Chrome_ChromeOS: Crash Report - ash::ScreenRotationAnimation::OnAbort |
|||||||||
Issue descriptionIn top 20 crashes in M-55 dev/beta. Product name: Chrome_ChromeOS Magic Signature: ash::ScreenRotationAnimation::OnAbort Current link: https://crash.corp.google.com/browse?q=product.name%3D'Chrome_ChromeOS'%20AND%20product.version%3D'55.0.2883.29'%20AND%20custom_data.ChromeCrashProto.ptype%3D'browser'%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D'ash%3A%3AScreenRotationAnimation%3A%3AOnAbort'%20AND%20ReportID%3D'34b05a7900000000'&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#3 Search properties: product.name: Chrome_ChromeOS product.version: 55.0.2883.29 custom_data.chromecrashproto.ptype: browser custom_data.chromecrashproto.magic_signature_1.name: ash::ScreenRotationAnimation::OnAbort reportid: 34b05a7900000000 Metadata : Product Name: Chrome_ChromeOS Product Version: 55.0.2883.29 Report ID: 34b05a7900000000 Report Time: Tue, 01 Nov 2016 19:28:01 GMT Uptime: 5862 ms Cumulative Uptime: 0 ms User Email: OS Name: Linux OS Version: 0.0.0 Linux 3.14.0 #1 SMP PREEMPT Wed Oct 26 21:47:53 PDT 2016 armv7l CPU Architecture: arm CPU Info: ARMv1 ARM part(0x4100c0d0) features: swp,half,thumb,fastmult,vfpv2,edsp,thumbee,neon,vfpv3,tls,vfpv4,idiva,idivt Thread 0 CRASHED [SIGSEGV @ 0x00000000 ] MAGIC SIGNATURE THREAD 0xb4cc238e (chrome -screen_rotation_animation.cc:63 ) ash::ScreenRotationAnimation::OnAbort 0xb44fb175 (chrome -layer_animation_element.cc:556 ) ui::LayerAnimationElement::Abort 0xb44f56e1 (chrome -layer_animation_sequence.cc:177 ) ui::LayerAnimationSequence::Abort 0xb44f77f3 (chrome -layer_animator.cc:904 ) ui::LayerAnimator::ClearAnimationsInternal 0xb44f78a3 (chrome -layer_animator.cc:66 ) ui::LayerAnimator::~LayerAnimator 0xb44f7983 (chrome -layer_animator.cc:69 ) ui::LayerAnimator::~LayerAnimator 0xb44f3abf (chrome -ref_counted.h:407 ) ui::Layer::SetAnimator 0xb44f46d3 (chrome -layer.cc:104 ) ui::Layer::~Layer 0xb44f487f (chrome -layer.cc:119 ) ui::Layer::~Layer 0xb4c7ed0d (chrome -unique_ptr.h:76 ) ash::SystemWallpaperController::~SystemWallpaperController 0xb4c7ed33 (chrome -system_wallpaper_controller.cc:27 ) ash::SystemWallpaperController::~SystemWallpaperController 0xb4c6e99d (chrome -unique_ptr.h:76 ) ash::RootWindowController::Shutdown 0xb4c6e9d1 (chrome -root_window_controller.cc:212 ) ash::RootWindowController::~RootWindowController 0xb4c6ea77 (chrome -root_window_controller.cc:217 ) ash::RootWindowController::~RootWindowController 0xb4c6b66b (chrome -window_tree_host_manager.cc:287 ) ash::WindowTreeHostManager::Shutdown 0xb4c71557 (chrome -shell.cc:572 ) ash::Shell::~Shell 0xb4c71bbf (chrome -shell.cc:602 ) ash::Shell::~Shell 0xb4e77f1f (chrome -ash_init.cc:107 ) chrome::CloseAsh 0xb38fda9d (chrome -chrome_browser_main.cc:2146 ) ChromeBrowserMainParts::PostMainMessageLoopRun 0xb3628ee9 (chrome -chrome_browser_main_chromeos.cc:863 ) chromeos::ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun 0xb3213faf (chrome -browser_main_loop.cc:1014 ) content::BrowserMainLoop::ShutdownThreadsAndCleanUp 0xb321694f (chrome -browser_main_runner.cc:211 ) content::BrowserMainRunnerImpl::Shutdown 0xb32127c9 (chrome -browser_main.cc:48 ) content::BrowserMain 0xb38b824d (chrome -content_main_runner.cc:779 ) content::ContentMainRunnerImpl::Run 0xb38b732d (chrome -content_main.cc:20 ) content::ContentMain 0xb2aef35f (chrome -chrome_main.cc:97 ) ChromeMain 0xb1cbc307 (libc-2.19.so -libc-start.c:285 ) __libc_start_main 0xb2aef223 (chrome + 0x0079d223 ) _start 0xb65fdb13 (chrome -elf-init.c:87 ) __libc_csu_init 0xb233b9df (ld-2.19.so + 0x0000b9df ) _dl_sort_fini
,
Nov 1 2016
Hmm, I don't know anything about this class. Presumably a null or freed delegate is being passed in:
60 void ScreenRotationAnimation::OnAbort(ui::LayerAnimationDelegate* delegate) {
61 TargetValue target_value;
62 OnGetTarget(&target_value);
63 delegate->SetTransformFromAnimation(target_value.transform);
64 }
ash/rotator/window_rotation.h has an incorrect "Implementation of ui::LayerAnimationDelegate" comment above some of its methods. ash::WindowRotation derives from ui::LayerAnimationElement, not ui::LayerAnimationDelegate.
I think that the only LayerAnimationDelegate is ui::Layer, so maybe ScreenRotationAnimation needs to observe its layer for destruction to avoid this... or something.
,
Nov 1 2016
+msw. Mash touched SystemWallpaperController on Aug 30, but that was just a rename: https://codereview.chromium.org/2290473004/ There has been some reshuffling of Shell/RootWindowController creation and shutdown. For example, see sky's CL https://codereview.chromium.org/2350953009/ In general we've tried to preserve the existing ordering, but it's possible we missed something. (FYI for my notes: M55 branched on October 7th.)
,
Nov 1 2016
Looked at the code some more and think I see the bug. Layer::~Layer() calls SetAnimator(nullptr), which sets the old LayerAnimator's delegate to null before destroying it. ScreenRotationAnimation::OnAbort() blindly uses the delegate that gets passed to it. I think it just needs to bail out early on null.
,
Nov 2 2016
Uploaded https://codereview.chromium.org/2473523003/ .
,
Nov 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee91b8bf594ec33c709a2a9826569fdd88ed98bd commit ee91b8bf594ec33c709a2a9826569fdd88ed98bd Author: derat <derat@chromium.org> Date: Wed Nov 02 14:36:56 2016 ash: Avoid a shutdown crash in ScreenRotationAnimation. Make ash::ScreenRotationAnimation::OnAbort() not dereference a null ui::LayerAnimationDelegate* when its layer is destroyed mid-animation. BUG= 661313 Review-Url: https://codereview.chromium.org/2473523003 Cr-Commit-Position: refs/heads/master@{#429277} [modify] https://crrev.com/ee91b8bf594ec33c709a2a9826569fdd88ed98bd/ash/rotator/screen_rotation_animation.cc [modify] https://crrev.com/ee91b8bf594ec33c709a2a9826569fdd88ed98bd/ash/rotator/screen_rotation_animation.h [modify] https://crrev.com/ee91b8bf594ec33c709a2a9826569fdd88ed98bd/ash/rotator/screen_rotation_animation_unittest.cc
,
Nov 2 2016
,
Nov 3 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Nov 3 2016
,
Nov 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f9ac2119692494dbc0a19be5181369d9670b1b6d commit f9ac2119692494dbc0a19be5181369d9670b1b6d Author: Daniel Erat <derat@chromium.org> Date: Thu Nov 03 15:20:40 2016 ash: Avoid a shutdown crash in ScreenRotationAnimation. Make ash::ScreenRotationAnimation::OnAbort() not dereference a null ui::LayerAnimationDelegate* when its layer is destroyed mid-animation. BUG= 661313 Review-Url: https://codereview.chromium.org/2473523003 Cr-Commit-Position: refs/heads/master@{#429277} (cherry picked from commit ee91b8bf594ec33c709a2a9826569fdd88ed98bd) Review URL: https://codereview.chromium.org/2474433009 . Cr-Commit-Position: refs/branch-heads/2883@{#432} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/f9ac2119692494dbc0a19be5181369d9670b1b6d/ash/rotator/screen_rotation_animation.cc [modify] https://crrev.com/f9ac2119692494dbc0a19be5181369d9670b1b6d/ash/rotator/screen_rotation_animation.h [modify] https://crrev.com/f9ac2119692494dbc0a19be5181369d9670b1b6d/ash/rotator/screen_rotation_animation_unittest.cc
,
Nov 17 2016
8872.54.0/55.2883.54 |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by zelidrag@chromium.org
, Nov 1 2016