Minor fixes for WidgetRemovalsObserver::OnWillRemoveView

ui/views/view.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #define _USE_MATH_DEFINES // For VC++ to get M_PI. This has to be first.
 
 #include "ui/views/view.h"
 
 #include <algorithm>
 #include <cmath>
@@ skipping 1785 matching lines @@
       if (prev_focusable)
         prev_focusable->next_focusable_view_ = next_focusable;
       if (next_focusable)
         next_focusable->previous_focusable_view_ = prev_focusable;
     }
 
     if (GetWidget()) {
       UnregisterChildrenForVisibleBoundsNotification(view);
       if (view->visible())
         view->SchedulePaint();
-      GetWidget()->NotifyWillRemoveView(view);
     }
 
     view->PropagateRemoveNotifications(this, new_parent);
     view->parent_ = NULL;
     view->UpdateLayerVisibility();
 
     if (delete_removed_view && !view->owned_by_client_)
       view_to_be_deleted.reset(view);
 
     children_.erase(i);
   }
 
   if (update_tool_tip)
     UpdateTooltip();
 
   if (layout_manager_.get())
     layout_manager_->ViewRemoved(this, view);
 }
 
 void View::PropagateRemoveNotifications(View* old_parent, View* new_parent) {
+  if (GetWidget())
+    GetWidget()->NotifyWillRemoveView(this);

[sky, 2016/01/25 21:22:14]

Doing this here means we have to constantly to determine the widget, which isn't
going to change. Also, I think OnWillRemoveView should only be sent if removed,
and not added to a different view in the same widget.

[dmazzoni, 2016/01/25 21:44:27]

OK, moved to a separate helper that takes the widget as an argument.
Good point, though it seems like it should be called if it gets reparented to a
new widget, right?

I added a test for those cases.

+
   for (int i = 0, count = child_count(); i < count; ++i)
     child_at(i)->PropagateRemoveNotifications(old_parent, new_parent);
 
   ViewHierarchyChangedDetails details(false, old_parent, this, new_parent);
   for (View* v = this; v; v = v->parent_)
     v->ViewHierarchyChangedImpl(true, details);
 }
 
 void View::PropagateAddNotifications(
     const ViewHierarchyChangedDetails& details) {
@@ skipping 567 matching lines @@
   // Message the RootView to do the drag and drop. That way if we're removed
   // the RootView can detect it and avoid calling us back.
   gfx::Point widget_location(event.location());
   ConvertPointToWidget(this, &widget_location);
   widget->RunShellDrag(this, data, widget_location, drag_operations, source);
   // WARNING: we may have been deleted.
   return true;
 }
 
 }  // namespace views