More cleanup in Browser:

- remove "RunSimpleFrameMenu" method that isn't used anymore.
- rename Browser::GetType() to Browser::type()


Review URL: http://codereview.chromium.org/10605

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

11 files changed, 36 insertions, 121 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 5323e10..1997a38 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -733,7 +733,7 @@ void Browser::CloseContents(TabContents* source) {
 }
 
 void Browser::MoveContents(TabContents* source, const gfx::Rect& pos) {
-  if (GetType() != BrowserType::BROWSER) {
+  if (type() != BrowserType::BROWSER) {
     NOTREACHED() << "moving invalid browser type";
     return;
   }
@@ -745,7 +745,7 @@ void Browser::MoveContents(TabContents* source, const gfx::Rect& pos) {
 
 bool Browser::IsPopup(TabContents* source) {
   // A non-tabbed BROWSER is an unconstrained popup.
-  return (GetType() == BrowserType::BROWSER);
+  return (type() == BrowserType::BROWSER);
 }
 
 void Browser::ShowHtmlDialog(HtmlDialogContentsDelegate* delegate,
@@ -1207,6 +1207,7 @@ TabContents* Browser::CreateTabContentsForURL(
 }
 
 void Browser::ValidateLoadingAnimations() {
+  // TODO(beng): Remove this, per http://crbug.com/3297
   if (window_)
     window_->ValidateThrobber();
 }
@@ -1340,10 +1341,6 @@ void Browser::TabStripEmpty() {
       method_factory_.NewRunnableMethod(&Browser::CloseFrame));
 }
 
-BrowserType::Type Browser::GetType() const {
-  return type_;
-}
-
 void Browser::InitHangMonitor() {
   PrefService* pref_service = g_browser_process->local_state();
   DCHECK(pref_service != NULL);
@@ -1449,7 +1446,7 @@ void Browser::OpenURLOffTheRecord(Profile* profile, const GURL& url) {
 }
 
 void Browser::ConvertToTabbedBrowser() {
-  if (GetType() != BrowserType::BROWSER) {
+  if (type() != BrowserType::BROWSER) {
     NOTREACHED();
     return;
   }
@@ -1560,7 +1557,7 @@ NavigationController* Browser::GetSelectedNavigationController() const {
 
 void Browser::SaveWindowPosition(const gfx::Rect& bounds, bool maximized) {
   // We don't save window position for popups.
-  if (GetType() == BrowserType::BROWSER)
+  if (type() == BrowserType::BROWSER)
     return;
 
   // First save to local state, this is for remembering on subsequent starts.
diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h
index 1912fd8..0855d2f 100644
--- a/chrome/browser/browser.h
+++ b/chrome/browser/browser.h
@@ -36,17 +36,6 @@ class Browser : public TabStripModelDelegate,
                 public NotificationObserver,
                 public SelectFileDialog::Listener {
  public:
-  // TODO(beng): (Cleanup) This is a hack. Right now the |initial_bounds|
-  // parameter to Browser's ctor specifies the size of the frame, not the size
-  // of the contents that will be displayed within it. So this flag exists,
-  // which can be passed instead of a typical value for |show_command| that
-  // tells the Browser to create its window, and then use the |initial_bounds|
-  // parameter as the size of the contents, resizing the frame to fit. See
-  // SizeToContents method on chrome_frame.h
-  enum {
-    SIZE_TO_CONTENTS = 9999
-  };
-
   // Creates a new browser with the given bounds. If the bounds are empty, the
   // system will try to find a saved size from a previous session, if none
   // exists, the operating system will be allowed to size the window.
@@ -61,6 +50,8 @@ class Browser : public TabStripModelDelegate,
           const std::wstring& app_name);
   ~Browser();
 
+  BrowserType::Type type() const { return type_; }
+
   // Shows the browser window. It is initially created hidden. It will be shown
   // with the show command passed to the constructor, or possibly another state
   // if it was overridden in the preferences.
@@ -232,6 +223,9 @@ class Browser : public TabStripModelDelegate,
   virtual int GetDragActions() const;
   // Construct a TabContents for a given URL, profile and transition type.
   // If instance is not null, its process will be used to render the tab.
+  // TODO(beng): remove this from TabStripDelegate, it's only used by
+  //             TabStripModel::AddBlankTab*, which should really live here
+  //             on Browser.
   virtual TabContents* CreateTabContentsForURL(
       const GURL& url,
       const GURL& referrer,
@@ -288,13 +282,6 @@ class Browser : public TabStripModelDelegate,
   virtual void ContentsStateChanged(TabContents* source);
   virtual bool ShouldDisplayURLField();
 
-  // Return this browser type.
-  BrowserType::Type GetType() const;
-
-  // Invoke the menu we use for application and popup windows at the provided
-  // point and for the provided hwnd.
-  void RunSimpleFrameMenu(const gfx::Point& pt, HWND hwnd);
-
   // Show some native UI given a URL. If a tab with the same URL is already
   // visible in this browser, it becomes selected. Otherwise a new tab is
   // created.
@@ -318,10 +305,6 @@ class Browser : public TabStripModelDelegate,
   // window. Otherwise, create a new window with an off the record tab.
   static void OpenURLOffTheRecord(Profile* p, const GURL& url);
 
-  // Computes a title suitable for popups without a URL field.
-  static std::wstring ComputePopupTitle(const GURL& url,
-                                        const std::wstring& title);
-
   // Compute a deterministic name based on the URL. We use this pseudo name
   // as a key to store window location per application URLs.
   static std::wstring ComputeApplicationNameFromURL(const GURL& url);
@@ -337,8 +320,6 @@ class Browser : public TabStripModelDelegate,
   // Returns the location bar view for this browser.
   LocationBarView* GetLocationBarView() const;
 
-  void ConvertTabToApplication(TabContents* contents);
-
   // NEW FRAME METHODS BELOW THIS LINE ONLY... TODO(beng): clean up this file!
 
   // Save and restore the window position.
@@ -480,6 +461,7 @@ class Browser : public TabStripModelDelegate,
 
   // Removes the tab from the associated vector. Returns whether the tab
   // was in the vector in the first place.
+  // TODO(beng): this method needs a better name!
   bool RemoveFromVector(UnloadListenerVector* vector, TabContents* tab);
 
   // Cleans up state appropriately when we are trying to close the browser and
@@ -534,10 +516,6 @@ class Browser : public TabStripModelDelegate,
   // close the browser. Only gets populated when we try to close the browser.
   UnloadListenerVector tabs_needing_unload_fired_;
 
-  // Whether we already handled the OnStripEmpty event - it can be called
-  // multiple times.
-  bool handled_strip_empty_;
-
   // Whether we are processing the beforeunload and unload events of each tab
   // in preparation for closing the browser.
   bool is_attempting_to_close_browser_;
diff --git a/chrome/browser/browser_commands.cc b/chrome/browser/browser_commands.cc
index f359c14..6b90148 100644
--- a/chrome/browser/browser_commands.cc
+++ b/chrome/browser/browser_commands.cc
@@ -54,7 +54,7 @@ void Browser::InitCommandState() {
   controller_.UpdateCommandEnabled(IDC_STOP, true);
   controller_.UpdateCommandEnabled(IDC_RELOAD, true);
   controller_.UpdateCommandEnabled(IDC_HOME,
-                                   GetType() == BrowserType::TABBED_BROWSER);
+                                   type() == BrowserType::TABBED_BROWSER);
   controller_.UpdateCommandEnabled(IDC_GO, true);
   controller_.UpdateCommandEnabled(IDC_NEWTAB, true);
   controller_.UpdateCommandEnabled(IDC_CLOSETAB, true);
@@ -149,12 +149,12 @@ void Browser::InitCommandState() {
   controller_.UpdateCommandEnabled(IDC_ENCODING_WINDOWS1258, true);
   controller_.UpdateCommandEnabled(IDC_OPTIONS, true);
   controller_.UpdateCommandEnabled(IDC_CLOSE_WEB_APP,
-                                   GetType() != BrowserType::TABBED_BROWSER);
+                                   type() != BrowserType::TABBED_BROWSER);
   controller_.UpdateCommandEnabled(IDC_SHOW_AS_TAB,
-                                   GetType() == BrowserType::BROWSER);
+                                   type() == BrowserType::BROWSER);
   controller_.UpdateCommandEnabled(
       IDC_RESTORE_TAB, (!profile_->IsOffTheRecord() &&
-                        GetType() == BrowserType::TABBED_BROWSER));
+                        type() == BrowserType::TABBED_BROWSER));
   controller_.UpdateCommandEnabled(IDC_EXIT, true);
   // the debugger doesn't work in single process mode
   controller_.UpdateCommandEnabled(IDC_DEBUGGER,
@@ -288,7 +288,7 @@ void Browser::ExecuteCommand(int id) {
 
     case IDC_NEWTAB:
       UserMetrics::RecordAction(L"NewTab", profile_);
-      if (GetType() == BrowserType::TABBED_BROWSER) {
+      if (type() == BrowserType::TABBED_BROWSER) {
         AddBlankTab(true);
       } else {
         Browser* b = GetOrCreateTabbedBrowser();
@@ -906,64 +906,6 @@ void Browser::OpenClearBrowsingDataDialog() {
         new ClearBrowsingDataView(profile_))->Show();
 }
 
-void Browser::RunSimpleFrameMenu(const gfx::Point& pt, HWND hwnd) {
-  bool for_popup = !IsApplication();
-  EncodingMenuControllerDelegate d(this, &controller_);
-
-  // The menu's anchor point is different based on the UI layout.
-  Menu::AnchorPoint anchor;
-  if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
-    anchor = Menu::TOPRIGHT;
-  else
-    anchor = Menu::TOPLEFT;
-
-  Menu m(&d, anchor, hwnd);
-  m.AppendMenuItemWithLabel(IDC_BACK,
-                            l10n_util::GetString(IDS_CONTENT_CONTEXT_BACK));
-  m.AppendMenuItemWithLabel(IDC_FORWARD,
-                            l10n_util::GetString(
-                                IDS_CONTENT_CONTEXT_FORWARD));
-  m.AppendMenuItemWithLabel(IDC_RELOAD,
-                            l10n_util::GetString(IDS_APP_MENU_RELOAD));
-  m.AppendSeparator();
-  m.AppendMenuItemWithLabel(IDC_DUPLICATE,
-                            l10n_util::GetString(IDS_APP_MENU_DUPLICATE));
-  m.AppendMenuItemWithLabel(IDC_COPY_URL,
-                            l10n_util::GetString(IDS_APP_MENU_COPY_URL));
-  if (for_popup) {
-    m.AppendMenuItemWithLabel(IDC_SHOW_AS_TAB,
-                              l10n_util::GetString(IDS_SHOW_AS_TAB));
-  }
-  m.AppendMenuItemWithLabel(IDC_NEWTAB,
-                            l10n_util::GetString(IDS_APP_MENU_NEW_WEB_PAGE));
-  m.AppendSeparator();
-  m.AppendMenuItemWithLabel(IDC_CUT, l10n_util::GetString(IDS_CUT));
-  m.AppendMenuItemWithLabel(IDC_COPY, l10n_util::GetString(IDS_COPY));
-  m.AppendMenuItemWithLabel(IDC_PASTE, l10n_util::GetString(IDS_PASTE));
-  m.AppendSeparator();
-  m.AppendMenuItemWithLabel(IDC_FIND, l10n_util::GetString(IDS_FIND_IN_PAGE));
-  m.AppendMenuItemWithLabel(IDC_SAVEPAGE, l10n_util::GetString(IDS_SAVEPAGEAS));
-  m.AppendMenuItemWithLabel(IDC_PRINT, l10n_util::GetString(IDS_PRINT));
-  m.AppendSeparator();
-  Menu* zoom_menu = m.AppendSubMenu(IDC_ZOOM,
-                                    l10n_util::GetString(IDS_ZOOM));
-  zoom_menu->AppendMenuItemWithLabel(IDC_ZOOM_PLUS,
-                                     l10n_util::GetString(IDS_ZOOM_PLUS));
-  zoom_menu->AppendMenuItemWithLabel(IDC_ZOOM_NORMAL,
-                                     l10n_util::GetString(IDS_ZOOM_NORMAL));
-  zoom_menu->AppendMenuItemWithLabel(IDC_ZOOM_MINUS,
-                                     l10n_util::GetString(IDS_ZOOM_MINUS));
-
-  // Create encoding menu.
-  Menu* encoding_menu = m.AppendSubMenu(IDC_ENCODING,
-                                        l10n_util::GetString(IDS_ENCODING));
-  EncodingMenuControllerDelegate::BuildEncodingMenu(profile_, encoding_menu);
-
-  m.AppendSeparator();
-  m.AppendMenuItemWithLabel(IDC_CLOSE_WEB_APP, l10n_util::GetString(IDS_CLOSE));
-  m.RunMenuAt(pt.x(), pt.y());
-}
-
 void Browser::CopyCurrentURLToClipBoard() {
   TabContents* tc = GetSelectedTabContents();
   DCHECK(tc);
diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc
index 37364f6..1a2941d 100644
--- a/chrome/browser/browser_init.cc
+++ b/chrome/browser/browser_init.cc
@@ -532,7 +532,7 @@ Browser* BrowserInit::LaunchWithProfile::OpenURLsInBrowser(
     bool process_startup,
     const std::vector<GURL>& urls) {
   DCHECK(!urls.empty());
-  if (!browser || browser->GetType() != BrowserType::TABBED_BROWSER)
+  if (!browser || browser->type() != BrowserType::TABBED_BROWSER)
     browser = CreateTabbedBrowser();
 
   for (size_t i = 0; i < urls.size(); ++i) {
diff --git a/chrome/browser/browser_list.cc b/chrome/browser/browser_list.cc
index cd1e488..45d12c7 100644
--- a/chrome/browser/browser_list.cc
+++ b/chrome/browser/browser_list.cc
@@ -178,7 +178,7 @@ Browser* BrowserList::GetLastActive() {
 // static
 Browser* BrowserList::FindBrowserWithType(Profile* p, BrowserType::Type t) {
   Browser* last_active = GetLastActive();
-  if (last_active && last_active->profile() == p && last_active->GetType() == t)
+  if (last_active && last_active->profile() == p && last_active->type() == t)
     return last_active;
 
   BrowserList::const_iterator i;
@@ -186,7 +186,7 @@ Browser* BrowserList::FindBrowserWithType(Profile* p, BrowserType::Type t) {
     if (*i == last_active)
       continue;
 
-    if ((*i)->profile() == p && (*i)->GetType() == t)
+    if ((*i)->profile() == p && (*i)->type() == t)
       return *i;
   }
   return NULL;
@@ -197,7 +197,7 @@ size_t BrowserList::GetBrowserCountForType(Profile* p, BrowserType::Type type) {
   BrowserList::const_iterator i;
   size_t result = 0;
   for (i = BrowserList::begin(); i != BrowserList::end(); ++i) {
-    if ((*i)->profile() == p && (*i)->GetType() == type)
+    if ((*i)->profile() == p && (*i)->type() == type)
       result++;
   }
   return result;
diff --git a/chrome/browser/session_restore.cc b/chrome/browser/session_restore.cc
index 5b981c3..057b46b 100644
--- a/chrome/browser/session_restore.cc
+++ b/chrome/browser/session_restore.cc
@@ -297,7 +297,7 @@ class SessionRestoreImpl : public NotificationObserver {
         // If there is an open tabbed browser window, use it. Otherwise fall
         // through and create a new one.
         browser = current_browser;
-        if (browser && (browser->GetType() != BrowserType::TABBED_BROWSER ||
+        if (browser && (browser->type() != BrowserType::TABBED_BROWSER ||
                         browser->profile()->IsOffTheRecord())) {
           browser = NULL;
         }
@@ -320,7 +320,7 @@ class SessionRestoreImpl : public NotificationObserver {
     // included at least one tabbed browser, then close the browser window
     // that was opened when the user clicked to restore the session.
     if (clobber_existing_window_ && current_browser && has_tabbed_browser &&
-        current_browser->GetType() == BrowserType::TABBED_BROWSER) {
+        current_browser->type() == BrowserType::TABBED_BROWSER) {
       current_browser->CloseAllTabs();
     }
     if (last_browser && !urls_to_open_.empty())
diff --git a/chrome/browser/session_service.cc b/chrome/browser/session_service.cc
index 813b56a..d1b848b 100644
--- a/chrome/browser/session_service.cc
+++ b/chrome/browser/session_service.cc
@@ -476,7 +476,7 @@ void SessionService::Observe(NotificationType type,
     case NOTIFY_BROWSER_OPENED: {
       Browser* browser = Source<Browser>(source).ptr();
       if (browser->profile() != profile_ ||
-          !should_track_changes_for_browser_type(browser->GetType())) {
+          !should_track_changes_for_browser_type(browser->type())) {
         return;
       }
 
@@ -494,7 +494,7 @@ void SessionService::Observe(NotificationType type,
               profile_, browser, false, false, false, std::vector<GURL>());
         }
       }
-      SetWindowType(browser->session_id(), browser->GetType());
+      SetWindowType(browser->session_id(), browser->type());
       break;
     }
 
@@ -1092,7 +1092,7 @@ void SessionService::BuildCommandsForBrowser(
                                    browser->window()->IsMaximized()));
 
   commands->push_back(CreateSetWindowTypeCommand(
-      browser->session_id(), browser->GetType()));
+      browser->session_id(), browser->type()));
 
   bool added_to_windows_to_track = false;
   for (int i = 0; i < browser->tab_count(); ++i) {
@@ -1119,7 +1119,7 @@ void SessionService::BuildCommandsFromBrowsers(
   DCHECK(commands);
   for (BrowserList::const_iterator i = BrowserList::begin();
        i != BrowserList::end(); ++i) {
-    if (should_track_changes_for_browser_type((*i)->GetType())) {
+    if (should_track_changes_for_browser_type((*i)->type())) {
       BuildCommandsForBrowser(*i, commands, tab_to_available_range,
                               windows_to_track);
     }
@@ -1258,7 +1258,7 @@ bool SessionService::IsOnlyOneTabLeft() {
   for (BrowserList::const_iterator i = BrowserList::begin();
        i != BrowserList::end(); ++i) {
     const SessionID::id_type window_id = (*i)->session_id().id();
-    if (should_track_changes_for_browser_type((*i)->GetType()) &&
+    if (should_track_changes_for_browser_type((*i)->type()) &&
         (*i)->profile()->GetOriginalProfile() == profile_ &&
         window_closing_ids_.find(window_id) == window_closing_ids_.end()) {
       if (++window_count > 1)
@@ -1286,7 +1286,7 @@ bool SessionService::HasOpenTabbedBrowsers(const SessionID& window_id) {
     const SessionID::id_type browser_id = browser->session_id().id();
     if (browser_id != window_id.id() &&
         window_closing_ids_.find(browser_id) == window_closing_ids_.end() &&
-        should_track_changes_for_browser_type(browser->GetType()) &&
+        should_track_changes_for_browser_type(browser->type()) &&
         browser->profile()->GetOriginalProfile() == profile_) {
       return true;
     }
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index f8f8aa1..5f5a0bc 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -601,7 +601,7 @@ bool TabStripModel::ShouldAddToTabRestoreService(TabContents* contents) {
       Browser::GetBrowserForController(contents->controller(), NULL);
   if (!browser)
     return false; // Browser is null during unit tests.
-  return browser->GetType() == BrowserType::TABBED_BROWSER;
+  return browser->type() == BrowserType::TABBED_BROWSER;
 }
 
 // static
diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc
index 8cc8ead..5a992b8 100644
--- a/chrome/browser/views/frame/browser_view.cc
+++ b/chrome/browser/views/frame/browser_view.cc
@@ -154,8 +154,7 @@ bool BrowserView::IsOffTheRecord() const {
 }
 
 bool BrowserView::ShouldShowOffTheRecordAvatar() const {
-  return IsOffTheRecord() &&
-      browser_->GetType() == BrowserType::TABBED_BROWSER;
+  return IsOffTheRecord() && browser_->type() == BrowserType::TABBED_BROWSER;
 }
 
 bool BrowserView::AcceleratorPressed(const views::Accelerator& accelerator) {
@@ -246,7 +245,7 @@ void BrowserView::PrepareToRunSystemMenu(HMENU menu) {
 }
 
 bool BrowserView::SupportsWindowFeature(WindowFeature feature) const {
-  return !!(FeaturesForBrowserType(browser_->GetType()) & feature);
+  return !!(FeaturesForBrowserType(browser_->type()) & feature);
 }
 
 // static
@@ -517,7 +516,7 @@ bool BrowserView::ShouldShowWindowTitle() const {
 }
 
 SkBitmap BrowserView::GetWindowIcon() {
-  if (browser_->GetType() == BrowserType::APPLICATION)
+  if (browser_->type() == BrowserType::APPLICATION)
     return browser_->GetCurrentPageIcon();
   return SkBitmap();
 }
@@ -553,7 +552,7 @@ bool BrowserView::RestoreWindowPosition(CRect* bounds,
   DCHECK(bounds && maximized && always_on_top);
   *always_on_top = false;
 
-  if (browser_->GetType() == BrowserType::BROWSER) {
+  if (browser_->type() == BrowserType::BROWSER) {
     // We are a popup window. The value passed in |bounds| represents two
     // pieces of information:
     // - the position of the window, in screen coordinates (outer position).
@@ -795,7 +794,7 @@ void BrowserView::InitSystemMenu() {
   int insertion_index = std::max(0, system_menu_->ItemCount() - 1);
   // We add the menu items in reverse order so that insertion_index never needs
   // to change.
-  if (browser_->GetType() == BrowserType::TABBED_BROWSER) {
+  if (browser_->type() == BrowserType::TABBED_BROWSER) {
     system_menu_->AddSeparator(insertion_index);
     system_menu_->AddMenuItemWithLabel(insertion_index, IDC_TASKMANAGER,
                                        l10n_util::GetString(IDS_TASKMANAGER));
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index 589fa98..34f6c24 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -80,7 +80,7 @@ BrowserToolbarView::BrowserToolbarView(CommandController* controller,
   forward_menu_model_.reset(new BackForwardMenuModel(
       browser, BackForwardMenuModel::FORWARD_MENU_DELEGATE));
 
-  if (browser->GetType() == BrowserType::TABBED_BROWSER)
+  if (browser->type() == BrowserType::TABBED_BROWSER)
     display_mode_ = DISPLAYMODE_NORMAL;
   else
     display_mode_ = DISPLAYMODE_LOCATION;
diff --git a/chrome/browser/window_sizer.cc b/chrome/browser/window_sizer.cc
index 8535277..634fe9c 100644
--- a/chrome/browser/window_sizer.cc
+++ b/chrome/browser/window_sizer.cc
@@ -140,8 +140,7 @@ class DefaultStateProvider : public WindowSizer::StateProvider {
     BrowserList::const_reverse_iterator end = BrowserList::end_last_active();
     for (; it != end; ++it) {
       Browser* last_active = *it;
-      if (last_active &&
-          last_active->GetType() == BrowserType::TABBED_BROWSER) {
+      if (last_active && last_active->type() == BrowserType::TABBED_BROWSER) {
         BrowserWindow* frame = last_active->window();
         DCHECK(frame);
         *bounds = frame->GetNormalBounds();