Use ScopedObserver in TabDragController

This makes sure any observed Widgets are removed when this class is
destroyed. Found by adding a CHECK in its destructor in
crrev.com/c/2254598.

Bug: 1104876
Change-Id: I58def066b80c314a15d21bf75fd790b16bf54247
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2296981Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788548}

chrome/browser/ui/views/tabs/tab_drag_controller.cc

@@ -50,7 +50,6 @@
 #include "ui/views/event_monitor.h"
 #include "ui/views/view_tracker.h"
 #include "ui/views/widget/root_view.h"
-#include "ui/views/widget/widget.h"
 
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/ash_features.h"
@@ -411,8 +410,7 @@ TabDragController::~TabDragController() {
   if (g_tab_drag_controller == this)
     g_tab_drag_controller = nullptr;
 
-  if (move_loop_widget_)
-    move_loop_widget_->RemoveObserver(this);
+  widget_observer_.RemoveAll();
 
   if (is_dragging_window())
     GetAttachedBrowserWidget()->EndMoveLoop();
@@ -687,6 +685,10 @@ void TabDragController::OnWidgetBoundsChanged(views::Widget* widget,
   Drag(GetCursorScreenPoint());
 }
 
+void TabDragController::OnWidgetDestroyed(views::Widget* widget) {
+  widget_observer_.Remove(widget);
+}
+
 void TabDragController::OnSourceTabStripEmpty() {
   // NULL out source_context_ so that we don't attempt to add back to it (in
   // the case of a revert).
@@ -876,7 +878,8 @@ TabDragController::DragBrowserToNewTabStrip(TabDragContext* target_context,
     // results in a move). That'll cause all sorts of problems.  Reset the
     // observer so we don't get notified and process the event.
 #if defined(OS_CHROMEOS)
-    move_loop_widget_->RemoveObserver(this);
+    if (widget_observer_.IsObserving(move_loop_widget_))
+      widget_observer_.Remove(move_loop_widget_);
     move_loop_widget_ = nullptr;
 #endif  // OS_CHROMEOS
     views::Widget* browser_widget = GetAttachedBrowserWidget();
@@ -1394,7 +1397,7 @@ void TabDragController::RunMoveLoop(const gfx::Vector2d& drag_offset) {
 
   move_loop_widget_ = GetAttachedBrowserWidget();
   DCHECK(move_loop_widget_);
-  move_loop_widget_->AddObserver(this);
+  widget_observer_.Add(move_loop_widget_);
   current_state_ = DragState::kDraggingWindow;
   base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
   if (can_release_capture_) {
@@ -1422,10 +1425,10 @@ void TabDragController::RunMoveLoop(const gfx::Vector2d& drag_offset) {
 
   if (!ref)
     return;
-  if (move_loop_widget_) {
-    move_loop_widget_->RemoveObserver(this);
-    move_loop_widget_ = nullptr;
-  }
+
+  if (widget_observer_.IsObserving(move_loop_widget_))
+    widget_observer_.Remove(move_loop_widget_);
+  move_loop_widget_ = nullptr;
 
   if (current_state_ == DragState::kDraggingWindow) {
     current_state_ = DragState::kWaitingToStop;

chrome/browser/ui/views/tabs/tab_drag_controller.h

@@ -12,6 +12,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/views/tabs/tab_drag_context.h"
@@ -20,6 +21,7 @@
 #include "ui/base/models/list_selection_model.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace ui {
@@ -268,6 +270,7 @@ class TabDragController : public views::WidgetObserver {
   // Overriden from views::WidgetObserver:
   void OnWidgetBoundsChanged(views::Widget* widget,
                              const gfx::Rect& new_bounds) override;
+  void OnWidgetDestroyed(views::Widget* widget) override;
 
   // Forget the source tabstrip. It doesn't exist any more, so it doesn't
   // make sense to insert dragged tabs back into it if the drag is reverted.
@@ -690,6 +693,8 @@ class TabDragController : public views::WidgetObserver {
 
   std::unique_ptr<WindowFinder> window_finder_;
 
+  ScopedObserver<views::Widget, views::WidgetObserver> widget_observer_{this};
+
   base::WeakPtrFactory<TabDragController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(TabDragController);