Clean up bits of the toolbar code....

chrome/browser/views/toolbar_view.cc

 // Copyright (c) 2010 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/views/toolbar_view.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/command_line.h"
 #include "chrome/app/chrome_dll_resource.h"
@@ skipping 74 matching lines @@
       menu_bar_emulation_mode_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       destroyed_flag_(NULL),
       collapsed_(false) {
   SetID(VIEW_ID_TOOLBAR);
   browser_->command_updater()->AddCommandObserver(IDC_BACK, this);
   browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this);
   browser_->command_updater()->AddCommandObserver(IDC_HOME, this);
   browser_->command_updater()->AddCommandObserver(IDC_RELOAD, this);
 
-  if (browser->SupportsWindowFeature(Browser::FEATURE_TABSTRIP))
-    display_mode_ = DISPLAYMODE_NORMAL;
-  else
-    display_mode_ = DISPLAYMODE_LOCATION;
+  display_mode_ = browser->SupportsWindowFeature(Browser::FEATURE_TABSTRIP) ?
+      DISPLAYMODE_NORMAL : DISPLAYMODE_LOCATION;
 
   if (!kPopupBackgroundEdge) {
     kPopupBackgroundEdge = ResourceBundle::GetSharedInstance().GetBitmapNamed(
         IDR_LOCATIONBG_POPUPMODE_EDGE);
   }
 
   if (!Singleton<UpgradeDetector>::get()->notify_upgrade()) {
-    registrar_.Add(this,
-        NotificationType::UPGRADE_RECOMMENDED,
-        NotificationService::AllSources());
+    registrar_.Add(this, NotificationType::UPGRADE_RECOMMENDED,
+                   NotificationService::AllSources());
   }
 }
 
 ToolbarView::~ToolbarView() {
   if (destroyed_flag_)
     *destroyed_flag_ = true;
+
+  // NOTE: Don't remove the command observers here.  This object gets destroyed
+  // after the Browser (which owns the CommandUpdater), so the CommandUpdater is
+  // already gone.
+
   if (menu_bar_emulation_mode_) {
     focus_manager_->UnregisterAccelerators(this);
     focus_manager_->RemoveFocusChangeListener(this);
   }
 }
 
 void ToolbarView::Init(Profile* profile) {
   back_menu_model_.reset(new BackForwardMenuModel(
       browser_, BackForwardMenuModel::BACKWARD_MENU));
   forward_menu_model_.reset(new BackForwardMenuModel(
       browser_, BackForwardMenuModel::FORWARD_MENU));
 
-  // Create all the individual Views in the Toolbar.
-  CreateLeftSideControls();
-  CreateCenterStack(profile);
-  CreateRightSideControls(profile);
+  back_ = new views::ButtonDropDown(this, back_menu_model_.get());
+  back_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
+                                     views::Event::EF_MIDDLE_BUTTON_DOWN);
+  back_->set_tag(IDC_BACK);
+  back_->SetImageAlignment(views::ImageButton::ALIGN_RIGHT,
+                           views::ImageButton::ALIGN_TOP);
+  back_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_BACK));
+  back_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_BACK));
+  back_->SetID(VIEW_ID_BACK_BUTTON);
 
