Fixing crash Report - Magic Signature: views::View::ConvertPointToScreen

ui/views/bubble/bubble_delegate.cc

Index: ui/views/bubble/bubble_delegate.cc
diff --git a/ui/views/bubble/bubble_delegate.cc b/ui/views/bubble/bubble_delegate.cc
index af08994d487f5905b3471e806cfb899664de10e0..8d89ea16448385e718efa11a3fe44a7ce08ce0ce 100644
--- a/ui/views/bubble/bubble_delegate.cc
+++ b/ui/views/bubble/bubble_delegate.cc
@@ -170,10 +170,7 @@ BubbleDelegateView::BubbleDelegateView(
 }
 
 BubbleDelegateView::~BubbleDelegateView() {
-  if (anchor_widget() != NULL)
-    anchor_widget()->RemoveObserver(this);
-  anchor_widget_ = NULL;
-  anchor_view_ = NULL;
+  DetachFromAnchor();
 }
 
 // static
@@ -231,11 +228,8 @@ NonClientFrameView* BubbleDelegateView::CreateNonClientFrameView(
 }
 
 void BubbleDelegateView::OnWidgetDestroying(Widget* widget) {
-  if (anchor_widget() == widget) {
-    anchor_widget_->RemoveObserver(this);
-    anchor_view_ = NULL;
-    anchor_widget_ = NULL;
-  }
+  if (anchor_widget() == widget)
+    DetachFromAnchor();
 }
 
 void BubbleDelegateView::OnWidgetVisibilityChanging(Widget* widget,
@@ -257,8 +251,12 @@ void BubbleDelegateView::OnWidgetVisibilityChanged(Widget* widget,
 
 void BubbleDelegateView::OnWidgetActivationChanged(Widget* widget,
                                                    bool active) {
-  if (close_on_deactivate() && widget == GetWidget() && !active)
+  if (close_on_deactivate() && widget == GetWidget() && !active) {
+    // At this point the original anchor view could be gone and we should make
+    // sure that we do not query it anymore for screen locations.
+    DetachFromAnchor();
     GetWidget()->Close();
+  }
 }
 
 void BubbleDelegateView::OnWidgetBoundsChanged(Widget* widget,
@@ -280,10 +278,14 @@ void BubbleDelegateView::StartFade(bool fade_in) {
   // Use AURA's window layer animation instead of fading. This ensures that
   // hosts which rely on the layer animation callbacks to close the window
   // work correctly.
-  if (fade_in)
+  if (fade_in) {
     GetWidget()->Show();
-  else
+  } else {
+    // At this point the original anchor view could be gone and we should make
+    // sure that we do not query it anymore for screen locations.
+    DetachFromAnchor();
     GetWidget()->Close();
+  }
 #else
   fade_animation_.reset(new gfx::SlideAnimation(this));
   fade_animation_->SetSlideDuration(GetFadeDuration());
@@ -298,6 +300,9 @@ void BubbleDelegateView::StartFade(bool fade_in) {
   } else {
     original_opacity_ = 255;
     fade_animation_->Hide();
+    // At this point the original anchor view could be gone and we should make
+    // sure that we do not query it anymore for screen locations.
+    DetachFromAnchor();
   }
 #endif
 }
@@ -437,4 +442,12 @@ void BubbleDelegateView::HandleVisibilityChanged(Widget* widget,
   }
 }
 
+void BubbleDelegateView::DetachFromAnchor() {
+  if (!anchor_widget_)
+    return

[msw, 2013/09/26 22:17:19]

This statement needs a semicolon. Have you tested this patch?

[Mr4D (OOO till 08-26), 2013/09/27 17:22:50]

I told you to send a patch within 10 minutes so that you have a better
understanding what I am talking about (as you have pointed out that was a good
idea ;) ).

+  anchor_widget_->RemoveObserver(this);

[msw, 2013/09/26 22:17:19]

Do this in an if (anchor_widget) block, and set |anchor_view| and
|anchor_widget| to NULL regardless instead of early returning if |anchor_widget|
is NULL.

[Mr4D (OOO till 08-26), 2013/09/27 17:22:50]

Done.

+  anchor_view_ = NULL;
+  anchor_widget_ = NULL;
+}
+
 }  // namespace views