splitview: Speculative fix of crash in split view.

In current implementation, if a snapped window with a transient child
window gets unsnapped from splitview, it was not removed from
SplitViewDivider's observed transient window list, which may cause crash
later.

Bug: 1140115
Change-Id: I7f3b6b76a3e4a3f6b8605a19e4671b4c11fd29e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2553151Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831903}

ash/wm/splitview/split_view_controller_unittest.cc

@@ -2780,6 +2780,36 @@ TEST_P(SplitViewControllerTest, AutoSnapFromMinimizedState) {
   EndSplitView();
 }
 
+// Test that if the transient parent window is no longer snapped in split view,
+// split view divider should no longer observe the transient child window.
+TEST_P(SplitViewControllerTest, DoNotObserveTransientIfNotInSplitview) {
+  // Create two normal window.
+  const gfx::Rect bounds(0, 0, 400, 400);
+  std::unique_ptr<aura::Window> window1(CreateWindow(bounds));
+  std::unique_ptr<aura::Window> window2(CreateWindow(bounds));
+
+  // Add another two windows with one being a bubble transient child of the
+  // other.
+  std::unique_ptr<views::Widget> widget(CreateTestWidget());
+  aura::Window* parent = widget->GetNativeWindow();
+  parent->SetProperty(aura::client::kResizeBehaviorKey,
+                      aura::client::kResizeBehaviorCanResize |
+                          aura::client::kResizeBehaviorCanMaximize);
+  views::Widget* bubble_widget = views::BubbleDialogDelegateView::CreateBubble(
+      new TestBubbleDialogDelegateView(widget->GetContentsView()));
+  aura::Window* bubble_transient = bubble_widget->GetNativeWindow();
+  EXPECT_TRUE(::wm::HasTransientAncestor(bubble_transient, parent));
+
+  ToggleOverview();
+  split_view_controller()->SnapWindow(window1.get(), SplitViewController::LEFT);
+  split_view_controller()->SnapWindow(parent, SplitViewController::RIGHT);
+  EXPECT_TRUE(bubble_transient->HasObserver(split_view_divider()));
+
+  split_view_controller()->SnapWindow(window2.get(),
+                                      SplitViewController::RIGHT);
+  EXPECT_FALSE(bubble_transient->HasObserver(split_view_divider()));
+}
+
 // Test the tab-dragging related functionalities in tablet mode. Tab(s) can be
 // dragged out of a window and then put in split view mode or merge into another
 // window.

ash/wm/splitview/split_view_divider.cc

@@ -332,8 +332,12 @@ void SplitViewDivider::SetAlwaysOnTop(bool on_top) {
 void SplitViewDivider::AddObservedWindow(aura::Window* window) {
   if (!base::Contains(observed_windows_, window)) {
     window->AddObserver(this);
-    ::wm::TransientWindowManager::GetOrCreate(window)->AddObserver(this);
     observed_windows_.push_back(window);
+    ::wm::TransientWindowManager* transient_manager =
+        ::wm::TransientWindowManager::GetOrCreate(window);
+    transient_manager->AddObserver(this);
+    for (auto* transient_window : transient_manager->transient_children())
+      StartObservingTransientChild(transient_window);
   }
 }
 
@@ -342,8 +346,12 @@ void SplitViewDivider::RemoveObservedWindow(aura::Window* window) {
       std::find(observed_windows_.begin(), observed_windows_.end(), window);
   if (iter != observed_windows_.end()) {
     window->RemoveObserver(this);
-    ::wm::TransientWindowManager::GetOrCreate(window)->RemoveObserver(this);
     observed_windows_.erase(iter);
+    ::wm::TransientWindowManager* transient_manager =
+        ::wm::TransientWindowManager::GetOrCreate(window);
+    transient_manager->RemoveObserver(this);
+    for (auto* transient_window : transient_manager->transient_children())
+      StopObservingTransientChild(transient_window);
   }
 }
 
@@ -410,22 +418,12 @@ void SplitViewDivider::OnWindowActivated(ActivationReason reason,
 
 void SplitViewDivider::OnTransientChildAdded(aura::Window* window,
                                              aura::Window* transient) {
-  // For now, we only care about dialog bubbles type transient child. We may
-  // observe other types transient child window as well if need arises in the
-  // future.
-  views::Widget* widget = views::Widget::GetWidgetForNativeWindow(transient);
-  if (!widget || !widget->widget_delegate()->AsBubbleDialogDelegate())
-    return;
-
-  // At this moment, the transient window may not have the valid bounds yet.
-  // Start observe the transient window.
-  transient_windows_observer_.Add(transient);
+  StartObservingTransientChild(transient);
 }
 
 void SplitViewDivider::OnTransientChildRemoved(aura::Window* window,
                                                aura::Window* transient) {
-  if (transient_windows_observer_.IsObserving(transient))
-    transient_windows_observer_.Remove(transient);
+  StopObservingTransientChild(transient);
 }
 
 void SplitViewDivider::CreateDividerWidget(SplitViewController* controller) {
@@ -448,4 +446,22 @@ void SplitViewDivider::CreateDividerWidget(SplitViewController* controller) {
   divider_widget_->Show();
 }
 
+void SplitViewDivider::StartObservingTransientChild(aura::Window* transient) {
+  // For now, we only care about dialog bubbles type transient child. We may
+  // observe other types transient child window as well if need arises in the
+  // future.
+  views::Widget* widget = views::Widget::GetWidgetForNativeWindow(transient);
+  if (!widget || !widget->widget_delegate()->AsBubbleDialogDelegate())
+    return;
+
+  // At this moment, the transient window may not have the valid bounds yet.
+  // Start observe the transient window.
+  transient_windows_observer_.Add(transient);
+}
+
+void SplitViewDivider::StopObservingTransientChild(aura::Window* transient) {
+  if (transient_windows_observer_.IsObserving(transient))
+    transient_windows_observer_.Remove(transient);
+}
+
 }  // namespace ash

ash/wm/splitview/split_view_divider.h

@@ -96,6 +96,9 @@ class ASH_EXPORT SplitViewDivider : public aura::WindowObserver,
  private:
   void CreateDividerWidget(SplitViewController* controller);
 
+  void StartObservingTransientChild(aura::Window* transient);
+  void StopObservingTransientChild(aura::Window* transient);
+
   SplitViewController* controller_;
 
   // The window targeter that is installed on the always on top container window