+  forward_ = new views::ButtonDropDown(this, forward_menu_model_.get());
+  forward_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
+                                        views::Event::EF_MIDDLE_BUTTON_DOWN);
+  forward_->set_tag(IDC_FORWARD);
+  forward_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_FORWARD));
+  forward_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_FORWARD));
+  forward_->SetID(VIEW_ID_FORWARD_BUTTON);
+
+  home_ = new views::ImageButton(this);
+  home_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
+                                     views::Event::EF_MIDDLE_BUTTON_DOWN);
+  home_->set_tag(IDC_HOME);
+  home_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_HOME));
+  home_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_HOME));
+  home_->SetID(VIEW_ID_HOME_BUTTON);
+
+  reload_ = new views::ImageButton(this);
+  reload_->set_tag(IDC_RELOAD);
+  reload_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_RELOAD));
+  reload_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_RELOAD));
+  reload_->SetID(VIEW_ID_RELOAD_BUTTON);
+
+  location_bar_ = new LocationBarView(profile, browser_->command_updater(),
+      model_, this, (display_mode_ == DISPLAYMODE_LOCATION) ?
+          LocationBarView::POPUP : LocationBarView::NORMAL);
+  location_bar_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_LOCATION));
+
+  go_ = new GoButton(location_bar_, browser_);
+  go_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_GO));
+  go_->SetID(VIEW_ID_GO_BUTTON);
+
+  browser_actions_ = new BrowserActionsContainer(browser_, this, true);
+
+  page_menu_ = new views::MenuButton(NULL, std::wstring(), this, false);
+  page_menu_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_PAGE));
+  page_menu_->SetTooltipText(l10n_util::GetString(IDS_PAGEMENU_TOOLTIP));
+  page_menu_->SetID(VIEW_ID_PAGE_MENU);
+
+  app_menu_ = new views::MenuButton(NULL, std::wstring(), this, false);
+  app_menu_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_APP));
+  app_menu_->SetTooltipText(l10n_util::GetStringF(IDS_APPMENU_TOOLTIP,
+      l10n_util::GetString(IDS_PRODUCT_NAME)));
+  app_menu_->SetID(VIEW_ID_APP_MENU);
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kBookmarkMenu)) {
+    bookmark_menu_ = new BookmarkMenuButton(browser_);
+    AddChildView(bookmark_menu_);
+  } else {
+    bookmark_menu_ = NULL;
+  }
+
+  LoadImages();
+
+  AddChildView(back_);
+  AddChildView(forward_);
+  AddChildView(home_);
+  AddChildView(reload_);
+  AddChildView(location_bar_);
+  AddChildView(go_);
+  AddChildView(browser_actions_);
+  AddChildView(page_menu_);
+  AddChildView(app_menu_);
+
+  location_bar_->Init();
   show_home_button_.Init(prefs::kShowHomeButton, profile->GetPrefs(), this);
 
+  // Catch the case where the window is created after we detect a new version.
+  if (Singleton<UpgradeDetector>::get()->notify_upgrade())
+    ShowUpgradeReminder();
+
   SetProfile(profile);
-  if (!app_menu_model_.get()) {
+  if (!app_menu_model_.get())
     SetAppMenuModel(new AppMenuModel(this, browser_));
-  }
 
   focus_manager_ = GetFocusManager();
 }
 
 void ToolbarView::SetProfile(Profile* profile) {
-  if (profile == profile_)
-    return;
-
-  profile_ = profile;
-  location_bar_->SetProfile(profile);
+  if (profile != profile_) {
+    profile_ = profile;
+    location_bar_->SetProfile(profile);
+  }
 }
 
 void ToolbarView::Update(TabContents* tab, bool should_restore_state) {
   if (location_bar_)
     location_bar_->Update(should_restore_state ? tab : NULL);
 
   if (browser_actions_)
     browser_actions_->RefreshBrowserActionViews();
 }
 
