Re-land this fix from earlier. Since I was making the tab icon view optional, I needed to adjust the accessibility tests so that they use the right index.

Don't initialize the TabIconView if we don't need to. It's expensive to create.

http://crbug/2369


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2298 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 29 insertions, 21 deletions

diff --git a/chrome/browser/views/frame/opaque_non_client_view.cc b/chrome/browser/views/frame/opaque_non_client_view.cc
index 7fdbe04..f10483b 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.cc
+++ b/chrome/browser/views/frame/opaque_non_client_view.cc
@@ -379,7 +379,7 @@ OpaqueNonClientView::OpaqueNonClientView(OpaqueFrame* frame,
       maximize_button_(new ChromeViews::Button),
       restore_button_(new ChromeViews::Button),
       close_button_(new ChromeViews::Button),
-      window_icon_(new TabIconView(this)),
+      window_icon_(NULL),
       frame_(frame),
       browser_view_(browser_view) {
   InitClass();
@@ -452,9 +452,13 @@ OpaqueNonClientView::OpaqueNonClientView(OpaqueFrame* frame,
   close_button_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_CLOSE));
   AddChildView(close_button_);
 
-  window_icon_->set_is_light(true);
-  AddChildView(window_icon_);
-  window_icon_->Update();
+  // Initializing the TabIconView is expensive, so only do it if we need to.
+  if (browser_view_->ShouldShowWindowIcon()) {
+    window_icon_ = new TabIconView(this);
+    window_icon_->set_is_light(true);
+    AddChildView(window_icon_);
+    window_icon_->Update();
+  }
 }
 
 OpaqueNonClientView::~OpaqueNonClientView() {
@@ -479,7 +483,8 @@ gfx::Rect OpaqueNonClientView::GetBoundsForTabStrip(TabStrip* tabstrip) {
 }
 
 void OpaqueNonClientView::UpdateWindowIcon() {
-  window_icon_->Update();
+  if (window_icon_)
+    window_icon_->Update();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -528,8 +533,7 @@ gfx::Size OpaqueNonClientView::CalculateWindowSizeForClientSize(
 }
 
 CPoint OpaqueNonClientView::GetSystemMenuPoint() const {
-  CPoint system_menu_point(window_icon_->GetX(),
-                           window_icon_->GetY() + window_icon_->GetHeight());
+  CPoint system_menu_point(icon_bounds_.x(), icon_bounds_.bottom());
   MapWindowPoints(frame_->GetHWND(), HWND_DESKTOP, &system_menu_point, 1);
   return system_menu_point;
 }
@@ -557,9 +561,11 @@ int OpaqueNonClientView::NonClientHitTest(const gfx::Point& point) {
   minimize_button_->GetBounds(&bounds, APPLY_MIRRORING_TRANSFORMATION);
   if (bounds.PtInRect(test_point))
     return HTMINBUTTON;
-  window_icon_->GetBounds(&bounds, APPLY_MIRRORING_TRANSFORMATION);
-  if (bounds.PtInRect(test_point))
-    return HTSYSMENU;
+  if (window_icon_) {
+    window_icon_->GetBounds(&bounds, APPLY_MIRRORING_TRANSFORMATION);
+    if (bounds.PtInRect(test_point))
+      return HTSYSMENU;
+  }
 
   component = GetHTComponentForFrame(
       point,
@@ -1005,21 +1011,20 @@ void OpaqueNonClientView::LayoutTitleBar() {
   int top_offset = frame_->IsMaximized() ? kWindowTopMarginZoomed : 0;
   ChromeViews::WindowDelegate* d = frame_->window_delegate();
 
-  // Size the window icon, if visible.
-  if (d->ShouldShowWindowIcon()) {
-    window_icon_->SetBounds(kWindowIconLeftOffset, kWindowIconLeftOffset,
-                            kWindowIconSize, kWindowIconSize);
-  } else {
-    // Put the menu in the right place at least even if it is hidden so we
-    // can size the title based on its position.
-    window_icon_->SetBounds(kWindowIconLeftOffset, kWindowIconTopOffset, 0, 0);
-  }
+  // Size the window icon, even if it is hidden so we can size the title based
+  // on its position.
+  bool show_icon = d->ShouldShowWindowIcon();
+  icon_bounds_.SetRect(kWindowIconLeftOffset, kWindowIconLeftOffset,
+                       show_icon ? kWindowIconSize : 0,
+                       show_icon ? kWindowIconSize : 0);
+  if (window_icon_)
+    window_icon_->SetBounds(icon_bounds_.ToRECT());
 
   // Size the title, if visible.
   if (d->ShouldShowWindowTitle()) {
     int spacing = d->ShouldShowWindowIcon() ? kWindowIconTitleSpacing : 0;
     int title_right = minimize_button_->GetX();
-    int icon_right = window_icon_->GetX() + window_icon_->GetWidth();
+    int icon_right = icon_bounds_.right();
     int title_left = icon_right + spacing;
     title_bounds_.SetRect(title_left, kTitleTopOffset + top_offset,
         std::max(0, static_cast<int>(title_right - icon_right)),
diff --git a/chrome/browser/views/frame/opaque_non_client_view.h b/chrome/browser/views/frame/opaque_non_client_view.h
index e684d81..42f9506 100644
--- a/chrome/browser/views/frame/opaque_non_client_view.h
+++ b/chrome/browser/views/frame/opaque_non_client_view.h
@@ -100,6 +100,9 @@ class OpaqueNonClientView : public ChromeViews::NonClientView,
   // The layout rect of the title, if visible.
   gfx::Rect title_bounds_;
 
+  // The layout rect of the window icon.
+  gfx::Rect icon_bounds_;
+
   // The layout rect of the distributor logo, if visible.
   gfx::Rect logo_bounds_;
 
diff --git a/chrome/test/accessibility/constants.h b/chrome/test/accessibility/constants.h
index 7a766f5..db0fb17 100644
--- a/chrome/test/accessibility/constants.h
+++ b/chrome/test/accessibility/constants.h
@@ -42,7 +42,7 @@
 
 // Chrome Client chidren.
 #ifdef NEW_FRAMES
-#define BROWSER_VIEW_ACC_INDEX          (5)
+#define BROWSER_VIEW_ACC_INDEX          (4)
 #define TABSTRIP_ACC_INDEX              (0)
 #define CHROME_MIN_ACC_INDEX            (0)
 #define CHROME_MAX_ACC_INDEX            (1)