TabStrip Cleanup

This patch is a cleanup of existing Tab code, as per the review comments of 5572007 Patch Set 2.  Some of the comments from that review apply to pre-existing code, and so I separated out those changes into their own CL (this one).

BUG=None
TEST=None

Review URL: http://codereview.chromium.org/6080002
Patch from Chad Faragher <wyck@chromium.org>.

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

3 files changed, 51 insertions, 53 deletions

diff --git a/chrome/browser/ui/views/tabs/tab.cc b/chrome/browser/ui/views/tabs/tab.cc
index 98e19cc..e24478e 100644
--- a/chrome/browser/ui/views/tabs/tab.cc
+++ b/chrome/browser/ui/views/tabs/tab.cc
@@ -53,9 +53,9 @@ static const int kMiniTabRendererAsNormalTabWidth =
 // How opaque to make the hover state (out of 1).
 static const double kHoverOpacity = 0.33;
 
-Tab::TabImage Tab::tab_alpha = {0};
-Tab::TabImage Tab::tab_active = {0};
-Tab::TabImage Tab::tab_inactive = {0};
+Tab::TabImage Tab::tab_alpha_ = {0};
+Tab::TabImage Tab::tab_active_ = {0};
+Tab::TabImage Tab::tab_inactive_ = {0};
 
 // Durations for the various parts of the mini tab title animation.
 static const int kMiniTitleChangeAnimationDuration1MS = 1600;
@@ -82,19 +82,6 @@ static const SkScalar kTabCapWidth = 15;
 static const SkScalar kTabTopCurveWidth = 4;
 static const SkScalar kTabBottomCurveWidth = 3;
 
-namespace {
-
-void InitTabResources() {
-  static bool initialized = false;
-  if (initialized)
-    return;
-
-  initialized = true;
-  Tab::LoadTabImages();
-}
-
-}  // namespace
-
 // static
 const char Tab::kViewClassName[] = "browser/tabs/Tab";
 
@@ -140,10 +127,6 @@ void Tab::StopMiniTabTitleAnimation() {
     mini_title_animation_->Stop();
 }
 
