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

chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc

Index: chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
diff --git a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
index 5aa2b0f83de7170ac837d2ff9ec5701d60e31c06..8f061776edc3fb9e79b698a6c4cc1e363bbe59c1 100644
--- a/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
+++ b/chrome/browser/ui/views/frame/immersive_mode_controller_ash.cc
@@ -586,8 +586,8 @@ void ImmersiveModeControllerAsh::OnAddTransientChild(aura::Window* window,
                                                      aura::Window* transient) {
   views::BubbleDelegateView* bubble_delegate = AsBubbleDelegate(transient);
   if (bubble_delegate &&
-      bubble_delegate->anchor_view() &&
-      top_container_->Contains(bubble_delegate->anchor_view())) {
+      bubble_delegate->GetAnchorView() &&
+      top_container_->Contains(bubble_delegate->GetAnchorView())) {
     // Observe the aura::Window because the BubbleDelegateView may not be
     // parented to the widget's root view yet so |bubble_delegate->GetWidget()|
     // may still return NULL.
@@ -837,10 +837,13 @@ void ImmersiveModeControllerAsh::UpdateFocusRevealedLock() {
         native_window_->GetRootWindow())->GetActiveWindow();
     views::BubbleDelegateView* bubble_delegate =
         AsBubbleDelegate(active_window);
-    if (bubble_delegate && bubble_delegate->anchor_view()) {
+    if (bubble_delegate && bubble_delegate->has_anchor_view()) {

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

It seems like this code is avoiding a lock here for "simplicity" by making
BubbleManager obtain its own lock. Wouldn't it be far simpler to remove the
condition here and always grab a lock; then either allow this redundant lock, or
remove the special cased BubbleManager code to obtain a lock?

Either way, I don't like that this code needs to know if the bubble was
previously attached to a view, regardless of it's current condition. Perhaps
those bubbles should set an anchor_widget and this code should check for an
anchor_widget if that's what really matters.

TL;DR: Remove this check and has_anchor_view(); consider other cleanup.

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

Okay - I see you really really really don't want to have a new function in the
class. The luck is with us and with setting the anchor view the anchor widget
will be set as well. So I can only check for the widget here. Done.

BUT WAIT! There was actually a bug since a long time! The function SetAnchorView
was never setting the widget and thus not the observer. As such it is
conceivable that this might have caused problems. Fixed. 

       // BubbleManager will already have locked the top-of-window views if the
       // bubble is anchored to a child of |top_container_|. Don't acquire
       // |focus_revealed_lock_| here for the sake of simplicity.
+      // Note that even though the anchor view might already be gone, the owning
+      // anchor widget does still exist and as such it should be handled as if
+      // the view is still there.
     } else {
       // The currently active window is not |native_window_| and it is not a
       // bubble with an anchor view. The top-of-window views should be revealed
@@ -1170,8 +1173,8 @@ void ImmersiveModeControllerAsh::RecreateBubbleManager() {
     views::BubbleDelegateView* bubble_delegate =
         AsBubbleDelegate(transient_child);
     if (bubble_delegate &&
-        bubble_delegate->anchor_view() &&
-        top_container_->Contains(bubble_delegate->anchor_view())) {
+        bubble_delegate->GetAnchorView() &&
+        top_container_->Contains(bubble_delegate->GetAnchorView())) {
       bubble_manager_->StartObserving(transient_child);
     }
   }