Commit 730f5630 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Fix crash for navigation glow shutdown.

NavigationGlow can be destroyed after the window and view
associated with it, so we need to observe those events happening
and clean up accordingly.

OnDetachCompositor is called prior to the WindowAndroid being destroyed,
and surprisingly clears the observer list, so that is the best option
we have to clean up that reference.

BUG=1048851

Change-Id: I61e081dcab951ceeb6515a4e79e2d42ea2a96f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2065416
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743316}
parent b6e76435
......@@ -45,7 +45,7 @@ NavigationGlow::~NavigationGlow() = default;
void NavigationGlow::OnAttachedToWindow() {
window_ = view_->GetWindowAndroid();
if (window_) {
if (window_ != nullptr) {
window_->AddObserver(this);
if (!glow_effect_)
glow_effect_ = std::make_unique<ui::OverscrollGlow>(this);
......@@ -53,13 +53,22 @@ void NavigationGlow::OnAttachedToWindow() {
}
void NavigationGlow::OnDetachedFromWindow() {
if (window_) {
if (window_ != nullptr) {
window_->RemoveObserver(this);
glow_effect_.reset();
}
window_ = nullptr;
}
void NavigationGlow::OnViewAndroidDestroyed() {
// Ideally, ViewAndroid should be detaching itself as part of destruction, but
// this also clears the window reference as the View is being destroyed prior
// to the Window.
OnDetachedFromWindow();
view_ = nullptr;
layer_ = nullptr;
}
void NavigationGlow::Prepare(JNIEnv* env,
const JavaParamRef<jobject>& obj,
jfloat start_x,
......@@ -95,12 +104,13 @@ void NavigationGlow::OnReset(JNIEnv* env, const JavaParamRef<jobject>& obj) {
void NavigationGlow::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
OnDetachedFromWindow();
if (view_ != nullptr)
view_->RemoveObserver(this);
delete this;
}
void NavigationGlow::OnAnimate(base::TimeTicks frame_time) {
if (glow_effect_->Animate(frame_time, layer_))
if (layer_ != nullptr && glow_effect_->Animate(frame_time, layer_))
window_->SetNeedsAnimate();
}
......
......@@ -57,6 +57,7 @@ class NavigationGlow : public ui::OverscrollGlowClient,
// ui::ViewAndroidObserver implementation.
void OnAttachedToWindow() override;
void OnDetachedFromWindow() override;
void OnViewAndroidDestroyed() override;
private:
// OverscrollGlowClient implementation.
......
......@@ -90,6 +90,8 @@ ViewAndroid::ViewAndroid() : ViewAndroid(LayoutType::NORMAL) {}
ViewAndroid::~ViewAndroid() {
RemoveAllChildren(GetWindowAndroid() != nullptr);
for (auto& observer : observer_list_)
observer.OnViewAndroidDestroyed();
observer_list_.Clear();
RemoveFromParent();
}
......
......@@ -19,6 +19,9 @@ class UI_ANDROID_EXPORT ViewAndroidObserver {
// is not sent if view is already in detached state.
virtual void OnDetachedFromWindow() = 0;
// Notifies the view has been destroyed.
virtual void OnViewAndroidDestroyed() {}
protected:
virtual ~ViewAndroidObserver() {}
};
......
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