-void Tab::PaintIcon(gfx::Canvas* canvas) {
-  BaseTab::PaintIcon(canvas, favicon_bounds_.x(), favicon_bounds_.y());
-}
-
 // static
 gfx::Size Tab::GetMinimumUnselectedSize() {
   InitTabResources();
@@ -152,7 +135,7 @@ gfx::Size Tab::GetMinimumUnselectedSize() {
   minimum_size.set_width(kLeftPadding + kRightPadding);
   // Since we use bitmap images, the real minimum height of the image is
   // defined most accurately by the height of the end cap images.
-  minimum_size.set_height(tab_active.image_l->height());
+  minimum_size.set_height(tab_active_.image_l->height());
   return minimum_size;
 }
 
@@ -321,7 +304,7 @@ void Tab::Layout() {
 }
 
 void Tab::OnThemeChanged() {
-  Tab::LoadTabImages();
+  LoadTabImages();
 }
 
 bool Tab::HasHitTestMask() const {
@@ -370,6 +353,10 @@ void Tab::OnMouseMoved(const views::MouseEvent& e) {
 ////////////////////////////////////////////////////////////////////////////////
 // Tab, private
 
+void Tab::PaintIcon(gfx::Canvas* canvas) {
+  BaseTab::PaintIcon(canvas, favicon_bounds_.x(), favicon_bounds_.y());
+}
+
 void Tab::PaintTabBackground(gfx::Canvas* canvas) {
   if (IsSelected()) {
     PaintActiveTabBackground(canvas);
@@ -465,9 +452,9 @@ void Tab::PaintInactiveTabBackground(gfx::Canvas* canvas) {
 
   SkBitmap* tab_bg = GetThemeProvider()->GetBitmapNamed(tab_id);
 
-  TabImage* tab_image = &tab_active;
-  TabImage* tab_inactive_image = &tab_inactive;
-  TabImage* alpha = &tab_alpha;
+  TabImage* tab_image = &tab_active_;
+  TabImage* tab_inactive_image = &tab_inactive_;
+  TabImage* alpha = &tab_alpha_;
 
   // If the theme is providing a custom background image, then its top edge
   // should be at the top of the tab. Otherwise, we assume that the background
@@ -539,13 +526,12 @@ void Tab::PaintActiveTabBackground(gfx::Canvas* canvas) {
   int offset = GetX(views::View::APPLY_MIRRORING_TRANSFORMATION) +
       background_offset_.x();
   ThemeProvider* tp = GetThemeProvider();
-  if (!tp)
-    NOTREACHED() << "Unable to get theme provider";
+  DCHECK(tp) << "Unable to get theme provider";
 
   SkBitmap* tab_bg = GetThemeProvider()->GetBitmapNamed(IDR_THEME_TOOLBAR);
 
-  TabImage* tab_image = &tab_active;
-  TabImage* alpha = &tab_alpha;
+  TabImage* tab_image = &tab_active_;
+  TabImage* alpha = &tab_alpha_;
 
   // Draw left edge.
   SkBitmap tab_l = SkBitmapOperations::CreateTiledBitmap(
@@ -655,22 +641,34 @@ double Tab::GetThrobValue() {
 // Tab, private:
 
 // static
+void Tab::InitTabResources() {
+  static bool initialized = false;
+  if (initialized)
+    return;
+
+  initialized = true;
+
+  // Load the tab images once now, and maybe again later if the theme changes.
+  LoadTabImages();
+}
+
+// static
 void Tab::LoadTabImages() {
   // We're not letting people override tab images just yet.
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
 
-  tab_alpha.image_l = rb.GetBitmapNamed(IDR_TAB_ALPHA_LEFT);
-  tab_alpha.image_r = rb.GetBitmapNamed(IDR_TAB_ALPHA_RIGHT);
+  tab_alpha_.image_l = rb.GetBitmapNamed(IDR_TAB_ALPHA_LEFT);
+  tab_alpha_.image_r = rb.GetBitmapNamed(IDR_TAB_ALPHA_RIGHT);
 
-  tab_active.image_l = rb.GetBitmapNamed(IDR_TAB_ACTIVE_LEFT);
-  tab_active.image_c = rb.GetBitmapNamed(IDR_TAB_ACTIVE_CENTER);
-  tab_active.image_r = rb.GetBitmapNamed(IDR_TAB_ACTIVE_RIGHT);
-  tab_active.l_width = tab_active.image_l->width();
-  tab_active.r_width = tab_active.image_r->width();
+  tab_active_.image_l = rb.GetBitmapNamed(IDR_TAB_ACTIVE_LEFT);
+  tab_active_.image_c = rb.GetBitmapNamed(IDR_TAB_ACTIVE_CENTER);
+  tab_active_.image_r = rb.GetBitmapNamed(IDR_TAB_ACTIVE_RIGHT);
+  tab_active_.l_width = tab_active_.image_l->width();
+  tab_active_.r_width = tab_active_.image_r->width();
 
-  tab_inactive.image_l = rb.GetBitmapNamed(IDR_TAB_INACTIVE_LEFT);
-  tab_inactive.image_c = rb.GetBitmapNamed(IDR_TAB_INACTIVE_CENTER);
-  tab_inactive.image_r = rb.GetBitmapNamed(IDR_TAB_INACTIVE_RIGHT);
-  tab_inactive.l_width = tab_inactive.image_l->width();
-  tab_inactive.r_width = tab_inactive.image_r->width();
+  tab_inactive_.image_l = rb.GetBitmapNamed(IDR_TAB_INACTIVE_LEFT);
+  tab_inactive_.image_c = rb.GetBitmapNamed(IDR_TAB_INACTIVE_CENTER);
+  tab_inactive_.image_r = rb.GetBitmapNamed(IDR_TAB_INACTIVE_RIGHT);
+  tab_inactive_.l_width = tab_inactive_.image_l->width();
+  tab_inactive_.r_width = tab_inactive_.image_r->width();
 }
diff --git a/chrome/browser/ui/views/tabs/tab.h b/chrome/browser/ui/views/tabs/tab.h
index 5665fd1..1e9dbfa 100644
--- a/chrome/browser/ui/views/tabs/tab.h
+++ b/chrome/browser/ui/views/tabs/tab.h
@@ -38,14 +38,10 @@ class Tab : public BaseTab {
 
   // Set the background offset used to match the image in the inactive tab
   // to the frame image.
-  void SetBackgroundOffset(const gfx::Point& offset) {
+  void set_background_offset(const gfx::Point& offset) {
     background_offset_ = offset;
   }
 
-  // Paints the icon. Most of the time you'll want to invoke Paint directly, but
-  // in certain situations this invoked outside of Paint.
-  void PaintIcon(gfx::Canvas* canvas);
-
   // Returns the minimum possible size of a single unselected Tab.
   static gfx::Size GetMinimumUnselectedSize();
   // Returns the minimum possible size of a selected Tab. Selected tabs must
@@ -59,9 +55,6 @@ class Tab : public BaseTab {
   // Returns the width for mini-tabs. Mini-tabs always have this width.
   static int GetMiniWidth();
 
-  // Loads the images to be used for the tab background.
-  static void LoadTabImages();
-
  protected:
   virtual const gfx::Rect& title_bounds() const { return title_bounds_; }
 
@@ -84,6 +77,7 @@ class Tab : public BaseTab {
   void PaintInactiveTabBackgroundWithTitleChange(gfx::Canvas* canvas);
   void PaintInactiveTabBackground(gfx::Canvas* canvas);
   void PaintActiveTabBackground(gfx::Canvas* canvas);
+  void PaintIcon(gfx::Canvas* canvas);
   SkBitmap DrawHoverGlowBitmap(int width, int height);
 
   // Returns the number of favicon-size elements that can fit in the tab's
@@ -101,6 +95,12 @@ class Tab : public BaseTab {
   // mini tab title change and pulsing.
   double GetThrobValue();
 
+  // Performs a one-time initialization of static resources such as tab images.
+  static void InitTabResources();
+
+  // Loads the images to be used for the tab background.
+  static void LoadTabImages();
+
   // The bounds of various sections of the display.
   gfx::Rect favicon_bounds_;
   gfx::Rect title_bounds_;
@@ -122,9 +122,9 @@ class Tab : public BaseTab {
     int r_width;
     int y_offset;
   };
-  static TabImage tab_active;
-  static TabImage tab_inactive;
-  static TabImage tab_alpha;
+  static TabImage tab_active_;
+  static TabImage tab_inactive_;
+  static TabImage tab_alpha_;
 
   // Whether we're showing the icon. It is cached so that we can detect when it
   // changes and layout appropriately.
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 05aa581..0f3fae4 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -171,7 +171,7 @@ int TabStrip::GetPreferredHeight() {
 
 void TabStrip::SetBackgroundOffset(const gfx::Point& offset) {
   for (int i = 0; i < tab_count(); ++i)
-    GetTabAtTabDataIndex(i)->SetBackgroundOffset(offset);
+    GetTabAtTabDataIndex(i)->set_background_offset(offset);
 }
 
 bool TabStrip::IsPositionInWindowCaption(const gfx::Point& point) {