From 999aa609b3fc8a9bc14d0d39d34890cf1df1160c Mon Sep 17 00:00:00 2001
From: "ben@chromium.org"
<ben@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 11 Nov 2008 21:57:56 +0000
Subject: 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
---
chrome/browser/browser.cc | 13 +++---
chrome/browser/browser.h | 34 +++------------
chrome/browser/browser_commands.cc | 68 +++---------------------------
chrome/browser/browser_init.cc | 2 +-
chrome/browser/browser_list.cc | 6 +--
chrome/browser/session_restore.cc | 4 +-
chrome/browser/session_service.cc | 12 +++---
chrome/browser/tabs/tab_strip_model.cc | 2 +-
chrome/browser/views/frame/browser_view.cc | 11 +++--
chrome/browser/views/toolbar_view.cc | 2 +-
chrome/browser/window_sizer.cc | 3 +-
11 files changed, 36 insertions(+), 121 deletions(-)
(limited to 'chrome/browser')
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();
--
cgit v1.1