Update avatar bubble if profile info changes

chrome/browser/ui/views/avatar_menu_bubble_view.cc

 // Copyright (c) 2011 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 "chrome/browser/ui/views/avatar_menu_bubble_view.h"
 
 #include <algorithm>
 
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 333 matching lines @@
     views::BubbleBorder::ArrowLocation arrow_location,
     const gfx::Rect& anchor_rect,
     Browser* browser)
     : BubbleDelegateView(anchor_view, arrow_location),
       add_profile_link_(NULL),
       anchor_rect_(anchor_rect),
       browser_(browser) {
   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) {
     gfx::Size size = item_views_[i]->GetPreferredSize();
     max_width = std::max(max_width, size.width());
     total_height += size.height() + kItemMarginY;
   }
 
   total_height += kSeparatorPaddingY * 2 +
                   separator_->GetPreferredSize().height();
@@ skipping 52 matching lines @@
   // Moved the focus up or down by 1.
   if (accelerator.key_code() == ui::VKEY_DOWN)
     focus_index = (focus_index + 1) % item_views_.size();
   else
     focus_index = ((focus_index > 0) ? focus_index : item_views_.size()) - 1;
   item_views_[focus_index]->RequestFocus();
 
   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) {
     ProfileItemView* item_view = static_cast<ProfileItemView*>(item_views_[i]);
     if (sender == item_view) {
       // Clicking on the active profile shouldn't do anything.
       if (!item_view->item().active)
         avatar_menu_model_->SwitchToProfile(i);
       break;
     }
@@ skipping 46 matching lines @@
   AddChildView(separator_);
 
   add_profile_link_ = new views::Link(
       l10n_util::GetStringUTF16(IDS_PROFILES_CREATE_NEW_PROFILE_LINK));
   add_profile_link_->set_listener(this);
   add_profile_link_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
   add_profile_link_->SetBackgroundColor(color());
   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();
 }