diff options
| -rw-r--r-- | chrome/browser/cocoa/html_dialog_window_controller.h | 1 | ||||
| -rw-r--r-- | chrome/browser/cocoa/html_dialog_window_controller.mm | 123 | ||||
| -rw-r--r-- | chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc | 107 | ||||
| -rw-r--r-- | chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h | 66 | ||||
| -rw-r--r-- | chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc | 158 | ||||
| -rw-r--r-- | chrome/browser/gtk/browser_window_gtk.cc | 2 | ||||
| -rw-r--r-- | chrome/browser/gtk/html_dialog_gtk.cc | 82 | ||||
| -rw-r--r-- | chrome/browser/gtk/html_dialog_gtk.h | 40 | ||||
| -rw-r--r-- | chrome/browser/views/html_dialog_view.cc | 72 | ||||
| -rw-r--r-- | chrome/browser/views/html_dialog_view.h | 36 | ||||
| -rwxr-xr-x | chrome/chrome_browser.gypi | 2 | ||||
| -rwxr-xr-x | chrome/chrome_tests.gypi | 1 |
12 files changed, 387 insertions, 303 deletions
diff --git a/chrome/browser/cocoa/html_dialog_window_controller.h b/chrome/browser/cocoa/html_dialog_window_controller.h index 26b761d..c8e3884 100644 --- a/chrome/browser/cocoa/html_dialog_window_controller.h +++ b/chrome/browser/cocoa/html_dialog_window_controller.h @@ -22,7 +22,6 @@ class TabContents; // from a HTMLDialogUIDelegate object. @interface HtmlDialogWindowController : NSWindowController { @private - Profile* profile_; // weak. Always a non-incognito profile. // Order here is important, as tab_contents_ may send messages to // delegate_ when it gets destroyed. scoped_ptr<HtmlDialogWindowDelegateBridge> delegate_; diff --git a/chrome/browser/cocoa/html_dialog_window_controller.mm b/chrome/browser/cocoa/html_dialog_window_controller.mm index c82aca3..6e880d1 100644 --- a/chrome/browser/cocoa/html_dialog_window_controller.mm +++ b/chrome/browser/cocoa/html_dialog_window_controller.mm @@ -8,23 +8,21 @@ #include "base/logging.h" #include "base/scoped_nsobject.h" #include "base/sys_string_conversions.h" -#include "chrome/browser/browser.h" -#include "chrome/browser/browser_window.h" #import "chrome/browser/cocoa/browser_command_executor.h" #import "chrome/browser/cocoa/chrome_event_processing_window.h" #include "chrome/browser/dom_ui/html_dialog_ui.h" +#include "chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h" #include "chrome/browser/profile.h" #include "chrome/browser/tab_contents/tab_contents.h" -#include "chrome/browser/tab_contents/tab_contents_delegate.h" -#include "googleurl/src/gurl.h" // Thin bridge that routes notifications to // HtmlDialogWindowController's member variables. class HtmlDialogWindowDelegateBridge : public HtmlDialogUIDelegate, - public TabContentsDelegate { + public HtmlDialogTabContentsDelegate { public: // All parameters must be non-NULL/non-nil. HtmlDialogWindowDelegateBridge(HtmlDialogWindowController* controller, + Profile* profile, HtmlDialogUIDelegate* delegate); virtual ~HtmlDialogWindowDelegateBridge(); @@ -43,33 +41,9 @@ public: virtual std::string GetDialogArgs() const; virtual void OnDialogClosed(const std::string& json_retval); - // TabContentsDelegate declarations. - // - // TODO(akalin): decomp this out into a separate platform-independent class - // so they don't diverge in behavior. In particular, - // ShouldAddNavigationToHistory() should return false on all platforms but - // currently does so only on OS X. This is tracked in - // http://code.google.com/p/chromium/issues/detail?id=28609 . - virtual void OpenURLFromTab(TabContents* source, - const GURL& url, const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition); - virtual void NavigationStateChanged(const TabContents* source, - unsigned changed_flags); - virtual void AddNewContents(TabContents* source, - TabContents* new_contents, - WindowOpenDisposition disposition, - const gfx::Rect& initial_pos, - bool user_gesture); - virtual void ActivateContents(TabContents* contents); - virtual void LoadingStateChanged(TabContents* source); - virtual void CloseContents(TabContents* source); + // HtmlDialogTabContentsDelegate declarations. virtual void MoveContents(TabContents* source, const gfx::Rect& pos); - virtual bool IsPopup(TabContents* source); virtual void ToolbarSizeChanged(TabContents* source, bool is_animating); - virtual void URLStarredChanged(TabContents* source, bool starred); - virtual void UpdateTargetURL(TabContents* source, const GURL& url); - virtual bool ShouldAddNavigationToHistory() const; private: HtmlDialogWindowController* controller_; // weak @@ -88,8 +62,6 @@ private: // BrowserCommandExecutor protocol. @interface HtmlDialogWindowController (InternalAPI) <BrowserCommandExecutor> -- (Profile*)profile; - // BrowserCommandExecutor methods. - (void)executeCommand:(int)command; @@ -104,8 +76,10 @@ void ShowHtmlDialog(HtmlDialogUIDelegate* delegate, Profile* profile) { } // namespace html_dialog_window_controller HtmlDialogWindowDelegateBridge::HtmlDialogWindowDelegateBridge( - HtmlDialogWindowController* controller, HtmlDialogUIDelegate* delegate) - : controller_(controller), delegate_(delegate) { + HtmlDialogWindowController* controller, Profile* profile, + HtmlDialogUIDelegate* delegate) + : HtmlDialogTabContentsDelegate(profile), + controller_(controller), delegate_(delegate) { DCHECK(controller_); DCHECK(delegate_); } @@ -113,8 +87,9 @@ HtmlDialogWindowDelegateBridge::HtmlDialogWindowDelegateBridge( HtmlDialogWindowDelegateBridge::~HtmlDialogWindowDelegateBridge() {} void HtmlDialogWindowDelegateBridge::WindowControllerClosed() { - DelegateOnDialogClosed(""); + Detach(); controller_ = nil; + DelegateOnDialogClosed(""); } bool HtmlDialogWindowDelegateBridge::DelegateOnDialogClosed( @@ -175,6 +150,7 @@ std::string HtmlDialogWindowDelegateBridge::GetDialogArgs() const { void HtmlDialogWindowDelegateBridge::OnDialogClosed( const std::string& json_retval) { + Detach(); // [controller_ close] should be called at most once, too. if (DelegateOnDialogClosed(json_retval)) { [controller_ close]; @@ -182,83 +158,18 @@ void HtmlDialogWindowDelegateBridge::OnDialogClosed( controller_ = nil; } -// TabContentsDelegate definitions. Most of this logic is copied from -// chrome/browser/views/html_dialog_view.cc . All functions with empty -// bodies are notifications we don't care about. - -void HtmlDialogWindowDelegateBridge::OpenURLFromTab( - TabContents* source, const GURL& url, const GURL& referrer, - WindowOpenDisposition disposition, PageTransition::Type transition) { - if (controller_) { - // Force all links to open in a new window. Code adapted from - // Browser::OpenURLFromTab() with disposition == NEW_WINDOW. - Browser* browser = Browser::Create([controller_ profile]); - TabContents* new_contents = - browser->AddTabWithURL(url, referrer, transition, true, -1, false, NULL); - new_contents->Focus(); - } -} - -void HtmlDialogWindowDelegateBridge::NavigationStateChanged( - const TabContents* source, unsigned changed_flags) { -} - -void HtmlDialogWindowDelegateBridge::AddNewContents( - TabContents* source, TabContents* new_contents, - WindowOpenDisposition disposition, const gfx::Rect& initial_pos, - bool user_gesture) { - if (controller_) { - // Force this to open in a new window, too. Code adapted from - // Browser::AddNewContents() with disposition == NEW_WINDOW. - Browser* browser = Browser::Create([controller_ profile]); - static_cast<TabContentsDelegate*>(browser)-> - AddNewContents(source, new_contents, NEW_FOREGROUND_TAB, - initial_pos, user_gesture); - browser->window()->Show(); - } -} - -void HtmlDialogWindowDelegateBridge::ActivateContents(TabContents* contents) {} - -void HtmlDialogWindowDelegateBridge::LoadingStateChanged(TabContents* source) {} - -void HtmlDialogWindowDelegateBridge::CloseContents(TabContents* source) {} - void HtmlDialogWindowDelegateBridge::MoveContents(TabContents* source, const gfx::Rect& pos) { // TODO(akalin): Actually set the window bounds. } -bool HtmlDialogWindowDelegateBridge::IsPopup(TabContents* source) { - // This needs to return true so that we are allowed to be resized by - // our contents. - return true; -} - void HtmlDialogWindowDelegateBridge::ToolbarSizeChanged( TabContents* source, bool is_animating) { // TODO(akalin): Figure out what to do here. } -void HtmlDialogWindowDelegateBridge::URLStarredChanged( - TabContents* source, bool starred) { - // We don't have a visible star to click in the window. - NOTREACHED(); -} - -void HtmlDialogWindowDelegateBridge::UpdateTargetURL( - TabContents* source, const GURL& url) {} - -bool HtmlDialogWindowDelegateBridge::ShouldAddNavigationToHistory() const { - return false; -} - @implementation HtmlDialogWindowController (InternalAPI) -- (Profile*)profile { - return profile_; -} - // This gets called whenever a chrome-specific keyboard shortcut is performed // in the HTML dialog window. We simply swallow all those events. - (void)executeCommand:(int)command {} @@ -309,19 +220,13 @@ bool HtmlDialogWindowDelegateBridge::ShouldAddNavigationToHistory() const { [window setDelegate:self]; [window setTitle:base::SysWideToNSString(delegate->GetDialogTitle())]; [window center]; - delegate_.reset(new HtmlDialogWindowDelegateBridge(self, delegate)); - // Incognito profiles are not long-lived, so we always want to store a - // non-incognito profile. - // - // TODO(akalin): Should we make it so that we have a default incognito - // profile that's long-lived? Of course, we'd still have to clear it out - // when all incognito browsers close. - profile_ = profile->GetOriginalProfile(); + delegate_.reset(new HtmlDialogWindowDelegateBridge(self, profile, delegate)); return self; } - (void)loadDialogContents { - tabContents_.reset(new TabContents(profile_, NULL, MSG_ROUTING_NONE, NULL)); + tabContents_.reset( + new TabContents(delegate_->profile(), NULL, MSG_ROUTING_NONE, NULL)); [[self window] setContentView:tabContents_->GetNativeView()]; tabContents_->set_delegate(delegate_.get()); diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc new file mode 100644 index 0000000..411982c --- /dev/null +++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.cc @@ -0,0 +1,107 @@ +// Copyright (c) 2009 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/dom_ui/html_dialog_tab_contents_delegate.h" + +#include "chrome/browser/browser.h" +#include "chrome/browser/browser_window.h" +#include "chrome/browser/profile.h" +#include "chrome/browser/tab_contents/tab_contents.h" + +// Incognito profiles are not long-lived, so we always want to store a +// non-incognito profile. +// +// TODO(akalin): Should we make it so that we have a default incognito +// profile that's long-lived? Of course, we'd still have to clear it out +// when all incognito browsers close. +HtmlDialogTabContentsDelegate::HtmlDialogTabContentsDelegate(Profile* profile) + : profile_(profile->GetOriginalProfile()) {} + +HtmlDialogTabContentsDelegate::~HtmlDialogTabContentsDelegate() {} + +Profile* HtmlDialogTabContentsDelegate::profile() const { return profile_; } + +void HtmlDialogTabContentsDelegate::Detach() { + profile_ = NULL; +} + +Browser* HtmlDialogTabContentsDelegate::CreateBrowser() { + DCHECK(profile_); + return Browser::Create(profile_); +} + +void HtmlDialogTabContentsDelegate::OpenURLFromTab( + TabContents* source, const GURL& url, const GURL& referrer, + WindowOpenDisposition disposition, PageTransition::Type transition) { + if (profile_) { + // Force all links to open in a new window, ignoring the incoming + // disposition. This is a tabless, modal dialog so we can't just + // open it in the current frame. Code adapted from + // Browser::OpenURLFromTab() with disposition == NEW_WINDOW. + Browser* browser = CreateBrowser(); + TabContents* new_contents = + browser->AddTabWithURL(url, referrer, transition, true, -1, false, + NULL); + DCHECK(new_contents); + browser->window()->Show(); + new_contents->Focus(); + } +} + +void HtmlDialogTabContentsDelegate::NavigationStateChanged( + const TabContents* source, unsigned changed_flags) { + // We shouldn't receive any NavigationStateChanged except the first + // one, which we ignore because we're a dialog box. +} + +void HtmlDialogTabContentsDelegate::AddNewContents( + TabContents* source, TabContents* new_contents, + WindowOpenDisposition disposition, const gfx::Rect& initial_pos, + bool user_gesture) { + if (profile_) { + // Force this to open in a new window, too. Code adapted from + // Browser::AddNewContents() with disposition == NEW_WINDOW. + Browser* browser = CreateBrowser(); + static_cast<TabContentsDelegate*>(browser)-> + AddNewContents(source, new_contents, NEW_FOREGROUND_TAB, + initial_pos, user_gesture); + browser->window()->Show(); + } +} + +void HtmlDialogTabContentsDelegate::ActivateContents(TabContents* contents) { + // We don't do anything here because there's only one TabContents in + // this frame and we don't have a TabStripModel. +} + +void HtmlDialogTabContentsDelegate::LoadingStateChanged(TabContents* source) { + // We don't care about this notification. +} + +void HtmlDialogTabContentsDelegate::CloseContents(TabContents* source) { + // We receive this message but don't handle it because we really do the + // cleanup somewhere else (namely, HtmlDialogUIDelegate::OnDialogClosed()). +} + +bool HtmlDialogTabContentsDelegate::IsPopup(TabContents* source) { + // This needs to return true so that we are allowed to be resized by our + // contents. + return true; +} + +void HtmlDialogTabContentsDelegate::URLStarredChanged(TabContents* source, + bool starred) { + // We don't have a visible star to click in the window. + NOTREACHED(); +} + +void HtmlDialogTabContentsDelegate::UpdateTargetURL(TabContents* source, + const GURL& url) { + // Ignored. +} + +bool HtmlDialogTabContentsDelegate::ShouldAddNavigationToHistory() const { + return false; +} + diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h new file mode 100644 index 0000000..d827424 --- /dev/null +++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h @@ -0,0 +1,66 @@ +// Copyright (c) 2009 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. + +#ifndef CHROME_BROWSER_DOM_UI_HTML_DIALOG_TAB_CONTENTS_DELEGATE_H_ +#define CHROME_BROWSER_DOM_UI_HTML_DIALOG_TAB_CONTENTS_DELEGATE_H_ + +#include "chrome/browser/tab_contents/tab_contents_delegate.h" + +class Browser; +class Profile; + +// This class implements (and mostly ignores) most of TabContentsDelegate for +// use in an HTML dialog. Subclasses need only override a few methods instead +// of the everything from TabContentsDelegate; this way, implementations on +// all platforms behave consistently. + +class HtmlDialogTabContentsDelegate : public TabContentsDelegate { + public: + // Profile must be non-NULL. + explicit HtmlDialogTabContentsDelegate(Profile* profile); + + virtual ~HtmlDialogTabContentsDelegate(); + + // The returned profile is guaranteed to be original if non-NULL. + Profile* profile() const; + + // Calling this causes all following events sent from the + // TabContents object to be ignored. It also makes all following + // calls to profile() return NULL. + void Detach(); + + // TabContentsDelegate declarations. Subclasses of this still need to + // override: + // virtual void MoveContents(TabContents* source, const gfx::Rect& pos); + // virtual void ToolbarSizeChanged(TabContents* source, bool is_animating); + + virtual void OpenURLFromTab(TabContents* source, + const GURL& url, const GURL& referrer, + WindowOpenDisposition disposition, + PageTransition::Type transition); + virtual void NavigationStateChanged(const TabContents* source, + unsigned changed_flags); + virtual void AddNewContents(TabContents* source, + TabContents* new_contents, + WindowOpenDisposition disposition, + const gfx::Rect& initial_pos, + bool user_gesture); + virtual void ActivateContents(TabContents* contents); + virtual void LoadingStateChanged(TabContents* source); + virtual void CloseContents(TabContents* source); + virtual bool IsPopup(TabContents* source); + virtual void URLStarredChanged(TabContents* source, bool starred); + virtual void UpdateTargetURL(TabContents* source, const GURL& url); + virtual bool ShouldAddNavigationToHistory() const; + + protected: + // Overridden only for testing. + virtual Browser* CreateBrowser(); + + private: + Profile* profile_; // Weak pointer. Always an original profile. +}; + +#endif // CHROME_BROWSER_DOM_UI_HTML_DIALOG_TAB_CONTENTS_DELEGATE_H_ + diff --git a/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc new file mode 100644 index 0000000..5dfe24a0 --- /dev/null +++ b/chrome/browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc @@ -0,0 +1,158 @@ +// Copyright (c) 2009 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/dom_ui/html_dialog_tab_contents_delegate.h" + +#include <vector> + +#include "base/gfx/rect.h" +#include "base/logging.h" +#include "base/scoped_ptr.h" +#include "chrome/browser/browser.h" +#include "chrome/browser/browser_list.h" +#include "chrome/common/url_constants.h" +#include "chrome/test/browser_with_test_window_test.h" +#include "chrome/test/test_browser_window.h" +#include "googleurl/src/gurl.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { + +class MockTestBrowserWindow : public TestBrowserWindow { + public: + // TestBrowserWindow() doesn't actually use its browser argument so we just + // pass NULL. + MockTestBrowserWindow() : TestBrowserWindow(NULL) {} + + virtual ~MockTestBrowserWindow() {} + + MOCK_METHOD0(Show, void()); + + private: + DISALLOW_COPY_AND_ASSIGN(MockTestBrowserWindow); +}; + +class TestTabContentsDelegate : public HtmlDialogTabContentsDelegate { + public: + explicit TestTabContentsDelegate(Profile* profile) + : HtmlDialogTabContentsDelegate(profile), + window_for_next_created_browser_(NULL) {} + + virtual ~TestTabContentsDelegate() { + CHECK(!window_for_next_created_browser_); + for (std::vector<Browser*>::iterator i = created_browsers_.begin(); + i != created_browsers_.end(); ++i) { + (*i)->CloseAllTabs(); + // We need to explicitly cast this since BrowserWindow does *not* + // have a virtual destructor. + delete static_cast<MockTestBrowserWindow*>((*i)->window()); + delete *i; + } + } + + virtual void MoveContents(TabContents* source, const gfx::Rect& pos) {} + virtual void ToolbarSizeChanged(TabContents* source, bool is_animating) {} + + // Takes ownership of window. + void SetWindowForNextCreatedBrowser(BrowserWindow* window) { + CHECK(window); + window_for_next_created_browser_ = window; + } + + protected: + // CreateBrowser() is called by OpenURLFromTab() and AddNewContents(). + virtual Browser* CreateBrowser() { + DCHECK(profile()); + DCHECK(window_for_next_created_browser_); + Browser* browser = new Browser(Browser::TYPE_NORMAL, profile()); + browser->set_window(window_for_next_created_browser_); + window_for_next_created_browser_ = NULL; + created_browsers_.push_back(browser); + return browser; + } + + private: + BrowserWindow* window_for_next_created_browser_; + std::vector<Browser*> created_browsers_; + + DISALLOW_COPY_AND_ASSIGN(TestTabContentsDelegate); +}; + +class HtmlDialogTabContentsDelegateTest : public BrowserWithTestWindowTest { + public: + virtual void SetUp() { + BrowserWithTestWindowTest::SetUp(); + test_tab_contents_delegate_.reset(new TestTabContentsDelegate(profile())); + } + + virtual void TearDown() { + test_tab_contents_delegate_.reset(NULL); + BrowserWithTestWindowTest::TearDown(); + } + + protected: + scoped_ptr<TestTabContentsDelegate> test_tab_contents_delegate_; +}; + +TEST_F(HtmlDialogTabContentsDelegateTest, DoNothingMethodsTest) { + // None of the following calls should do anything. + EXPECT_TRUE(test_tab_contents_delegate_->IsPopup(NULL)); + EXPECT_FALSE(test_tab_contents_delegate_->ShouldAddNavigationToHistory()); + test_tab_contents_delegate_->NavigationStateChanged(NULL, 0); + test_tab_contents_delegate_->ActivateContents(NULL); + test_tab_contents_delegate_->LoadingStateChanged(NULL); + test_tab_contents_delegate_->CloseContents(NULL); + // URLStarredChanged() calls NOTREACHED() which immediately crashes. + // Death tests take too long for unit_test tests on OS X and + // there's http://code.google.com/p/chromium/issues/detail?id=24885 on + // death tests on Windows so we simply don't call it. + test_tab_contents_delegate_->UpdateTargetURL(NULL, GURL()); + test_tab_contents_delegate_->MoveContents(NULL, gfx::Rect()); + test_tab_contents_delegate_->ToolbarSizeChanged(NULL, false); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(1U, BrowserList::size()); +} + +TEST_F(HtmlDialogTabContentsDelegateTest, OpenURLFromTabTest) { + MockTestBrowserWindow* window = new MockTestBrowserWindow(); + EXPECT_CALL(*window, Show()).Times(1); + test_tab_contents_delegate_->SetWindowForNextCreatedBrowser(window); + + test_tab_contents_delegate_->OpenURLFromTab( + NULL, GURL(chrome::kAboutBlankURL), GURL(), + NEW_FOREGROUND_TAB, PageTransition::LINK); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(2U, BrowserList::size()); +} + +TEST_F(HtmlDialogTabContentsDelegateTest, AddNewContentsTest) { + MockTestBrowserWindow* window = new MockTestBrowserWindow(); + EXPECT_CALL(*window, Show()).Times(1); + test_tab_contents_delegate_->SetWindowForNextCreatedBrowser(window); + + TabContents* contents = + new TabContents(profile(), NULL, MSG_ROUTING_NONE, NULL); + test_tab_contents_delegate_->AddNewContents( + NULL, contents, NEW_FOREGROUND_TAB, gfx::Rect(), false); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(2U, BrowserList::size()); +} + +TEST_F(HtmlDialogTabContentsDelegateTest, DetachTest) { + EXPECT_EQ(profile(), test_tab_contents_delegate_->profile()); + test_tab_contents_delegate_->Detach(); + EXPECT_EQ(NULL, test_tab_contents_delegate_->profile()); + // Now, none of the following calls should do anything. + test_tab_contents_delegate_->OpenURLFromTab( + NULL, GURL(chrome::kAboutBlankURL), GURL(), + NEW_FOREGROUND_TAB, PageTransition::LINK); + test_tab_contents_delegate_->AddNewContents(NULL, NULL, NEW_FOREGROUND_TAB, + gfx::Rect(), false); + EXPECT_EQ(0, browser()->tab_count()); + EXPECT_EQ(1U, BrowserList::size()); +} + +} // namespace + diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index 1b0f7db..f9709cd 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -1167,7 +1167,7 @@ void BrowserWindowGtk::ShowThemeInstallBubble() { void BrowserWindowGtk::ShowHTMLDialog(HtmlDialogUIDelegate* delegate, gfx::NativeWindow parent_window) { - HtmlDialogGtk::ShowHtmlDialogGtk(browser_.get(), delegate); + HtmlDialogGtk::ShowHtmlDialogGtk(browser_.get(), delegate, parent_window); } void BrowserWindowGtk::UserChangedTheme() { diff --git a/chrome/browser/gtk/html_dialog_gtk.cc b/chrome/browser/gtk/html_dialog_gtk.cc index 5eca0ec..6b72742 100644 --- a/chrome/browser/gtk/html_dialog_gtk.cc +++ b/chrome/browser/gtk/html_dialog_gtk.cc @@ -6,6 +6,7 @@ #include <gtk/gtk.h> +#include "chrome/browser/browser.h" #include "chrome/browser/browser_window.h" #include "chrome/browser/dom_ui/html_dialog_ui.h" #include "chrome/browser/gtk/tab_contents_container_gtk.h" @@ -14,18 +15,22 @@ // static void HtmlDialogGtk::ShowHtmlDialogGtk(Browser* browser, - HtmlDialogUIDelegate* delegate) { - HtmlDialogGtk* html_dialog = new HtmlDialogGtk(browser, delegate); + HtmlDialogUIDelegate* delegate, + gfx::NativeWindow parent_window) { + HtmlDialogGtk* html_dialog = + new HtmlDialogGtk(browser->profile(), delegate, parent_window); html_dialog->InitDialog(); } //////////////////////////////////////////////////////////////////////////////// // HtmlDialogGtk, public: -HtmlDialogGtk::HtmlDialogGtk(Browser* parent_browser, - HtmlDialogUIDelegate* delegate) - : parent_browser_(parent_browser), +HtmlDialogGtk::HtmlDialogGtk(Profile* profile, + HtmlDialogUIDelegate* delegate, + gfx::NativeWindow parent_window) + : HtmlDialogTabContentsDelegate(profile), delegate_(delegate), + parent_window_(parent_window), dialog_(NULL) { } @@ -78,6 +83,7 @@ void HtmlDialogGtk::OnDialogClosed(const std::string& json_retval) { HtmlDialogUIDelegate* dialog_delegate = delegate_; delegate_ = NULL; // We will not communicate further with the delegate. + Detach(); dialog_delegate->OnDialogClosed(json_retval); gtk_widget_destroy(dialog_); delete this; @@ -86,82 +92,22 @@ void HtmlDialogGtk::OnDialogClosed(const std::string& json_retval) { //////////////////////////////////////////////////////////////////////////////// // TabContentsDelegate implementation: -void HtmlDialogGtk::OpenURLFromTab(TabContents* source, - const GURL& url, - const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition) { - // Force all links to open in a new window, ignoring the incoming - // disposition. This is a tabless, modal dialog so we can't just - // open it in the current frame. - static_cast<TabContentsDelegate*>(parent_browser_)->OpenURLFromTab( - source, url, referrer, NEW_WINDOW, transition); -} - -void HtmlDialogGtk::NavigationStateChanged(const TabContents* source, - unsigned changed_flags) { - // We shouldn't receive any NavigationStateChanged except the first - // one, which we ignore because we're a dialog box. -} - -void HtmlDialogGtk::ReplaceContents(TabContents* source, - TabContents* new_contents) { -} - -void HtmlDialogGtk::AddNewContents(TabContents* source, - TabContents* new_contents, - WindowOpenDisposition disposition, - const gfx::Rect& initial_pos, - bool user_gesture) { - static_cast<TabContentsDelegate*>(parent_browser_)->AddNewContents( - source, new_contents, NEW_WINDOW, initial_pos, user_gesture); -} - -void HtmlDialogGtk::ActivateContents(TabContents* contents) { - // We don't do anything here because there's only one TabContents in - // this frame and we don't have a TabStripModel. -} - -void HtmlDialogGtk::LoadingStateChanged(TabContents* source) { - // We don't care about this notification. -} - -void HtmlDialogGtk::CloseContents(TabContents* source) { - // We receive this message but don't handle it because we really do the - // cleanup in OnDialogClosed(). -} - void HtmlDialogGtk::MoveContents(TabContents* source, const gfx::Rect& pos) { // The contained web page wishes to resize itself. We let it do this because // if it's a dialog we know about, we trust it not to be mean to the user. } -bool HtmlDialogGtk::IsPopup(TabContents* source) { - // This needs to return true so that we are allowed to be resized by our - // contents. - return true; -} - void HtmlDialogGtk::ToolbarSizeChanged(TabContents* source, bool is_animating) { // Ignored. } -void HtmlDialogGtk::URLStarredChanged(TabContents* source, bool starred) { - // We don't have a visible star to click in the window. - NOTREACHED(); -} - -void HtmlDialogGtk::UpdateTargetURL(TabContents* source, const GURL& url) { - // Ignored. -} - //////////////////////////////////////////////////////////////////////////////// // HtmlDialogGtk: void HtmlDialogGtk::InitDialog() { - Profile* profile = parent_browser_->profile(); - tab_contents_.reset(new TabContents(profile, NULL, MSG_ROUTING_NONE, NULL)); + tab_contents_.reset( + new TabContents(profile(), NULL, MSG_ROUTING_NONE, NULL)); tab_contents_->set_delegate(this); // This must be done before loading the page; see the comments in @@ -177,7 +123,7 @@ void HtmlDialogGtk::InitDialog() { dialog_ = gtk_dialog_new_with_buttons( WideToUTF8(delegate_->GetDialogTitle()).c_str(), - parent_browser_->window()->GetNativeHandle(), + parent_window_, flags, NULL); diff --git a/chrome/browser/gtk/html_dialog_gtk.h b/chrome/browser/gtk/html_dialog_gtk.h index 7ec5003..0d096cf 100644 --- a/chrome/browser/gtk/html_dialog_gtk.h +++ b/chrome/browser/gtk/html_dialog_gtk.h @@ -8,25 +8,29 @@ #include <string> #include <vector> +#include "app/gfx/native_widget_types.h" #include "base/gfx/size.h" #include "base/scoped_ptr.h" -#include "chrome/browser/browser.h" #include "chrome/browser/dom_ui/html_dialog_ui.h" -#include "chrome/browser/tab_contents/tab_contents_delegate.h" +#include "chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h" typedef struct _GtkWidget GtkWidget; +class Browser; +class Profile; class TabContents; class TabContentsContainerGtk; -class HtmlDialogGtk : public TabContentsDelegate, +class HtmlDialogGtk : public HtmlDialogTabContentsDelegate, public HtmlDialogUIDelegate { public: - HtmlDialogGtk(Browser* parent_browser, HtmlDialogUIDelegate* delegate); + HtmlDialogGtk(Profile* profile, HtmlDialogUIDelegate* delegate, + gfx::NativeWindow parent_window); virtual ~HtmlDialogGtk(); static void ShowHtmlDialogGtk(Browser* browser, - HtmlDialogUIDelegate* delegate); + HtmlDialogUIDelegate* delegate, + gfx::NativeWindow parent_window); // Initializes the contents of the dialog (the DOMView and the callbacks). void InitDialog(); @@ -41,43 +45,21 @@ class HtmlDialogGtk : public TabContentsDelegate, virtual void OnDialogClosed(const std::string& json_retval); // Overridden from TabContentsDelegate: - virtual void OpenURLFromTab(TabContents* source, - const GURL& url, - const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition); - virtual void NavigationStateChanged(const TabContents* source, - unsigned changed_flags); - virtual void ReplaceContents(TabContents* source, - TabContents* new_contents); - virtual void AddNewContents(TabContents* source, - TabContents* new_contents, - WindowOpenDisposition disposition, - const gfx::Rect& initial_pos, - bool user_gesture); - virtual void ActivateContents(TabContents* contents); - virtual void LoadingStateChanged(TabContents* source); - virtual void CloseContents(TabContents* source); virtual void MoveContents(TabContents* source, const gfx::Rect& pos); - virtual bool IsPopup(TabContents* source); virtual void ToolbarSizeChanged(TabContents* source, bool is_animating); - virtual void URLStarredChanged(TabContents* source, bool starred); - virtual void UpdateTargetURL(TabContents* source, const GURL& url); private: static void OnResponse(GtkWidget* widget, int response, HtmlDialogGtk* dialog); - // The Browser object which created this html dialog; we send all - // window opening/navigations to this object. - Browser* parent_browser_; - // This view is a delegate to the HTML content since it needs to get notified // about when the dialog is closing. For all other actions (besides dialog // closing) we delegate to the creator of this view, which we keep track of // using this variable. HtmlDialogUIDelegate* delegate_; + gfx::NativeWindow parent_window_; + GtkWidget* dialog_; scoped_ptr<TabContents> tab_contents_; diff --git a/chrome/browser/views/html_dialog_view.cc b/chrome/browser/views/html_dialog_view.cc index cda5340..e20e401 100644 --- a/chrome/browser/views/html_dialog_view.cc +++ b/chrome/browser/views/html_dialog_view.cc @@ -16,7 +16,8 @@ namespace browser { // Declared in browser_dialogs.h so that others don't need to depend on our .h. void ShowHtmlDialogView(gfx::NativeWindow parent, Browser* browser, HtmlDialogUIDelegate* delegate) { - HtmlDialogView* html_view = new HtmlDialogView(browser, delegate); + HtmlDialogView* html_view = + new HtmlDialogView(browser->profile(), delegate); views::Window::CreateChromeWindow(parent, gfx::Rect(), html_view); html_view->InitDialog(); html_view->window()->Show(); @@ -27,13 +28,11 @@ void ShowHtmlDialogView(gfx::NativeWindow parent, Browser* browser, //////////////////////////////////////////////////////////////////////////////// // HtmlDialogView, public: -HtmlDialogView::HtmlDialogView(Browser* parent_browser, +HtmlDialogView::HtmlDialogView(Profile* profile, HtmlDialogUIDelegate* delegate) : DOMView(), - parent_browser_(parent_browser), - profile_(parent_browser->profile()), + HtmlDialogTabContentsDelegate(profile), delegate_(delegate) { - DCHECK(profile_); } HtmlDialogView::~HtmlDialogView() { @@ -132,6 +131,7 @@ std::string HtmlDialogView::GetDialogArgs() const { void HtmlDialogView::OnDialogClosed(const std::string& json_retval) { HtmlDialogUIDelegate* dialog_delegate = delegate_; delegate_ = NULL; // We will not communicate further with the delegate. + HtmlDialogTabContentsDelegate::Detach(); dialog_delegate->OnDialogClosed(json_retval); window()->Close(); } @@ -139,83 +139,23 @@ void HtmlDialogView::OnDialogClosed(const std::string& json_retval) { //////////////////////////////////////////////////////////////////////////////// // TabContentsDelegate implementation: -void HtmlDialogView::OpenURLFromTab(TabContents* source, - const GURL& url, - const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition) { - // Force all links to open in a new window, ignoring the incoming - // disposition. This is a tabless, modal dialog so we can't just - // open it in the current frame. - static_cast<TabContentsDelegate*>(parent_browser_)->OpenURLFromTab( - source, url, referrer, NEW_WINDOW, transition); -} - -void HtmlDialogView::NavigationStateChanged(const TabContents* source, - unsigned changed_flags) { - // We shouldn't receive any NavigationStateChanged except the first - // one, which we ignore because we're a dialog box. -} - -void HtmlDialogView::ReplaceContents(TabContents* source, - TabContents* new_contents) { -} - -void HtmlDialogView::AddNewContents(TabContents* source, - TabContents* new_contents, - WindowOpenDisposition disposition, - const gfx::Rect& initial_pos, - bool user_gesture) { - static_cast<TabContentsDelegate*>(parent_browser_)->AddNewContents( - source, new_contents, NEW_WINDOW, initial_pos, user_gesture); -} - -void HtmlDialogView::ActivateContents(TabContents* contents) { - // We don't do anything here because there's only one TabContents in - // this frame and we don't have a TabStripModel. -} - -void HtmlDialogView::LoadingStateChanged(TabContents* source) { - // We don't care about this notification. -} - -void HtmlDialogView::CloseContents(TabContents* source) { - // We receive this message but don't handle it because we really do the - // cleanup in OnDialogClosed(). -} - void HtmlDialogView::MoveContents(TabContents* source, const gfx::Rect& pos) { // The contained web page wishes to resize itself. We let it do this because // if it's a dialog we know about, we trust it not to be mean to the user. GetWidget()->SetBounds(pos); } -bool HtmlDialogView::IsPopup(TabContents* source) { - // This needs to return true so that we are allowed to be resized by our - // contents. - return true; -} - void HtmlDialogView::ToolbarSizeChanged(TabContents* source, bool is_animating) { Layout(); } -void HtmlDialogView::URLStarredChanged(TabContents* source, bool starred) { - // We don't have a visible star to click in the window. - NOTREACHED(); -} - -void HtmlDialogView::UpdateTargetURL(TabContents* source, const GURL& url) { - // Ignored. -} - //////////////////////////////////////////////////////////////////////////////// // HtmlDialogView: void HtmlDialogView::InitDialog() { // Now Init the DOMView. This view runs in its own process to render the html. - DOMView::Init(profile_, NULL); + DOMView::Init(profile(), NULL); tab_contents_->set_delegate(this); diff --git a/chrome/browser/views/html_dialog_view.h b/chrome/browser/views/html_dialog_view.h index ee9e250..e61c50e 100644 --- a/chrome/browser/views/html_dialog_view.h +++ b/chrome/browser/views/html_dialog_view.h @@ -9,7 +9,7 @@ #include "base/gfx/size.h" #include "chrome/browser/dom_ui/html_dialog_ui.h" -#include "chrome/browser/tab_contents/tab_contents_delegate.h" +#include "chrome/browser/dom_ui/html_dialog_tab_contents_delegate.h" #include "chrome/browser/views/dom_view.h" #include "views/window/window_delegate.h" @@ -27,13 +27,17 @@ class Window; // expected to send back a JSON file as a return value. // //////////////////////////////////////////////////////////////////////////////// +// +// TODO(akalin): Make HtmlDialogView contain an HtmlDialogTabContentsDelegate +// instead of inheriting from it to avoid violating the "no multiple +// inheritance" rule. class HtmlDialogView : public DOMView, - public TabContentsDelegate, + public HtmlDialogTabContentsDelegate, public HtmlDialogUIDelegate, public views::WindowDelegate { public: - HtmlDialogView(Browser* parent_browser, HtmlDialogUIDelegate* delegate); + HtmlDialogView(Profile* profile, HtmlDialogUIDelegate* delegate); virtual ~HtmlDialogView(); // Initializes the contents of the dialog (the DOMView and the callbacks). @@ -62,36 +66,10 @@ class HtmlDialogView virtual void OnDialogClosed(const std::string& json_retval); // Overridden from TabContentsDelegate: - virtual void OpenURLFromTab(TabContents* source, - const GURL& url, - const GURL& referrer, - WindowOpenDisposition disposition, - PageTransition::Type transition); - virtual void NavigationStateChanged(const TabContents* source, - unsigned changed_flags); - virtual void ReplaceContents(TabContents* source, - TabContents* new_contents); - virtual void AddNewContents(TabContents* source, - TabContents* new_contents, - WindowOpenDisposition disposition, - const gfx::Rect& initial_pos, - bool user_gesture); - virtual void ActivateContents(TabContents* contents); - virtual void LoadingStateChanged(TabContents* source); - virtual void CloseContents(TabContents* source); virtual void MoveContents(TabContents* source, const gfx::Rect& pos); - virtual bool IsPopup(TabContents* source); virtual void ToolbarSizeChanged(TabContents* source, bool is_animating); - virtual void URLStarredChanged(TabContents* source, bool starred); - virtual void UpdateTargetURL(TabContents* source, const GURL& url); private: - // The Browser object which created this html dialog; we send all - // window opening/navigations to this object. - Browser* parent_browser_; - - Profile* profile_; - // This view is a delegate to the HTML content since it needs to get notified // about when the dialog is closing. For all other actions (besides dialog // closing) we delegate to the creator of this view, which we keep track of diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index edd36fd..0e97269 100755 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -568,6 +568,8 @@ 'browser/dom_ui/filebrowse_ui.h', 'browser/dom_ui/history_ui.cc', 'browser/dom_ui/history_ui.h', + 'browser/dom_ui/html_dialog_tab_contents_delegate.cc', + 'browser/dom_ui/html_dialog_tab_contents_delegate.h', 'browser/dom_ui/html_dialog_ui.cc', 'browser/dom_ui/html_dialog_ui.h', 'browser/dom_ui/most_visited_handler.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 46d4a15..2aa5eac 100755 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -607,6 +607,7 @@ 'browser/debugger/devtools_manager_unittest.cc', 'browser/dom_ui/dom_ui_theme_source_unittest.cc', 'browser/dom_ui/dom_ui_unittest.cc', + 'browser/dom_ui/html_dialog_tab_contents_delegate_unittest.cc', 'browser/dom_ui/shown_sections_handler_unittest.cc', 'browser/download/download_manager_unittest.cc', 'browser/download/download_request_infobar_delegate_unittest.cc', |