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

ui/views/bubble/bubble_delegate.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.
 
 #include "ui/views/bubble/bubble_delegate.h"
 
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/rect.h"
 #include "ui/native_theme/native_theme.h"
@@ skipping 152 matching lines @@
       use_focusless_(false),
       accept_events_(true),
       border_accepts_events_(true),
       adjust_if_offscreen_(true),
       parent_window_(NULL) {
   AddAccelerator(ui::Accelerator(ui::VKEY_ESCAPE, ui::EF_NONE));
   UpdateColorsFromTheme(GetNativeTheme());
 }
 
 BubbleDelegateView::~BubbleDelegateView() {
-  if (anchor_widget() != NULL)
-    anchor_widget()->RemoveObserver(this);
-  anchor_widget_ = NULL;
-  anchor_view_ = NULL;
+  DetachFromAnchor();
 }
 
 // static
 Widget* BubbleDelegateView::CreateBubble(BubbleDelegateView* bubble_delegate) {
   bubble_delegate->Init();
   // Determine the anchor widget from the anchor view at bubble creation time.
   bubble_delegate->anchor_widget_ = bubble_delegate->anchor_view() ?
       bubble_delegate->anchor_view()->GetWidget() : NULL;
   if (bubble_delegate->anchor_widget())
     bubble_delegate->anchor_widget()->AddObserver(bubble_delegate);
@@ skipping 37 matching lines @@
 NonClientFrameView* BubbleDelegateView::CreateNonClientFrameView(
     Widget* widget) {
   BubbleFrameView* frame = new BubbleFrameView(margins());
   const BubbleBorder::Arrow adjusted_arrow = base::i18n::IsRTL() ?
       BubbleBorder::horizontal_mirror(arrow()) : arrow();
   frame->SetBubbleBorder(new BubbleBorder(adjusted_arrow, shadow(), color()));
   return frame;
 }
 
 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,
                                                     bool visible) {
 #if defined(OS_WIN)
   // On Windows we need to handle this before the bubble is visible or hidden.
   // Please see the comment on the OnWidgetVisibilityChanging function. On
   // other platforms it is fine to handle it after the bubble is shown/hidden.
   HandleVisibilityChanged(widget, visible);
 #endif
 }
 
 void BubbleDelegateView::OnWidgetVisibilityChanged(Widget* widget,
                                                    bool visible) {
 #if !defined(OS_WIN)
   HandleVisibilityChanged(widget, visible);
 #endif
 }
 
 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,
                                                const gfx::Rect& new_bounds) {
   if (move_with_anchor() && anchor_widget() == widget)
     SizeToContents();
 }
 
 gfx::Rect BubbleDelegateView::GetAnchorRect() {
   if (!anchor_view())
     return anchor_rect_;
   gfx::Rect anchor_bounds = anchor_view()->GetBoundsInScreen();
   anchor_bounds.Inset(anchor_view_insets_);
   return anchor_bounds;
 }
 
 void BubbleDelegateView::StartFade(bool fade_in) {
 #if defined(USE_AURA)
   // 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());
   fade_animation_->Reset(fade_in ? 0.0 : 1.0);
   if (fade_in) {
     original_opacity_ = 0;
     if (border_widget_)
       border_widget_->SetOpacity(original_opacity_);
     GetWidget()->SetOpacity(original_opacity_);
     GetWidget()->Show();
     fade_animation_->Show();
   } 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
 }
 
 void BubbleDelegateView::ResetFade() {
   fade_animation_.reset();
   if (border_widget_)
     border_widget_->SetOpacity(original_opacity_);
   GetWidget()->SetOpacity(original_opacity_);
 }
@@ skipping 119 matching lines @@
     if (border_widget_)
       border_widget_->ShowInactive();
     if (anchor_widget() && anchor_widget()->GetTopLevelWidget())
       anchor_widget()->GetTopLevelWidget()->DisableInactiveRendering();
   } else {
     if (border_widget_)
       border_widget_->Hide();
   }
 }
 
+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