Commit 364d528b authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

tablet: Speculative fix for signature ui::LayerAnimator::RemoveObserver.

Stack trace shows window destroying calling |animating_layer_| which is
the one of the windows associated with the destroyed window. My guess is
that it might be a dangling pointer, so when window is destroying, set
it to null.

I tried reproing with a test that destroys the windows at certain points
of the animation, but no luck :(.

Test: none
Bug: 1028815
Change-Id: I5baa2d3784836296c2afe063e6861aa43fb68e5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111059
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarXiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752670}
parent fa830869
...@@ -426,6 +426,7 @@ void TabletModeController::MaybeObserveBoundsAnimation(aura::Window* window) { ...@@ -426,6 +426,7 @@ void TabletModeController::MaybeObserveBoundsAnimation(aura::Window* window) {
/*delete_screenshot=*/true)); /*delete_screenshot=*/true));
animating_layer_ = window->layer(); animating_layer_ = window->layer();
animating_layer_->GetAnimator()->AddObserver(this); animating_layer_->GetAnimator()->AddObserver(this);
animating_layer_->AddObserver(this);
} }
void TabletModeController::StopObservingAnimation(bool record_stats, void TabletModeController::StopObservingAnimation(bool record_stats,
...@@ -434,9 +435,17 @@ void TabletModeController::StopObservingAnimation(bool record_stats, ...@@ -434,9 +435,17 @@ void TabletModeController::StopObservingAnimation(bool record_stats,
ResetDestroyObserver(); ResetDestroyObserver();
if (animating_layer_) if (animating_layer_) {
animating_layer_->GetAnimator()->RemoveObserver(this); animating_layer_->GetAnimator()->StopAnimating();
animating_layer_ = nullptr;
// If the observed layer is part of a cross fade animation, stopping the
// animation will end up destroying the layer.
if (animating_layer_) {
animating_layer_->RemoveObserver(this);
animating_layer_->GetAnimator()->RemoveObserver(this);
animating_layer_ = nullptr;
}
}
if (record_stats && fps_counter_) if (record_stats && fps_counter_)
fps_counter_->LogUma(); fps_counter_->LogUma();
...@@ -695,6 +704,13 @@ void TabletModeController::OnLayerAnimationScheduled( ...@@ -695,6 +704,13 @@ void TabletModeController::OnLayerAnimationScheduled(
StopObservingAnimation(/*record_stats=*/false, /*delete_screenshot=*/true); StopObservingAnimation(/*record_stats=*/false, /*delete_screenshot=*/true);
} }
void TabletModeController::LayerDestroyed(ui::Layer* layer) {
DCHECK_EQ(animating_layer_, layer);
animating_layer_->RemoveObserver(this);
animating_layer_->GetAnimator()->RemoveObserver(this);
animating_layer_ = nullptr;
}
void TabletModeController::SetEnabledForDev(bool enabled) { void TabletModeController::SetEnabledForDev(bool enabled) {
tablet_mode_behavior_ = enabled ? kOnForDev : kDefault; tablet_mode_behavior_ = enabled ? kOnForDev : kDefault;
force_notify_events_blocking_changed_ = true; force_notify_events_blocking_changed_ = true;
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "chromeos/dbus/power/power_manager_client.h" #include "chromeos/dbus/power/power_manager_client.h"
#include "ui/aura/window_occlusion_tracker.h" #include "ui/aura/window_occlusion_tracker.h"
#include "ui/compositor/layer_animation_observer.h" #include "ui/compositor/layer_animation_observer.h"
#include "ui/compositor/layer_observer.h"
#include "ui/compositor/layer_tree_owner.h" #include "ui/compositor/layer_tree_owner.h"
#include "ui/events/devices/input_device_event_observer.h" #include "ui/events/devices/input_device_event_observer.h"
#include "ui/gfx/geometry/vector3d_f.h" #include "ui/gfx/geometry/vector3d_f.h"
...@@ -71,7 +72,8 @@ class ASH_EXPORT TabletModeController ...@@ -71,7 +72,8 @@ class ASH_EXPORT TabletModeController
public WindowTreeHostManager::Observer, public WindowTreeHostManager::Observer,
public SessionObserver, public SessionObserver,
public ui::InputDeviceEventObserver, public ui::InputDeviceEventObserver,
public ui::LayerAnimationObserver { public ui::LayerAnimationObserver,
public ui::LayerObserver {
public: public:
// Enable or disable using a screenshot for testing as it makes the // Enable or disable using a screenshot for testing as it makes the
// initialization flow async, which makes most tests harder to write. // initialization flow async, which makes most tests harder to write.
...@@ -158,6 +160,9 @@ class ASH_EXPORT TabletModeController ...@@ -158,6 +160,9 @@ class ASH_EXPORT TabletModeController
void OnLayerAnimationAborted(ui::LayerAnimationSequence* sequence) override; void OnLayerAnimationAborted(ui::LayerAnimationSequence* sequence) override;
void OnLayerAnimationScheduled(ui::LayerAnimationSequence* sequence) override; void OnLayerAnimationScheduled(ui::LayerAnimationSequence* sequence) override;
// ui::LayerObserver:
void LayerDestroyed(ui::Layer* layer) override;
void increment_app_window_drag_count() { ++app_window_drag_count_; } void increment_app_window_drag_count() { ++app_window_drag_count_; }
void increment_app_window_drag_in_splitview_count() { void increment_app_window_drag_in_splitview_count() {
++app_window_drag_in_splitview_count_; ++app_window_drag_in_splitview_count_;
...@@ -447,6 +452,8 @@ class ASH_EXPORT TabletModeController ...@@ -447,6 +452,8 @@ class ASH_EXPORT TabletModeController
// everything in the screen rotation container except the top window. It helps // everything in the screen rotation container except the top window. It helps
// with animation performance because it fully occludes all windows except the // with animation performance because it fully occludes all windows except the
// animating window for the duration of the animation. // animating window for the duration of the animation.
// TODO(sammiequon): See if we can move screenshot and tablet mode transition
// animation related code into a separate class/file.
std::unique_ptr<ui::Layer> screenshot_layer_; std::unique_ptr<ui::Layer> screenshot_layer_;
base::ObserverList<TabletModeObserver>::Unchecked tablet_mode_observers_; base::ObserverList<TabletModeObserver>::Unchecked tablet_mode_observers_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment