Commit 712b2013 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Android: Ensure |OnDetachedFromWindow| is invoked upon destruction

https://crrev.com/c/1020942 meant to make sure |DetachedFromWindow| is
invoked upon ViewAndroid destruction. This had a bug when
WindowAndroid is destroyed first, which can happen for Chromecast
where WebContents destruction is delayed by design.

WindowAndroid dtor invokes |GetWindowAndroid| indirectly via its
base dtor (through RemoveAllChildren), but it didn't work as intended
because |GetWindowAndroid| is a virtual function. So it ended up
calling |ViewAndroid::GetWindowAndroid|, not
|WindowAndroid::GetWindowAndroid|.

This CL fixes it by pulling out the task of invoking
|OnDetachedFromWindow| so that the destructor won't use the virtual
function. WindowAndroid can call |OnDetachedFromWindow| in its dtor
since it doesn't need |GetWindowAndroid| - it knows for sure
WindowAndroid (itself) is present.


Bug: b/78251221
Change-Id: I754e45ca3ea61ceb86101ede3b529e75e206689f
Reviewed-on: https://chromium-review.googlesource.com/1102823
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568064}
parent 9e073a82
...@@ -88,7 +88,7 @@ ViewAndroid::ViewAndroid(LayoutType layout_type) ...@@ -88,7 +88,7 @@ ViewAndroid::ViewAndroid(LayoutType layout_type)
ViewAndroid::ViewAndroid() : ViewAndroid(LayoutType::NORMAL) {} ViewAndroid::ViewAndroid() : ViewAndroid(LayoutType::NORMAL) {}
ViewAndroid::~ViewAndroid() { ViewAndroid::~ViewAndroid() {
RemoveAllChildren(); RemoveAllChildren(GetWindowAndroid() != nullptr);
observer_list_.Clear(); observer_list_.Clear();
RemoveFromParent(); RemoveFromParent();
} }
...@@ -252,11 +252,10 @@ gfx::PointF ViewAndroid::GetLocationOnScreen(float x, float y) { ...@@ -252,11 +252,10 @@ gfx::PointF ViewAndroid::GetLocationOnScreen(float x, float y) {
return gfx::PointF(x + loc_x, y + loc_y); return gfx::PointF(x + loc_x, y + loc_y);
} }
void ViewAndroid::RemoveAllChildren() { void ViewAndroid::RemoveAllChildren(bool attached_to_window) {
bool has_window = GetWindowAndroid();
auto it = children_.begin(); auto it = children_.begin();
while (it != children_.end()) { while (it != children_.end()) {
if (has_window) if (attached_to_window)
(*it)->OnDetachedFromWindow(); (*it)->OnDetachedFromWindow();
(*it)->parent_ = nullptr; (*it)->parent_ = nullptr;
// erase returns a new iterator for the element following the ereased one. // erase returns a new iterator for the element following the ereased one.
......
...@@ -179,6 +179,8 @@ class UI_ANDROID_EXPORT ViewAndroid { ...@@ -179,6 +179,8 @@ class UI_ANDROID_EXPORT ViewAndroid {
ViewAndroid* parent() const { return parent_; } ViewAndroid* parent() const { return parent_; }
protected: protected:
void RemoveAllChildren(bool attached_to_window);
ViewAndroid* parent_; ViewAndroid* parent_;
private: private:
...@@ -204,7 +206,6 @@ class UI_ANDROID_EXPORT ViewAndroid { ...@@ -204,7 +206,6 @@ class UI_ANDROID_EXPORT ViewAndroid {
bool ScrollTo(float x, float y); bool ScrollTo(float x, float y);
void RemoveChild(ViewAndroid* child); void RemoveChild(ViewAndroid* child);
void RemoveAllChildren();
void OnAttachedToWindow(); void OnAttachedToWindow();
void OnDetachedFromWindow(); void OnDetachedFromWindow();
......
...@@ -152,6 +152,7 @@ ScopedJavaLocalRef<jobject> WindowAndroid::GetJavaObject() { ...@@ -152,6 +152,7 @@ ScopedJavaLocalRef<jobject> WindowAndroid::GetJavaObject() {
WindowAndroid::~WindowAndroid() { WindowAndroid::~WindowAndroid() {
DCHECK(parent_ == nullptr) << "WindowAndroid must be a root view."; DCHECK(parent_ == nullptr) << "WindowAndroid must be a root view.";
DCHECK(!compositor_); DCHECK(!compositor_);
RemoveAllChildren(true);
Java_WindowAndroid_clearNativePointer(AttachCurrentThread(), GetJavaObject()); Java_WindowAndroid_clearNativePointer(AttachCurrentThread(), GetJavaObject());
} }
......
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