Update avatar bubble if profile info changes

chrome/browser/ui/views/avatar_menu_bubble_view.cc

Index: chrome/browser/ui/views/avatar_menu_bubble_view.cc
diff --git a/chrome/browser/ui/views/avatar_menu_bubble_view.cc b/chrome/browser/ui/views/avatar_menu_bubble_view.cc
index 9f1b0816bbb73414ca795ccb99911cccdf942864..d48fe6350cd93f81571739723d91f510a9d40f95 100644
--- a/chrome/browser/ui/views/avatar_menu_bubble_view.cc
+++ b/chrome/browser/ui/views/avatar_menu_bubble_view.cc
@@ -351,14 +351,15 @@ AvatarMenuBubbleView::AvatarMenuBubbleView(
   avatar_menu_model_.reset(new AvatarMenuModel(
       &g_browser_process->profile_manager()->GetProfileInfoCache(),
       this, browser_));
-  // Build the menu for the first time.
-  OnAvatarMenuModelChanged(avatar_menu_model_.get());
 }
 
 AvatarMenuBubbleView::~AvatarMenuBubbleView() {
 }
 
 gfx::Size AvatarMenuBubbleView::GetPreferredSize() {
+  if (!add_profile_link_)

[Peter Kasting, 2011/12/13 02:25:11]

Is this really necessary?  Normally it's safe to assume the object is in a
hierarchy when this function is reached.

[sail, 2011/12/13 02:31:40]

This is currently necessary. I think it's called by the bubble delegate code
(didn't get a chance to check).

+    return gfx::Size();
+
   int max_width = 0;
   int total_height = 0;
   for (size_t i = 0; i < item_views_.size(); ++i) {
@@ -431,6 +432,16 @@ bool AvatarMenuBubbleView::AcceleratorPressed(
   return true;
 }
 
+void AvatarMenuBubbleView::ViewHierarchyChanged(bool is_add,
+                                                views::View* parent,
+                                                views::View* child) {
+  views::BubbleDelegateView::ViewHierarchyChanged(is_add, parent, child);
+
+  // Build the menu for the first time.

[Peter Kasting, 2011/12/13 02:25:11]

Nit: I think normally we do class-specific stuff before calling the superclass
version, but I'm not 100% sure.

[sail, 2011/12/13 02:31:40]

Done.

+  if (!add_profile_link_ && is_add && child == this)
+    OnAvatarMenuModelChanged(avatar_menu_model_.get());
+}
+
 void AvatarMenuBubbleView::ButtonPressed(views::Button* sender,
                                          const views::Event& event) {
   for (size_t i = 0; i < item_views_.size(); ++i) {
@@ -497,5 +508,7 @@ void AvatarMenuBubbleView::OnAvatarMenuModelChanged(
   add_profile_link_->SetEnabledColor(SkColorSetRGB(0xe3, 0xed, 0xf6));
   AddChildView(add_profile_link_);
 
-  PreferredSizeChanged();
+  // If the bubble has already been shown then resize and reposition the bubble.

[Peter Kasting, 2011/12/13 02:25:11]

It still seems strange to me that we need this.  I think you should get Scott to
comment here.  Perhaps something in the bubble should be causing this to happen
that currently isn't.  Or perhaps we do need to call something here, but only
InvalidateLayout() or something.  I'm not sure.

[sail, 2011/12/13 02:31:40]

Adding Scott and msw to the review. If this requires changes to the
views::Bubble code then I'd prefer to do this separately since this CL needs to
get merged into M17.

[msw, 2011/12/13 03:20:23]

Doing a synchronous Layout and then calling SizeToContents (to resize and
potentially move the bubble widget[s]) seems reasonable for now.

[sky, 2011/12/13 05:18:46]

We could make whatever the content view is for the bubble override
ChildPreferredSizeChanged and do the resize/layout then. But I'm not sure that
we want that for all bubbles.

[msw, 2011/12/13 18:17:47]

AvatarMenuBubbleView is the content view (a BubbleDelegateView).
So maybe you could override ChildPreferredSizeChanged here for now.

Maybe add a TODO to consider the same behavior for other bubbles.
Changing all bubbles seems okay for a followup change that won't be merged.

+  Layout();
+  SizeToContents();
 }