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

ui/views/bubble/bubble_delegate.h

Index: ui/views/bubble/bubble_delegate.h
diff --git a/ui/views/bubble/bubble_delegate.h b/ui/views/bubble/bubble_delegate.h
index 4a0bb30ec0160b49e25167769261f49be2894b10..285185bef31bcf8c5a518a8067f936b23bd656f8 100644
--- a/ui/views/bubble/bubble_delegate.h
+++ b/ui/views/bubble/bubble_delegate.h
@@ -59,11 +59,14 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   bool close_on_deactivate() const { return close_on_deactivate_; }
   void set_close_on_deactivate(bool close) { close_on_deactivate_ = close; }
 
-  View* anchor_view() const { return anchor_view_; }
+  View* AnchorView() const;

[msw, 2013/09/27 18:20:31]

nit: GetAnchorView.

[Mr4D (OOO till 08-26), 2013/09/27 20:29:35]

Done.

   Widget* anchor_widget() const { return anchor_widget_; }
+  // Even though AnchorView might return NULL, the bubble might have an

[msw, 2013/09/27 18:20:31]

Why is this important? I think it can be removed.

[Mr4D (OOO till 08-26), 2013/09/27 20:29:35]

Because the bubble might have been created with an anchor_view, has still an
anchor_widget and the anchor_rect from its last use but the view itself is gone.

(See immersive_mode_controller as an example)

[msw, 2013/09/27 21:21:52]

I understand how that works, but why would anyone care if there was an anchor
view but there is not one any more? If that code should care about the existence
of a parent window or an anchor widget, it should use those instead.

+  // anchor_view attached to it upon creation.
+  bool has_anchor_view() const { return has_anchor_view_; }
 
-  // The anchor rect is used in the absence of an anchor view.
-  const gfx::Rect& anchor_rect() const { return anchor_rect_; }
+  // The anchor rect is used in the absence of an assigned anchor view.
+  const gfx::Rect& AnchorRect() const;

[msw, 2013/09/27 18:20:31]

nit: GetAnchorRect.

[Mr4D (OOO till 08-26), 2013/09/27 20:29:35]

Done.

 
   BubbleBorder::Arrow arrow() const { return arrow_; }
   void set_arrow(BubbleBorder::Arrow arrow) { arrow_ = arrow; }
@@ -141,9 +144,12 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   // Perform view initialization on the contents for bubble sizing.
   virtual void Init();
 
-  // Set the anchor view or rect; set these before CreateBubble or Show.
-  void set_anchor_view(View* anchor_view) { anchor_view_ = anchor_view; }
-  void set_anchor_rect(const gfx::Rect& rect) { anchor_rect_ = rect; }
+  // Set the anchor view or rect; set these before CreateBubble or Show. Note
+  // that if a valid view gets passed, the anchor rect will get ignored. If the
+  // view gets deleted, but no new view gets set, the last known anchor postion
+  // will get returned.
+  void SetAnchorView(View* anchor_view);
+  void SetAnchorRect(const gfx::Rect& rect);
 
   // Resize and potentially move the bubble to fit the content's preferred size.
   void SizeToContents();
@@ -167,6 +173,9 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   // Handles widget visibility changes.
   void HandleVisibilityChanged(Widget* widget, bool visible);
 
+  // Detach the bubble from the anchor (has no effect if there is no anchor).
+  void DetachFromAnchor();
+
   // Fade animation for bubble.
   scoped_ptr<gfx::SlideAnimation> fade_animation_;
 
@@ -174,8 +183,16 @@ class VIEWS_EXPORT BubbleDelegateView : public WidgetDelegateView,
   bool close_on_esc_;
   bool close_on_deactivate_;
 
-  // The view and widget to which this bubble is anchored.
-  View* anchor_view_;
+  // The view and widget to which this bubble is anchored. Since an anchor view
+  // can be deleted without notice, we store it in the ViewStorage and retrieve
+  // it from there. It will make sure that the view is still valid.
+  // To avoid the bubble to jump upon early anchor destruction, we remember the

[msw, 2013/09/27 18:20:31]

nit: I think the complex behavior and the rest of this comment "To avoid ...
cleared." is unnecessary; please remove that part of this comment and see my
other comments about simplifying the change.

[Mr4D (OOO till 08-26), 2013/09/27 20:29:35]

Done.

+  // last known anchor bounds in |last_known_anchor_view_rect_|. The originally
+  // set |anchor_rect_| will only be passed back when the anchor_view got
+  // explicitly cleared.
+  bool has_anchor_view_;
+  gfx::Rect last_known_anchor_view_rect_;
+  int anchor_view_storage_id_;
   Widget* anchor_widget_;
 
   // The anchor rect used in the absence of an anchor view.