@@ skipping 40 matching lines @@
   focus_manager_->RegisterAccelerator(left, this);
   views::Accelerator right(base::VKEY_RIGHT, false, false, false);
   focus_manager_->RegisterAccelerator(right, this);
 }
 
 void ToolbarView::AddMenuListener(views::MenuListener* listener) {
   menu_listeners_.push_back(listener);
 }
 
 void ToolbarView::RemoveMenuListener(views::MenuListener* listener) {
-  for (std::vector<views::MenuListener*>::iterator iter =
-      menu_listeners_.begin();
-    iter != menu_listeners_.end();
-    ++iter) {
-      if (*iter == listener) {
-        menu_listeners_.erase(iter);
-        return;
-      }
+  for (std::vector<views::MenuListener*>::iterator i(menu_listeners_.begin());
+       i != menu_listeners_.end(); ++i) {
+    if (*i == listener) {
+      menu_listeners_.erase(i);
+      return;
+    }
   }
 }
 
 void ToolbarView::SetCollapsed(bool val) {
   if (collapsed_ == val)
     return;
 
   collapsed_ = val;
 
   // When switching to and from collapsed view, we need to force hide/show the
   // location bar entry view, like we do when we switch to full screen mode in
   // BrowserView::ProcessFullscreen. Otherwise the text view can appear floating
   // on top of web content.
   if (collapsed_)
     location_bar_->PushForceHidden();
   else
     location_bar_->PopForceHidden();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, FocusChangeListener overrides:
 
 void ToolbarView::FocusWillChange(views::View* focused_before,
                                   views::View* focused_now) {
   // If the focus is switching to something outside the menu bar,
   // take it out of the focus traversal.
-  if (focused_now != NULL &&
-      focused_now != page_menu_ &&
-      focused_now != app_menu_) {
+  if ((focused_now != NULL) && (focused_now != page_menu_) &&
+      (focused_now != app_menu_)) {
     // Post ExitMenuBarEmulationMode to the queue rather than running it
     // right away, because otherwise we'll remove ourselves from the
     // list of listeners while FocusManager is in the middle of iterating
     // over that list.
     MessageLoop::current()->PostTask(
         FROM_HERE, method_factory_.NewRunnableMethod(
             &ToolbarView::ExitMenuBarEmulationMode));
   }
 }
 
@@ skipping 113 matching lines @@
     }
   } else if (type == NotificationType::UPGRADE_RECOMMENDED) {
     ShowUpgradeReminder();
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, menus::SimpleMenuModel::Delegate implementation:
 
 bool ToolbarView::IsCommandIdChecked(int command_id) const {
-  if (command_id == IDC_SHOW_BOOKMARK_BAR)
-    return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
-  return false;
+  return (command_id == IDC_SHOW_BOOKMARK_BAR) &&
+      profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
 }
 
 bool ToolbarView::IsCommandIdEnabled(int command_id) const {
   return browser_->command_updater()->IsCommandEnabled(command_id);
 }
 
 bool ToolbarView::GetAcceleratorForCommandId(int command_id,
     menus::Accelerator* accelerator) {
   // The standard Ctrl-X, Ctrl-V and Ctrl-C are not defined as accelerators
   // anywhere so we need to check for them explicitly here.
@@ skipping 30 matching lines @@
   }
 
   // Safe to cast, given the check above.
   views::MenuButton* menu = static_cast<views::MenuButton*>(focused_view);
   switch (accelerator.GetKeyCode()) {
     case base::VKEY_ESCAPE:
       RestoreLastFocusedView();
       return true;
     case base::VKEY_LEFT:
     case base::VKEY_RIGHT:
-      if (menu == app_menu_)
-        page_menu_->RequestFocus();
-      else
-        app_menu_->RequestFocus();
+      ((menu == app_menu_) ? page_menu_ : app_menu_)->RequestFocus();
       return true;
     case base::VKEY_UP:
     case base::VKEY_DOWN:
     case base::VKEY_RETURN:
     case base::VKEY_SPACE:
       // Hide the tooltip before activating a menu button.
       if (GetWidget()->GetTooltipManager())
         GetWidget()->GetTooltipManager()->HideKeyboardTooltip();
-
       ActivateMenuButton(menu);
       return true;
     default:
       return false;
   }
 }
 
 gfx::Size ToolbarView::GetPreferredSize() {
   if (IsDisplayModeNormal()) {
     int min_width = kControlIndent + back_->GetPreferredSize().width() +
         forward_->GetPreferredSize().width() + kControlHorizOffset +
         (show_home_button_.GetValue() ?
             (home_->GetPreferredSize().width() + kControlHorizOffset) : 0) +
         reload_->GetPreferredSize().width() +
         browser_actions_->GetPreferredSize().width() +
         go_->GetPreferredSize().width() +
         kMenuButtonOffset +
         (bookmark_menu_ ? bookmark_menu_->GetPreferredSize().width() : 0) +
         page_menu_->GetPreferredSize().width() +
         app_menu_->GetPreferredSize().width() + kPaddingRight;
 
     static SkBitmap normal_background;
     if (normal_background.isNull()) {
       ResourceBundle& rb = ResourceBundle::GetSharedInstance();
       normal_background = *rb.GetBitmapNamed(IDR_CONTENT_TOP_CENTER);
     }
 
-    if (collapsed_)
-      return gfx::Size(min_width, kCollapsedToolbarHeight);
-    else
-      return gfx::Size(min_width, normal_background.height());
+    return gfx::Size(min_width,
+        collapsed_ ? kCollapsedToolbarHeight : normal_background.height());
   }
 
   int vertical_spacing = PopupTopSpacing() +
       (GetWindow()->GetNonClientView()->UseNativeFrame() ?
           kPopupBottomSpacingGlass : kPopupBottomSpacingNonGlass);
   return gfx::Size(0, location_bar_->GetPreferredSize().height() +
       vertical_spacing);
 }
 
 void ToolbarView::Layout() {
   // If we have not been initialized yet just do nothing.
   if (back_ == NULL)
     return;
 
   if (!IsDisplayModeNormal()) {
     int edge_width = (browser_->window() && browser_->window()->IsMaximized()) ?
         0 : kPopupBackgroundEdge->width();  // See Paint().
     location_bar_->SetBounds(edge_width, PopupTopSpacing(),
         width() - (edge_width * 2), location_bar_->GetPreferredSize().height());
     return;
   }
 
   // In collapsed mode, we don't show any of the child controls.
   for (int i = 0; i < GetChildViewCount(); ++i)
     GetChildViewAt(i)->SetVisible(!collapsed_);
 
   int child_y = std::min(kControlVertOffset, height());
   // We assume all child elements are the same height.
   int child_height =
-      std::min(go_->GetPreferredSize().height(), height() - child_y);
+      std::min(back_->GetPreferredSize().height(), height() - child_y);
 
   // If the window is maximized, we extend the back button to the left so that
   // clicking on the left-most pixel will activate the back button.
   // TODO(abarth):  If the window becomes maximized but is not resized,
   //                then Layout() might not be called and the back button
   //                will be slightly the wrong size.  We should force a
   //                Layout() in this case.
   //                http://crbug.com/5540
   int back_width = back_->GetPreferredSize().width();
   if (browser_->window() && browser_->window()->IsMaximized())
@@ skipping 26 matching lines @@
   int location_x = reload_->x() + reload_->width();
   int available_width = width() - kPaddingRight - bookmark_menu_width -
       app_menu_width - page_menu_width - browser_actions_width -
       kMenuButtonOffset - go_button_width - location_x;
 
   location_bar_->SetBounds(location_x, child_y, std::max(available_width, 0),
                            child_height);
 
   go_->SetBounds(location_bar_->x() + location_bar_->width(), child_y,
                  go_button_width, child_height);
-
   int next_menu_x = go_->x() + go_->width() + kMenuButtonOffset;
 
   browser_actions_->SetBounds(next_menu_x, 0, browser_actions_width, height());
-
   // The browser actions need to do a layout explicitly, because when an
   // extension is loaded/unloaded/changed, BrowserActionContainer removes and
   // re-adds everything, regardless of whether it has a page action. For a
   // page action, browser action bounds do not change, as a result of which
   // SetBounds does not do a layout at all.
-  // TODO(sidchat): Rework the above bahavior so that explicit layout is not
+  // TODO(sidchat): Rework the above behavior so that explicit layout is not
   //                required.
   browser_actions_->Layout();
-
   next_menu_x += browser_actions_width;
 
   if (bookmark_menu_) {
     bookmark_menu_->SetBounds(next_menu_x, child_y, bookmark_menu_width,
                               child_height);
     next_menu_x += bookmark_menu_width;
   }
 
   page_menu_->SetBounds(next_menu_x, child_y, page_menu_width, child_height);
   next_menu_x += page_menu_width;
@@ skipping 17 matching lines @@
   }
 
   // For glass, we need to draw a black line below the location bar to separate
   // it from the content area.  For non-glass, the NonClientView draws the
   // toolbar background below the location bar for us.
   if (GetWindow()->GetNonClientView()->UseNativeFrame())
     canvas->FillRectInt(SK_ColorBLACK, 0, height() - 1, width(), 1);
 }
 
 void ToolbarView::ThemeChanged() {
-  LoadLeftSideControlsImages();
-  LoadCenterStackImages();
-  LoadRightSideControlsImages();
+  LoadImages();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ToolbarView, private:
 
 int ToolbarView::PopupTopSpacing() const {
   return GetWindow()->GetNonClientView()->UseNativeFrame() ?
       0 : kPopupTopSpacingNonGlass;
 }
 
-void ToolbarView::CreateLeftSideControls() {
-  back_ = new views::ButtonDropDown(this, back_menu_model_.get());
-  back_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
-                                  views::Event::EF_MIDDLE_BUTTON_DOWN);
-  back_->set_tag(IDC_BACK);
-  back_->SetImageAlignment(views::ImageButton::ALIGN_RIGHT,
-                           views::ImageButton::ALIGN_TOP);
-  back_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_BACK));
-  back_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_BACK));
-  back_->SetID(VIEW_ID_BACK_BUTTON);
-
-  forward_ = new views::ButtonDropDown(this, forward_menu_model_.get());
-  forward_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
-                                        views::Event::EF_MIDDLE_BUTTON_DOWN);
-  forward_->set_tag(IDC_FORWARD);
-  forward_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_FORWARD));
-  forward_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_FORWARD));
-  forward_->SetID(VIEW_ID_FORWARD_BUTTON);
-
-  home_ = new views::ImageButton(this);
-  home_->set_triggerable_event_flags(views::Event::EF_LEFT_BUTTON_DOWN |
-                                  views::Event::EF_MIDDLE_BUTTON_DOWN);
-  home_->set_tag(IDC_HOME);
-  home_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_HOME));
-  home_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_HOME));
-  home_->SetID(VIEW_ID_HOME_BUTTON);
-
-  LoadLeftSideControlsImages();
-
-  AddChildView(back_);
-  AddChildView(forward_);
-  AddChildView(home_);
-}
-
-void ToolbarView::CreateCenterStack(Profile *profile) {
-  reload_ = new views::ImageButton(this);
-  reload_->set_tag(IDC_RELOAD);
-  reload_->SetTooltipText(l10n_util::GetString(IDS_TOOLTIP_RELOAD));
-  reload_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_RELOAD));
-  reload_->SetID(VIEW_ID_RELOAD_BUTTON);
-
-  location_bar_ = new LocationBarView(profile, browser_->command_updater(),
-      model_, this, (display_mode_ == DISPLAYMODE_LOCATION) ?
-          LocationBarView::POPUP : LocationBarView::NORMAL);
-
-  // The Go button.
-  go_ = new GoButton(location_bar_, browser_);
-  go_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_GO));
-  go_->SetID(VIEW_ID_GO_BUTTON);
-
-  LoadCenterStackImages();
-
-  AddChildView(reload_);
-  location_bar_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_LOCATION));
-  AddChildView(location_bar_);
-  location_bar_->Init();
-  AddChildView(go_);
-}
-
-void ToolbarView::CreateRightSideControls(Profile* profile) {
-  browser_actions_ = new BrowserActionsContainer(browser_, this,
-                                                 true);  // should_save_size
-
-  page_menu_ = new views::MenuButton(NULL, std::wstring(), this, false);
-  page_menu_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_PAGE));
-  page_menu_->SetTooltipText(l10n_util::GetString(IDS_PAGEMENU_TOOLTIP));
-  page_menu_->SetID(VIEW_ID_PAGE_MENU);
-
-  app_menu_ = new views::MenuButton(NULL, std::wstring(), this, false);
-  app_menu_->SetAccessibleName(l10n_util::GetString(IDS_ACCNAME_APP));
-  app_menu_->SetTooltipText(l10n_util::GetStringF(IDS_APPMENU_TOOLTIP,
-      l10n_util::GetString(IDS_PRODUCT_NAME)));
-  app_menu_->SetID(VIEW_ID_APP_MENU);
-
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kBookmarkMenu)) {
-    bookmark_menu_ = new BookmarkMenuButton(browser_);
-    AddChildView(bookmark_menu_);
-  } else {
-    bookmark_menu_ = NULL;
-  }
-
-  // Catch the case where the window is created after we detect a new version.
-  if (Singleton<UpgradeDetector>::get()->notify_upgrade())
-    ShowUpgradeReminder();
-
-  LoadRightSideControlsImages();
-
-  AddChildView(browser_actions_);
-  AddChildView(page_menu_);
-  AddChildView(app_menu_);
-}
-
-void ToolbarView::LoadLeftSideControlsImages() {
+void ToolbarView::LoadImages() {
   ThemeProvider* tp = GetThemeProvider();
 
   SkColor color = tp->GetColor(BrowserThemeProvider::COLOR_BUTTON_BACKGROUND);
   SkBitmap* background = tp->GetBitmapNamed(IDR_THEME_BUTTON_BACKGROUND);
 
   back_->SetImage(views::CustomButton::BS_NORMAL, tp->GetBitmapNamed(IDR_BACK));
   back_->SetImage(views::CustomButton::BS_HOT, tp->GetBitmapNamed(IDR_BACK_H));
   back_->SetImage(views::CustomButton::BS_PUSHED,
       tp->GetBitmapNamed(IDR_BACK_P));
   back_->SetImage(views::CustomButton::BS_DISABLED,
@@ skipping 11 matching lines @@
       tp->GetBitmapNamed(IDR_FORWARD_D));
   forward_->SetBackground(color, background,
       tp->GetBitmapNamed(IDR_FORWARD_MASK));
 
   home_->SetImage(views::CustomButton::BS_NORMAL, tp->GetBitmapNamed(IDR_HOME));
   home_->SetImage(views::CustomButton::BS_HOT, tp->GetBitmapNamed(IDR_HOME_H));
   home_->SetImage(views::CustomButton::BS_PUSHED,
       tp->GetBitmapNamed(IDR_HOME_P));
   home_->SetBackground(color, background,
       tp->GetBitmapNamed(IDR_BUTTON_MASK));
-}
-
-void ToolbarView::LoadCenterStackImages() {
-  ThemeProvider* tp = GetThemeProvider();
-
-  SkColor color = tp->GetColor(BrowserThemeProvider::COLOR_BUTTON_BACKGROUND);
-  SkBitmap* background = tp->GetBitmapNamed(IDR_THEME_BUTTON_BACKGROUND);
 
   reload_->SetImage(views::CustomButton::BS_NORMAL,
       tp->GetBitmapNamed(IDR_RELOAD));
   reload_->SetImage(views::CustomButton::BS_HOT,
       tp->GetBitmapNamed(IDR_RELOAD_H));
   reload_->SetImage(views::CustomButton::BS_PUSHED,
       tp->GetBitmapNamed(IDR_RELOAD_P));
   reload_->SetBackground(color, background,
       tp->GetBitmapNamed(IDR_RELOAD_MASK));
 
   go_->SetImage(views::CustomButton::BS_NORMAL, tp->GetBitmapNamed(IDR_GO));
   go_->SetImage(views::CustomButton::BS_HOT, tp->GetBitmapNamed(IDR_GO_H));
   go_->SetImage(views::CustomButton::BS_PUSHED, tp->GetBitmapNamed(IDR_GO_P));
   go_->SetToggledImage(views::CustomButton::BS_NORMAL,
       tp->GetBitmapNamed(IDR_STOP));
   go_->SetToggledImage(views::CustomButton::BS_HOT,
       tp->GetBitmapNamed(IDR_STOP_H));
   go_->SetToggledImage(views::CustomButton::BS_PUSHED,
       tp->GetBitmapNamed(IDR_STOP_P));
   go_->SetBackground(color, background,
       tp->GetBitmapNamed(IDR_GO_MASK));
+
+  // We use different menu button images if the locale is right-to-left.
+  page_menu_->SetIcon(*tp->GetBitmapNamed(
+      base::i18n::IsRTL() ? IDR_MENU_PAGE_RTL : IDR_MENU_PAGE));
+  app_menu_->SetIcon(GetAppMenuIcon());
+  if (bookmark_menu_ != NULL)
+    bookmark_menu_->SetIcon(*tp->GetBitmapNamed(IDR_MENU_BOOKMARK));
 }
 
 void ToolbarView::ShowUpgradeReminder() {
   update_reminder_animation_.reset(new SlideAnimation(this));
   update_reminder_animation_->SetSlideDuration(kPulseDuration);
 
   // Then start the recurring timer for pulsating it.
   upgrade_reminder_pulse_timer_.Start(
       base::TimeDelta::FromMilliseconds(kPulsateEveryMs),
       this, &ToolbarView::PulsateUpgradeNotifier);
 }
 
 void ToolbarView::PulsateUpgradeNotifier() {
   // Start the pulsating animation.
   update_reminder_animation_->Reset(0.0);
   update_reminder_animation_->Show();
 }
 
 SkBitmap ToolbarView::GetAppMenuIcon() {
   ThemeProvider* tp = GetThemeProvider();
 
-  SkBitmap icon;
-
   // We use different menu button images if the locale is right-to-left.
-  if (base::i18n::IsRTL())
-    icon = *tp->GetBitmapNamed(IDR_MENU_CHROME_RTL);
-  else
-    icon = *tp->GetBitmapNamed(IDR_MENU_CHROME);
+  SkBitmap icon = *tp->GetBitmapNamed(
+      base::i18n::IsRTL() ? IDR_MENU_CHROME_RTL : IDR_MENU_CHROME);
 
   if (!Singleton<UpgradeDetector>::get()->notify_upgrade())
     return icon;
 
   // Draw the chrome app menu icon onto the canvas.
   scoped_ptr<gfx::Canvas> canvas(
       new gfx::Canvas(icon.width(), icon.height(), false));
   canvas->DrawBitmapInt(icon, 0, 0);
 
   SkBitmap badge;
-
   static bool has_faded_in = false;
   if (!has_faded_in) {
     SkBitmap* dot = tp->GetBitmapNamed(IDR_UPGRADE_DOT_INACTIVE);
     SkBitmap transparent;
     transparent.setConfig(dot->getConfig(), dot->width(), dot->height());
     transparent.allocPixels();
     transparent.eraseARGB(0, 0, 0, 0);
     badge = SkBitmapOperations::CreateBlendedBitmap(
         *dot, transparent, 1.0 - update_reminder_animation_->GetCurrentValue());
     if (update_reminder_animation_->GetCurrentValue() == 1.0)
       has_faded_in = true;
   } else {
     // Convert animation values that start from 0.0 and incrementally go
     // up to 1.0 into values that start in 0.0, go to 1.0 and then back
     // to 0.0 (to create a pulsing effect).
     double value = 1.0 -
                    abs(2.0 * update_reminder_animation_->GetCurrentValue() -
                        1.0);
 
     // Add the badge to it.
     badge = SkBitmapOperations::CreateBlendedBitmap(
         *tp->GetBitmapNamed(IDR_UPGRADE_DOT_INACTIVE),
         *tp->GetBitmapNamed(IDR_UPGRADE_DOT_ACTIVE),
         value);
   }
 
-  int x_pos = kUpgradeDotOffset;
-  if (base::i18n::IsRTL())
-    x_pos = icon.width() - badge.width();
+  int x_pos = base::i18n::IsRTL() ?
+      (icon.width() - badge.width()) : kUpgradeDotOffset;
   canvas->DrawBitmapInt(badge, x_pos, icon.height() - badge.height());
 
   return canvas->ExtractBitmap();
 }
 
-void ToolbarView::LoadRightSideControlsImages() {
-  ThemeProvider* tp = GetThemeProvider();
-
-  // We use different menu button images if the locale is right-to-left.
-  if (base::i18n::IsRTL())
-    page_menu_->SetIcon(*tp->GetBitmapNamed(IDR_MENU_PAGE_RTL));
-  else
-    page_menu_->SetIcon(*tp->GetBitmapNamed(IDR_MENU_PAGE));
-
-  app_menu_->SetIcon(GetAppMenuIcon());
-
-  if (bookmark_menu_ != NULL)
-    bookmark_menu_->SetIcon(*tp->GetBitmapNamed(IDR_MENU_BOOKMARK));
-}
-
 void ToolbarView::RunPageMenu(const gfx::Point& pt) {
   bool destroyed_flag = false;
   destroyed_flag_ = &destroyed_flag;
 
   page_menu_model_.reset(new PageMenuModel(this, browser_));
   page_menu_menu_.reset(new views::Menu2(page_menu_model_.get()));
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     page_menu_menu_->AddMenuListener(menu_listeners_[i]);
   }
   page_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
 
   if (destroyed_flag)
     return;
 
   destroyed_flag_ = NULL;
 
   for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
     page_menu_menu_->RemoveMenuListener(menu_listeners_[i]);
   }
   SwitchToOtherMenuIfNeeded(page_menu_menu_.get(), app_menu_);
 }
 
 void ToolbarView::RunAppMenu(const gfx::Point& pt) {
   bool destroyed_flag = false;
   destroyed_flag_ = &destroyed_flag;
 
   if (app_menu_model_->BuildProfileSubMenu())
     app_menu_menu_->Rebuild();
-  for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
+  for (size_t i = 0; i < menu_listeners_.size(); i++)
     app_menu_menu_->AddMenuListener(menu_listeners_[i]);
-  }
   app_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT);
 
   if (destroyed_flag)
     return;
