0

Don't expose the go and star button views directly through the BrowserWindow interface (for porting).

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@8572 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
ben@chromium.org
2009-01-23 20:37:29 +00:00
parent a3e4dd79aa
commit b7ca4e69c1
13 changed files with 103 additions and 115 deletions

@ -59,11 +59,9 @@
#include "chrome/browser/user_data_manager.h"
#include "chrome/browser/view_ids.h"
#include "chrome/browser/views/download_tab_view.h"
#include "chrome/browser/views/go_button.h"
#include "chrome/browser/views/location_bar_view.h"
#include "chrome/browser/views/new_profile_dialog.h"
#include "chrome/browser/views/select_profile_dialog.h"
#include "chrome/browser/views/toolbar_star_toggle.h"
#include "chrome/browser/window_sizer.h"
#include "chrome/common/l10n_util.h"
#include "chrome/common/win_util.h"
@ -732,40 +730,21 @@ void Browser::Exit() {
void Browser::BookmarkCurrentPage() {
UserMetrics::RecordAction(L"Star", profile_);
TabContents* tab = GetSelectedTabContents();
if (!tab->AsWebContents())
return;
WebContents* rvh = tab->AsWebContents();
BookmarkModel* model = tab->profile()->GetBookmarkModel();
TabContents* contents = GetSelectedTabContents();
BookmarkModel* model = contents->profile()->GetBookmarkModel();
if (!model || !model->IsLoaded())
return; // Ignore requests until bookmarks are loaded.
NavigationEntry* entry = rvh->controller()->GetActiveEntry();
NavigationEntry* entry = contents->controller()->GetActiveEntry();
if (!entry)
return; // Can't star if there is no URL.
const GURL& url = entry->display_url();
if (url.is_empty() || !url.is_valid())
return;
if (window_->GetStarButton()) {
if (!window_->GetStarButton()->is_bubble_showing()) {
const bool newly_bookmarked = !model->IsBookmarked(url);
if (newly_bookmarked) {
model->SetURLStarred(url, entry->title(), true);
if (!model->IsBookmarked(url)) {
// Starring failed. This shouldn't happen.
NOTREACHED();
return;
}
}
window_->GetStarButton()->ShowStarBubble(url, newly_bookmarked);
}
} else if (model->IsBookmarked(url)) {
// If we can't find the star button and the user wanted to unstar it,
// go ahead and unstar it without showing the bubble.
model->SetURLStarred(url, std::wstring(), false);
}
model->SetURLStarred(url, entry->title(), true);
if (!window_->IsBookmarkBubbleVisible())
window_->ShowBookmarkBubble(url, model->IsBookmarked(url));
}
void Browser::ViewSource() {
@ -1766,7 +1745,7 @@ void Browser::ToolbarSizeChanged(TabContents* source, bool is_animating) {
void Browser::URLStarredChanged(TabContents* source, bool starred) {
if (source == GetSelectedTabContents())
SetStarredButtonToggled(starred);
window_->SetStarredState(starred);
}
void Browser::ContentsMouseEvent(TabContents* source, UINT message) {
@ -2075,7 +2054,7 @@ void Browser::UpdateCommandsForTabState() {
// Only allow bookmarking for tabbed browsers.
command_updater_.UpdateCommandEnabled(IDC_STAR,
is_web_contents && (type() == TYPE_NORMAL));
SetStarredButtonToggled(is_web_contents && web_contents->is_starred());
window_->SetStarredState(is_web_contents && web_contents->is_starred());
// View-source should not be enabled if already in view-source mode.
command_updater_.UpdateCommandEnabled(IDC_VIEW_SOURCE,
is_web_contents && (current_tab->type() != TAB_CONTENTS_VIEW_SOURCE) &&
@ -2107,19 +2086,11 @@ void Browser::UpdateCommandsForTabState() {
}
void Browser::UpdateStopGoState(bool is_loading) {
GoButton* go_button = GetGoButton();
if (go_button)
go_button->ChangeMode(is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO);
window_->UpdateStopGoState(is_loading);
command_updater_.UpdateCommandEnabled(IDC_GO, !is_loading);
command_updater_.UpdateCommandEnabled(IDC_STOP, is_loading);
}
void Browser::SetStarredButtonToggled(bool starred) {
ToolbarStarToggle* star_button = window_->GetStarButton();
if (star_button)
star_button->SetToggled(starred);
}
#if defined(OS_WIN)
///////////////////////////////////////////////////////////////////////////////
@ -2259,10 +2230,6 @@ LocationBarView* Browser::GetLocationBarView() const {
return window_->GetLocationBarView();
}
GoButton* Browser::GetGoButton() {
return window_->GetGoButton();
}
StatusBubble* Browser::GetStatusBubble() {
return window_->GetStatusBubble();
}

@ -426,10 +426,6 @@ class Browser : public TabStripModelDelegate,
// |is_loading| is true if the current TabContents is loading.
void UpdateStopGoState(bool is_loading);
// Change the "starred" button display to starred/unstarred.
// TODO(evanm): migrate this to the commands framework.
void SetStarredButtonToggled(bool starred);
#if defined(OS_WIN)
// UI update coalescing and handling ////////////////////////////////////////

@ -3,11 +3,13 @@
// found in the LICENSE file.
#include "chrome/app/chrome_dll_resource.h"
#include "chrome/browser/bookmarks/bookmark_model.h"
#include "chrome/browser/browser.h"
#include "chrome/browser/browser_list.h"
#include "chrome/browser/tab_contents/navigation_controller.h"
#include "chrome/browser/tab_contents/navigation_entry.h"
#include "chrome/test/browser_with_test_window_test.h"
#include "chrome/test/testing_profile.h"
typedef BrowserWithTestWindowTest BrowserCommandsTest;
@ -72,3 +74,26 @@ TEST_F(BrowserCommandsTest, DuplicateTab) {
ASSERT_TRUE(url2 == controller->GetEntryAtIndex(1)->url());
ASSERT_TRUE(url3 == controller->GetEntryAtIndex(2)->url());
}
TEST_F(BrowserCommandsTest, BookmarkCurrentPage) {
// We use profile() here, since it's a TestingProfile.
profile()->CreateBookmarkModel(true);
profile()->BlockUntilBookmarkModelLoaded();
// Navigate to a url.
GURL url1 = test_url_with_path("1");
AddTestingTab(browser());
browser()->OpenURL(url1, GURL(), CURRENT_TAB, PageTransition::TYPED);
// TODO(beng): remove this once we can use WebContentses directly in testing
// instead of the TestTabContents which causes this command not to
// be enabled when the tab is added (and selected).
browser()->command_updater()->UpdateCommandEnabled(IDC_STAR, true);
// Star it.
browser()->ExecuteCommand(IDC_STAR);
// It should now be bookmarked in the bookmark model.
EXPECT_EQ(profile(), browser()->profile());
EXPECT_TRUE(browser()->profile()->GetBookmarkModel()->IsBookmarked(url1));
}

@ -10,14 +10,12 @@ class Browser;
class BrowserList;
class BrowserView;
class BrowserWindowTesting;
class GoButton;
class GURL;
class LocationBarView;
class HtmlDialogContentsDelegate;
class Profile;
class StatusBubble;
class TabContents;
class TabStrip;
class ToolbarStarToggle;
namespace gfx {
class Rect;
@ -59,10 +57,6 @@ class BrowserWindow {
// if there is none.
virtual BrowserWindowTesting* GetBrowserWindowTesting() = 0;
// TODO(beng): REMOVE (obtain via BrowserFrame).
// Return the TabStrip associated with the frame.
virtual TabStrip* GetTabStrip() const = 0;
// Return the status bubble associated with the frame
virtual StatusBubble* GetStatusBubble() = 0;
@ -82,6 +76,9 @@ class BrowserWindow {
// if there are no active loads and the animations should end.
virtual void UpdateLoadingAnimations(bool should_animate) = 0;
// Sets the starred state for the current tab.
virtual void SetStarredState(bool is_starred) = 0;
// TODO(beng): RENAME (GetRestoredBounds)
// Returns the nonmaximized bounds of the frame (even if the frame is
// currently maximized or minimized) in terms of the screen coordinates.
@ -91,14 +88,12 @@ class BrowserWindow {
// Returns true if the frame is maximized (aka zoomed).
virtual bool IsMaximized() = 0;
// Returns the star button.
virtual ToolbarStarToggle* GetStarButton() const = 0;
// Returns the location bar.
virtual LocationBarView* GetLocationBarView() const = 0;
// Returns the go button.
virtual GoButton* GetGoButton() const = 0;
// Informs the view whether or not a load is in progress for the current tab.
// The view can use this notification to update the go/stop button.
virtual void UpdateStopGoState(bool is_loading) = 0;
// Updates the toolbar with the state for the specified |contents|.
virtual void UpdateToolbar(TabContents* contents,
@ -119,6 +114,13 @@ class BrowserWindow {
// Shows the Bookmark Manager window.
virtual void ShowBookmarkManager() = 0;
// Returns true if the Bookmark bubble is visible.
virtual bool IsBookmarkBubbleVisible() const = 0;
// Shows the Bookmark bubble. |url| is the URL being bookmarked,
// |already_bookmarked| is true if the url is already bookmarked.
virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked) = 0;
// Shows the Report a Bug dialog box.
virtual void ShowReportBugDialog() = 0;

@ -28,17 +28,16 @@ class BrowserWindowCocoa : public BrowserWindow {
virtual void FlashFrame();
virtual void* GetNativeHandle();
virtual BrowserWindowTesting* GetBrowserWindowTesting();
virtual TabStrip* GetTabStrip() const;
virtual StatusBubble* GetStatusBubble();
virtual void SelectedTabToolbarSizeChanged(bool is_animating);
virtual void UpdateTitleBar();
virtual void UpdateLoadingAnimations(bool should_animate);
virtual void SetStarredState(bool is_starred);
virtual gfx::Rect GetNormalBounds() const;
virtual bool IsMaximized();
virtual ToolbarStarToggle* GetStarButton() const;
virtual LocationBarView* GetLocationBarView() const;
virtual GoButton* GetGoButton() const;
virtual BookmarkBarView* GetBookmarkBarView();
virtual void UpdateStopGoState(bool is_loading);
virtual void UpdateToolbar(TabContents* contents,
bool should_restore_state);
virtual void FocusToolbar();
@ -46,6 +45,8 @@ class BrowserWindowCocoa : public BrowserWindow {
virtual void ToggleBookmarkBar();
virtual void ShowAboutChromeDialog();
virtual void ShowBookmarkManager();
virtual bool IsBookmarkBubbleVisible() const;
virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked);
virtual void ShowReportBugDialog();
virtual void ShowClearBrowsingDataDialog();
virtual void ShowImportDialog();

@ -51,10 +51,6 @@ BrowserWindowTesting* BrowserWindowCocoa::GetBrowserWindowTesting() {
return NULL;
}
TabStrip* BrowserWindowCocoa::GetTabStrip() const {
return NULL;
}
StatusBubble* BrowserWindowCocoa::GetStatusBubble() {
return NULL;
}
@ -68,6 +64,9 @@ void BrowserWindowCocoa::UpdateTitleBar() {
void BrowserWindowCocoa::UpdateLoadingAnimations(bool should_animate) {
}
void BrowserWindowCocoa::SetStarredState(bool is_starred) {
}
gfx::Rect BrowserWindowCocoa::GetNormalBounds() const {
// TODO(pinkerton): not sure if we can get the non-zoomed bounds, or if it
// really matters. We may want to let Cocoa handle all this for us.
@ -82,22 +81,17 @@ bool BrowserWindowCocoa::IsMaximized() {
return [window_ isZoomed];
}
ToolbarStarToggle* BrowserWindowCocoa::GetStarButton() const {
return NULL;
}
LocationBarView* BrowserWindowCocoa::GetLocationBarView() const {
return NULL;
}
GoButton* BrowserWindowCocoa::GetGoButton() const {
return NULL;
}
BookmarkBarView* BrowserWindowCocoa::GetBookmarkBarView() {
return NULL;
}
void BrowserWindowCocoa::UpdateStopGoState(bool is_loading) {
}
void BrowserWindowCocoa::UpdateToolbar(TabContents* contents,
bool should_restore_state) {
}
@ -118,6 +112,14 @@ void BrowserWindowCocoa::ShowAboutChromeDialog() {
void BrowserWindowCocoa::ShowBookmarkManager() {
}
bool BrowserWindowCocoa::IsBookmarkBubbleVisible() const {
return false;
}
void BrowserWindowCocoa::ShowBookmarkBubble(const GURL& url,
bool already_bookmarked) {
}
void BrowserWindowCocoa::ShowReportBugDialog() {
}

@ -264,7 +264,7 @@ class DockToWindowFinder : public BaseWindowFinder {
protected:
virtual bool ShouldStopIterating(HWND hwnd) {
BrowserWindow* window = BrowserView::GetBrowserWindowForHWND(hwnd);
BrowserView* window = BrowserView::GetBrowserViewForHWND(hwnd);
CRect bounds;
if (!window || !::IsWindowVisible(hwnd) ||
!::GetWindowRect(hwnd, &bounds)) {

@ -30,6 +30,7 @@
#include "chrome/browser/views/status_bubble_views.h"
#include "chrome/browser/views/tab_contents_container_view.h"
#include "chrome/browser/views/tabs/tab_strip.h"
#include "chrome/browser/views/toolbar_star_toggle.h"
#include "chrome/browser/views/toolbar_view.h"
#include "chrome/browser/tab_contents/navigation_entry.h"
#include "chrome/browser/tab_contents/web_contents.h"
@ -70,7 +71,7 @@ static const int kStatusBubbleVerticalOverlap = 2;
static const int kSeparationLineHeight = 1;
// The name of a key to store on the window handle so that other code can
// locate this object using just the handle.
static const wchar_t* kBrowserWindowKey = L"__BROWSER_WINDOW__";
static const wchar_t* kBrowserViewKey = L"__BROWSER_VIEW__";
// The distance between tiled windows.
static const int kWindowTilePixels = 10;
// How frequently we check for hung plugin windows.
@ -140,11 +141,11 @@ BrowserView::~BrowserView() {
}
// static
BrowserWindow* BrowserView::GetBrowserWindowForHWND(HWND window) {
BrowserView* BrowserView::GetBrowserViewForHWND(HWND window) {
if (IsWindow(window)) {
HANDLE data = GetProp(window, kBrowserWindowKey);
HANDLE data = GetProp(window, kBrowserViewKey);
if (data)
return reinterpret_cast<BrowserWindow*>(data);
return reinterpret_cast<BrowserView*>(data);
}
return NULL;
}
@ -323,7 +324,7 @@ void BrowserView::RegisterBrowserViewPrefs(PrefService* prefs) {
void BrowserView::Init() {
// Stow a pointer to this object onto the window handle so that we can get
// at it later when all we have is a HWND.
SetProp(GetWidget()->GetHWND(), kBrowserWindowKey, this);
SetProp(GetWidget()->GetHWND(), kBrowserViewKey, this);
// Start a hung plugin window detector for this browser object (as long as
// hang detection is not disabled).
@ -423,10 +424,6 @@ BrowserWindowTesting* BrowserView::GetBrowserWindowTesting() {
return this;
}
TabStrip* BrowserView::GetTabStrip() const {
return tabstrip_;
}
StatusBubble* BrowserView::GetStatusBubble() {
return status_bubble_.get();
}
@ -465,6 +462,10 @@ void BrowserView::UpdateLoadingAnimations(bool should_animate) {
}
}
void BrowserView::SetStarredState(bool is_starred) {
toolbar_->star_button()->SetToggled(is_starred);
}
gfx::Rect BrowserView::GetNormalBounds() const {
WINDOWPLACEMENT wp;
wp.length = sizeof(wp);
@ -477,22 +478,19 @@ bool BrowserView::IsMaximized() {
return frame_->GetWindow()->IsMaximized();
}
ToolbarStarToggle* BrowserView::GetStarButton() const {
return toolbar_->star_button();
}
LocationBarView* BrowserView::GetLocationBarView() const {
return toolbar_->GetLocationBarView();
}
GoButton* BrowserView::GetGoButton() const {
return toolbar_->GetGoButton();
}
BrowserView* BrowserView::GetBrowserView() const {
return NULL;
}
void BrowserView::UpdateStopGoState(bool is_loading) {
toolbar_->GetGoButton()->ChangeMode(
is_loading ? GoButton::MODE_STOP : GoButton::MODE_GO);
}
void BrowserView::UpdateToolbar(TabContents* contents,
bool should_restore_state) {
toolbar_->Update(contents, should_restore_state);
@ -538,6 +536,14 @@ void BrowserView::ShowBookmarkManager() {
BookmarkManagerView::Show(browser_->profile());
}
bool BrowserView::IsBookmarkBubbleVisible() const {
return toolbar_->star_button()->is_bubble_showing();
}
void BrowserView::ShowBookmarkBubble(const GURL& url, bool already_bookmarked) {
toolbar_->star_button()->ShowStarBubble(url, !already_bookmarked);
}
void BrowserView::ShowReportBugDialog() {
// Retrieve the URL for the current tab (if any) and tell the BugReportView
TabContents* current_tab = browser_->GetSelectedTabContents();

@ -45,10 +45,10 @@ class BrowserView : public BrowserWindow,
void set_frame(BrowserFrame* frame) { frame_ = frame; }
// Returns a pointer to the BrowserWindow* interface implementation (an
// Returns a pointer to the BrowserView* interface implementation (an
// instance of this object, typically) for a given HWND, or NULL if there is
// no such association.
static BrowserWindow* GetBrowserWindowForHWND(HWND window);
static BrowserView* GetBrowserViewForHWND(HWND window);
// Returns the show flag that should be used to show the frame containing
// this view.
@ -155,17 +155,16 @@ class BrowserView : public BrowserWindow,
virtual void FlashFrame();
virtual void* GetNativeHandle();
virtual BrowserWindowTesting* GetBrowserWindowTesting();
virtual TabStrip* GetTabStrip() const;
virtual StatusBubble* GetStatusBubble();
virtual void SelectedTabToolbarSizeChanged(bool is_animating);
virtual void UpdateTitleBar();
virtual void UpdateLoadingAnimations(bool should_animate);
virtual void SetStarredState(bool is_starred);
virtual gfx::Rect GetNormalBounds() const;
virtual bool IsMaximized();
virtual ToolbarStarToggle* GetStarButton() const;
virtual LocationBarView* GetLocationBarView() const;
virtual GoButton* GetGoButton() const;
virtual BrowserView* GetBrowserView() const;
virtual void UpdateStopGoState(bool is_loading);
virtual void UpdateToolbar(TabContents* contents, bool should_restore_state);
virtual void FocusToolbar();
virtual void DestroyBrowser();
@ -173,6 +172,8 @@ class BrowserView : public BrowserWindow,
virtual void ToggleBookmarkBar();
virtual void ShowAboutChromeDialog();
virtual void ShowBookmarkManager();
virtual bool IsBookmarkBubbleVisible() const;
virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked);
virtual void ShowReportBugDialog();
virtual void ShowClearBrowsingDataDialog();
virtual void ShowImportDialog();

@ -91,7 +91,7 @@ void InfoBubble::Init(HWND parent_hwnd,
// We should always have a frame, but there was a bug elsewhere that
// made it possible for the frame to be NULL, so we have the check. If
// you hit this, file a bug.
DCHECK(BrowserView::GetBrowserWindowForHWND(owning_frame_hwnd));
DCHECK(BrowserView::GetBrowserViewForHWND(owning_frame_hwnd));
parent_ = reinterpret_cast<views::Window*>(win_util::GetWindowUserData(
owning_frame_hwnd));
parent_->DisableInactiveRendering(true);

@ -690,11 +690,11 @@ TabStrip* DraggedTabController::GetTabStripForPoint(
dock_windows_.erase(dragged_hwnd);
if (!local_window)
return NULL;
BrowserWindow* browser = BrowserView::GetBrowserWindowForHWND(local_window);
BrowserView* browser = BrowserView::GetBrowserViewForHWND(local_window);
if (!browser)
return NULL;
TabStrip* other_tabstrip = browser->GetTabStrip();
TabStrip* other_tabstrip = browser->tabstrip();
if (!other_tabstrip->IsCompatibleWith(source_tabstrip_))
return NULL;
return GetTabStripIfItContains(other_tabstrip, screen_point);

@ -249,17 +249,6 @@ class LocationBarView {
void ShowFirstRunBubble() { }
};
class GoButton {
public:
typedef enum Mode { MODE_GO = 0, MODE_STOP };
void ChangeMode(Mode mode) { }
};
class ToolbarStarToggle {
public:
void SetToggled(bool) { }
};
class DebuggerWindow : public base::RefCountedThreadSafe<DebuggerWindow> {
public:
};

@ -27,19 +27,16 @@ class TestBrowserWindow : public BrowserWindow {
virtual void FlashFrame() {}
virtual void* GetNativeHandle() { return NULL; }
virtual BrowserWindowTesting* GetBrowserWindowTesting() { return NULL; }
virtual TabStrip* GetTabStrip() const {
return const_cast<TabStrip*>(&tab_strip_);
}
virtual StatusBubble* GetStatusBubble() { return NULL; }
virtual void SelectedTabToolbarSizeChanged(bool is_animating) {}
virtual void UpdateTitleBar() {}
virtual void UpdateLoadingAnimations(bool should_animate) {}
virtual void SetStarredState(bool is_starred) {}
virtual gfx::Rect GetNormalBounds() const { return gfx::Rect(); }
virtual bool IsMaximized() { return false; }
virtual ToolbarStarToggle* GetStarButton() const { return NULL; }
virtual LocationBarView* GetLocationBarView() const { return NULL; }
virtual GoButton* GetGoButton() const { return NULL; }
virtual BookmarkBarView* GetBookmarkBarView() { return NULL; }
virtual void UpdateStopGoState(bool is_loading) {}
virtual void UpdateToolbar(TabContents* contents,
bool should_restore_state) {}
virtual void FocusToolbar() {}
@ -47,6 +44,8 @@ class TestBrowserWindow : public BrowserWindow {
virtual void ToggleBookmarkBar() {}
virtual void ShowAboutChromeDialog() {}
virtual void ShowBookmarkManager() {}
virtual bool IsBookmarkBubbleVisible() const { return false; }
virtual void ShowBookmarkBubble(const GURL& url, bool already_bookmarked) {}
virtual void ShowReportBugDialog() {}
virtual void ShowClearBrowsingDataDialog() {}
virtual void ShowImportDialog() {}