diff options
author | wittman@chromium.org <wittman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-25 23:32:03 +0000 |
---|---|---|
committer | wittman@chromium.org <wittman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-02-25 23:32:03 +0000 |
commit | cc687ec560fb4889c42d331784d4a3f805427b55 (patch) | |
tree | bc8526f9ee7a8f2f75f204cb39804028e1d70982 | |
parent | 564f10664749f79f945edd6f6e5cec993f1067ac (diff) | |
download | chromium_src-cc687ec560fb4889c42d331784d4a3f805427b55.zip chromium_src-cc687ec560fb4889c42d331784d4a3f805427b55.tar.gz chromium_src-cc687ec560fb4889c42d331784d4a3f805427b55.tar.bz2 |
Use opaque type to reference web contents modal dialog in WebContentsModalDialogManager
This CL is part 5 of 6 in the work to remove the WebContentsModalDialog
interface. It removes the remaining references to the
WebContentsModalDialog interface in WebContentsModalDialogManager and
replaces them with the new opaque NativeWebContentsModalDialog type.
This builds on part 4 in https://codereview.chromium.org/12280016.
BUG=157161
Review URL: https://chromiumcodereview.appspot.com/12283024
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@184514 0039d316-1c4b-4281-b951-d872f2087c98
7 files changed, 45 insertions, 70 deletions
diff --git a/chrome/browser/ui/gtk/constrained_window_gtk.cc b/chrome/browser/ui/gtk/constrained_window_gtk.cc index e35764e..e8b0829 100644 --- a/chrome/browser/ui/gtk/constrained_window_gtk.cc +++ b/chrome/browser/ui/gtk/constrained_window_gtk.cc @@ -80,7 +80,7 @@ ConstrainedWindowGtk::ConstrainedWindowGtk( WebContentsModalDialogManager* web_contents_modal_dialog_manager = WebContentsModalDialogManager::FromWebContents(web_contents_); - web_contents_modal_dialog_manager->AddDialog(this); + web_contents_modal_dialog_manager->AddDialog(widget()); } ConstrainedWindowGtk::~ConstrainedWindowGtk() { @@ -103,7 +103,7 @@ void ConstrainedWindowGtk::CloseWebContentsModalDialog() { delegate_->DeleteDelegate(); WebContentsModalDialogManager* web_contents_modal_dialog_manager = WebContentsModalDialogManager::FromWebContents(web_contents_); - web_contents_modal_dialog_manager->WillClose(this); + web_contents_modal_dialog_manager->WillClose(widget()); delete this; } diff --git a/chrome/browser/ui/native_web_contents_modal_dialog_manager.h b/chrome/browser/ui/native_web_contents_modal_dialog_manager.h index a769246..7fe87c7 100644 --- a/chrome/browser/ui/native_web_contents_modal_dialog_manager.h +++ b/chrome/browser/ui/native_web_contents_modal_dialog_manager.h @@ -7,8 +7,6 @@ #include "chrome/browser/ui/native_web_contents_modal_dialog.h" -class WebContentsModalDialog; - // Interface from NativeWebContentsModalDialogManager to // WebContentsModalDialogManager. class NativeWebContentsModalDialogManagerDelegate { @@ -16,7 +14,7 @@ class NativeWebContentsModalDialogManagerDelegate { NativeWebContentsModalDialogManagerDelegate() {} virtual ~NativeWebContentsModalDialogManagerDelegate() {} - virtual void WillClose(WebContentsModalDialog* dialog) = 0; + virtual void WillClose(NativeWebContentsModalDialog dialog) = 0; private: DISALLOW_COPY_AND_ASSIGN(NativeWebContentsModalDialogManagerDelegate); diff --git a/chrome/browser/ui/views/constrained_window_views.cc b/chrome/browser/ui/views/constrained_window_views.cc index 671a310..540fa47 100644 --- a/chrome/browser/ui/views/constrained_window_views.cc +++ b/chrome/browser/ui/views/constrained_window_views.cc @@ -625,7 +625,7 @@ ConstrainedWindowViews* ConstrainedWindowViews::Create( web_contents->GetNativeView(), web_contents->GetBrowserContext()->IsOffTheRecord(), widget_delegate); - manager->AddDialog(dialog); + manager->AddDialog(dialog->GetNativeView()); return dialog; } diff --git a/chrome/browser/ui/views/web_contents_modal_dialog_manager_views.cc b/chrome/browser/ui/views/web_contents_modal_dialog_manager_views.cc index 3d3fc1d..0591d31 100644 --- a/chrome/browser/ui/views/web_contents_modal_dialog_manager_views.cc +++ b/chrome/browser/ui/views/web_contents_modal_dialog_manager_views.cc @@ -99,7 +99,7 @@ class NativeWebContentsModalDialogManagerViews // views::WidgetObserver overrides virtual void OnWidgetClosing(views::Widget* widget) OVERRIDE { - native_delegate_->WillClose(static_cast<ConstrainedWindowViews*>(widget)); + native_delegate_->WillClose(widget->GetNativeView()); observed_widgets_.erase(widget); } diff --git a/chrome/browser/ui/web_contents_modal_dialog_manager.cc b/chrome/browser/ui/web_contents_modal_dialog_manager.cc index feb896f..1e4ead1 100644 --- a/chrome/browser/ui/web_contents_modal_dialog_manager.cc +++ b/chrome/browser/ui/web_contents_modal_dialog_manager.cc @@ -22,13 +22,13 @@ WebContentsModalDialogManager::~WebContentsModalDialogManager() { } void WebContentsModalDialogManager::AddDialog( - WebContentsModalDialog* dialog) { + NativeWebContentsModalDialog dialog) { child_dialogs_.push_back(dialog); - native_manager_->ManageDialog(dialog->GetNativeDialog()); + native_manager_->ManageDialog(dialog); if (child_dialogs_.size() == 1) { - native_manager_->ShowDialog(dialog->GetNativeDialog()); + native_manager_->ShowDialog(dialog); BlockWebContentsInteraction(true); } } @@ -57,14 +57,15 @@ bool WebContentsModalDialogManager::IsShowingDialog() const { void WebContentsModalDialogManager::FocusTopmostDialog() { DCHECK(dialog_count()); - WebContentsModalDialog* window = *dialog_begin(); - DCHECK(window); - native_manager_->FocusDialog(window->GetNativeDialog()); + native_manager_->FocusDialog(*dialog_begin()); } -void WebContentsModalDialogManager::WillClose(WebContentsModalDialog* dialog) { +void WebContentsModalDialogManager::WillClose( + NativeWebContentsModalDialog dialog) { WebContentsModalDialogList::iterator i( - std::find(child_dialogs_.begin(), child_dialogs_.end(), dialog)); + std::find(child_dialogs_.begin(), + child_dialogs_.end(), + dialog)); bool removed_topmost_dialog = i == child_dialogs_.begin(); if (i != child_dialogs_.end()) child_dialogs_.erase(i); @@ -72,7 +73,7 @@ void WebContentsModalDialogManager::WillClose(WebContentsModalDialog* dialog) { BlockWebContentsInteraction(false); } else { if (removed_topmost_dialog) - native_manager_->ShowDialog(child_dialogs_[0]->GetNativeDialog()); + native_manager_->ShowDialog(child_dialogs_[0]); BlockWebContentsInteraction(true); } } @@ -94,11 +95,8 @@ void WebContentsModalDialogManager::CloseAllDialogs() { WebContentsModalDialogList child_dialogs_copy(child_dialogs_); for (WebContentsModalDialogList::iterator it = child_dialogs_copy.begin(); it != child_dialogs_copy.end(); ++it) { - WebContentsModalDialog* dialog = *it; - if (dialog) { - native_manager_->CloseDialog(dialog->GetNativeDialog()); - BlockWebContentsInteraction(false); - } + native_manager_->CloseDialog(*it); + BlockWebContentsInteraction(false); } } @@ -112,10 +110,8 @@ void WebContentsModalDialogManager::DidNavigateMainFrame( } void WebContentsModalDialogManager::DidGetIgnoredUIEvent() { - if (dialog_count()) { - WebContentsModalDialog* dialog = *dialog_begin(); - native_manager_->FocusDialog(dialog->GetNativeDialog()); - } + if (dialog_count()) + native_manager_->FocusDialog(*dialog_begin()); } void WebContentsModalDialogManager::WebContentsDestroyed(WebContents* tab) { diff --git a/chrome/browser/ui/web_contents_modal_dialog_manager.h b/chrome/browser/ui/web_contents_modal_dialog_manager.h index 98b1eac..e14b05f0 100644 --- a/chrome/browser/ui/web_contents_modal_dialog_manager.h +++ b/chrome/browser/ui/web_contents_modal_dialog_manager.h @@ -32,7 +32,7 @@ class WebContentsModalDialogManager // Adds the given dialog to the list of child dialogs. The dialog will notify // via WillClose() when it is being destroyed. - void AddDialog(WebContentsModalDialog* dialog); + void AddDialog(NativeWebContentsModalDialog dialog); // Blocks/unblocks interaction with renderer process. void BlockWebContentsInteraction(bool blocked); @@ -46,7 +46,7 @@ class WebContentsModalDialogManager // Overriden from NativeWebContentsModalDialogManagerDelegate: // Called when a WebContentsModalDialogs we own is about to be closed. - virtual void WillClose(WebContentsModalDialog* dialog) OVERRIDE; + virtual void WillClose(NativeWebContentsModalDialog dialog) OVERRIDE; // For testing. class TestApi { @@ -69,7 +69,7 @@ class WebContentsModalDialogManager explicit WebContentsModalDialogManager(content::WebContents* web_contents); friend class content::WebContentsUserData<WebContentsModalDialogManager>; - typedef std::deque<WebContentsModalDialog*> WebContentsModalDialogList; + typedef std::deque<NativeWebContentsModalDialog> WebContentsModalDialogList; // Returns the number of dialogs in this tab. size_t dialog_count() const { return child_dialogs_.size(); } diff --git a/chrome/browser/ui/web_contents_modal_dialog_manager_unittest.cc b/chrome/browser/ui/web_contents_modal_dialog_manager_unittest.cc index 1ebdae5..7847039 100644 --- a/chrome/browser/ui/web_contents_modal_dialog_manager_unittest.cc +++ b/chrome/browser/ui/web_contents_modal_dialog_manager_unittest.cc @@ -2,8 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/memory/scoped_vector.h" -#include "chrome/browser/ui/web_contents_modal_dialog.h" +#include "chrome/browser/ui/native_web_contents_modal_dialog_manager.h" #include "chrome/browser/ui/web_contents_modal_dialog_manager.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "content/public/test/test_browser_thread.h" @@ -28,69 +27,51 @@ class WebContentsModalDialogManagerTest content::TestBrowserThread ui_thread_; }; -class WebContentsModalDialogCloseTest : public WebContentsModalDialog { - public: - explicit WebContentsModalDialogCloseTest(content::WebContents* web_contents, - int* close_count) - : close_count_(close_count), - web_contents_(web_contents) { - } - - virtual void ShowWebContentsModalDialog() OVERRIDE {} - virtual void CloseWebContentsModalDialog() OVERRIDE { - WebContentsModalDialogManager* web_contents_modal_dialog_manager = - WebContentsModalDialogManager::FromWebContents(web_contents_); - web_contents_modal_dialog_manager->WillClose(this); - (*close_count_)++; - } - virtual void FocusWebContentsModalDialog() OVERRIDE {} - virtual void PulseWebContentsModalDialog() OVERRIDE {} - virtual NativeWebContentsModalDialog GetNativeDialog() OVERRIDE { - // The WebContentsModalDialogManager requires a unique opaque ID for - // the dialog, so using reinterpret_cast of this is sufficient. - return reinterpret_cast<NativeWebContentsModalDialog>(this); - } - virtual ~WebContentsModalDialogCloseTest() {} - - int* close_count_; - content::WebContents* web_contents_; -}; - class NativeWebContentsModalDialogManagerCloseTest : public NativeWebContentsModalDialogManager { public: + NativeWebContentsModalDialogManagerCloseTest( + NativeWebContentsModalDialogManagerDelegate* delegate) + : delegate_(delegate) {} virtual void ManageDialog(NativeWebContentsModalDialog dialog) OVERRIDE { } virtual void ShowDialog(NativeWebContentsModalDialog dialog) OVERRIDE { } virtual void CloseDialog(NativeWebContentsModalDialog dialog) OVERRIDE { - reinterpret_cast<WebContentsModalDialogCloseTest*>(dialog)-> - CloseWebContentsModalDialog(); + delegate_->WillClose(dialog); + close_count++; } virtual void FocusDialog(NativeWebContentsModalDialog dialog) OVERRIDE { } virtual void PulseDialog(NativeWebContentsModalDialog dialog) OVERRIDE { } + + int close_count; + NativeWebContentsModalDialogManagerDelegate* delegate_; }; TEST_F(WebContentsModalDialogManagerTest, WebContentsModalDialogs) { - ScopedVector<WebContentsModalDialogCloseTest> windows; - int close_count = 0; WebContentsModalDialogManager* web_contents_modal_dialog_manager = WebContentsModalDialogManager::FromWebContents(web_contents()); WebContentsModalDialogManager::TestApi test_api( web_contents_modal_dialog_manager); - test_api.ResetNativeManager(new NativeWebContentsModalDialogManagerCloseTest); + NativeWebContentsModalDialogManagerCloseTest* native_manager = + new NativeWebContentsModalDialogManagerCloseTest( + web_contents_modal_dialog_manager); + native_manager->close_count = 0; + + test_api.ResetNativeManager(native_manager); const int kWindowCount = 4; - for (int i = 0; i < kWindowCount; i++) { - windows.push_back( - new WebContentsModalDialogCloseTest(web_contents(), &close_count)); - web_contents_modal_dialog_manager->AddDialog(windows.back()); - } - EXPECT_EQ(0, close_count); + for (int i = 0; i < kWindowCount; i++) + // WebContentsModalDialogManager treats the NativeWebContentsModalDialog as + // an opaque type, so creating fake NativeWebContentsModalDialogs using + // reinterpret_cast is valid. + web_contents_modal_dialog_manager->AddDialog( + reinterpret_cast<NativeWebContentsModalDialog>(i)); + EXPECT_EQ(native_manager->close_count, 0); test_api.CloseAllDialogs(); - EXPECT_EQ(kWindowCount, close_count); + EXPECT_EQ(native_manager->close_count, kWindowCount); } |