-
   destroyed_flag_ = NULL;
 
   // Stop pulsating the upgrade reminder on the app menu, if active.
   upgrade_reminder_pulse_timer_.Stop();
 
-  for (unsigned int i = 0; i < menu_listeners_.size(); i++) {
+  for (size_t i = 0; i < menu_listeners_.size(); i++)
     app_menu_menu_->RemoveMenuListener(menu_listeners_[i]);
-  }
   SwitchToOtherMenuIfNeeded(app_menu_menu_.get(), page_menu_);
 }
 
 void ToolbarView::SwitchToOtherMenuIfNeeded(
-    views::Menu2* previous_menu, views::MenuButton* next_menu_button) {
+    views::Menu2* previous_menu,
+    views::MenuButton* next_menu_button) {
   // If the user tried to move to the right or left, switch from the
   // app menu to the page menu. Switching to the next menu is delayed
   // until the next event loop so that the call stack that initiated
   // activating the first menu can return. (If we didn't do this, the
   // call stack would grow each time the user switches menus, and
   // the actions taken after the user finally exits a menu would cause
   // flicker.)
   views::MenuWrapper::MenuAction action = previous_menu->GetMenuAction();
   if (action == views::MenuWrapper::MENU_ACTION_NEXT ||
       action == views::MenuWrapper::MENU_ACTION_PREVIOUS) {
-    MessageLoop::current()->PostTask(
-        FROM_HERE, method_factory_.NewRunnableMethod(
-            &ToolbarView::ActivateMenuButton,
-            next_menu_button));
+    MessageLoop::current()->PostTask(FROM_HERE,
+        method_factory_.NewRunnableMethod(&ToolbarView::ActivateMenuButton,
+                                          next_menu_button));
   }
 }
 
 void ToolbarView::ActivateMenuButton(views::MenuButton* menu_button) {
-#if defined(OS_LINUX)
+#if defined(OS_WIN)
+  // On Windows, we have to explicitly clear the focus before opening
+  // the pop-up menu, then set the focus again when it closes.
+  GetFocusManager()->ClearFocus();
+#elif defined(OS_LINUX)
   // Under GTK, opening a pop-up menu causes the main window to lose focus.
   // Focus is automatically returned when the menu closes.
   //
   // Make sure that the menu button being activated has focus, so that
   // when the user escapes from the menu without selecting anything, focus
   // will be returned here.
   if (!menu_button->HasFocus()) {
     menu_button->RequestFocus();
     GetFocusManager()->StoreFocusedView();
   }
 #endif
 
-#if defined(OS_WIN)
-  // On Windows, we have to explicitly clear the focus before opening
-  // the pop-up menu, then set the focus again when it closes.
-  GetFocusManager()->ClearFocus();
-#endif
-
   // Tell the menu button to activate, opening its pop-up menu.
   menu_button->Activate();
 
 #if defined(OS_WIN)
   EnterMenuBarEmulationMode(last_focused_view_storage_id_, menu_button);
 #endif
 }
 
 void ToolbarView::ExitMenuBarEmulationMode() {
   if (page_menu_->HasFocus() || app_menu_->HasFocus())
@@ skipping 14 matching lines @@
     last_focused_view->RequestFocus();
   } else {
     // Focus the location bar
     views::View* view = GetAncestorWithClassName(BrowserView::kViewClassName);
     if (view) {
       BrowserView* browser_view = static_cast<BrowserView*>(view);
       browser_view->SetFocusToLocationBar(false);
     }
   }
 }