From ae57a18e11170253cf2515738bbdd09c37000c8f Mon Sep 17 00:00:00 2001 From: "wittman@chromium.org" Date: Wed, 31 Jul 2013 17:56:06 +0000 Subject: Revert 212329 "Reland "Close web contents modal dialogs on conte..." Causes several problems related to the print preview dialog: http://crbug.com/262023 http://crbug.com/262916 http://crbug.com/265427 > Reland "Close web contents modal dialogs on content load start" > > Address the failure of some web contents modal dialogs to close when > interstitial WebUI is displayed by closing the dialogs on content load start. > Remove existing ad hoc support in dialogs for closing on page loading in favor > of a uniform approach. > > I'm using load start as the trigger for dialog close as this seems to be the > point in time at which the user first perceives the page to be changing. > > Notes on the print preview changes: with this change the dialog is closed when > load starts and the initiator tab gets removed via RemovePreviewDialog, so there > is no longer a need to handle NOTIFICATION_NAV_ENTRY_COMITTED for the initiator > tab. The PrintViewManager::PrintPreviewDone() DCHECK is removed since now > PrintViewManager::RenderViewGone() may be invoked and print_preview_state_ reset > due to WebContents closure before the dialog is destroyed and PrintPreviewDone() > is invoked. > > BUG=240575 > > Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=211058 > > Review URL: https://chromiumcodereview.appspot.com/17500003 TBR=wittman@chromium.org Review URL: https://codereview.chromium.org/21372006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214775 0039d316-1c4b-4281-b951-d872f2087c98 --- .../web_modal/web_contents_modal_dialog_manager.cc | 79 ++++++++-------------- .../web_modal/web_contents_modal_dialog_manager.h | 21 ++---- 2 files changed, 32 insertions(+), 68 deletions(-) (limited to 'components') diff --git a/components/web_modal/web_contents_modal_dialog_manager.cc b/components/web_modal/web_contents_modal_dialog_manager.cc index 461fb65..0e07f8d 100644 --- a/components/web_modal/web_contents_modal_dialog_manager.cc +++ b/components/web_modal/web_contents_modal_dialog_manager.cc @@ -13,6 +13,7 @@ #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_view.h" +#include "net/base/registry_controlled_domains/registry_controlled_domain.h" using content::WebContents; @@ -26,7 +27,7 @@ WebContentsModalDialogManager::~WebContentsModalDialogManager() { void WebContentsModalDialogManager::ShowDialog( NativeWebContentsModalDialog dialog) { - child_dialogs_.push_back(DialogState(dialog)); + child_dialogs_.push_back(dialog); native_manager_->ManageDialog(dialog); @@ -43,15 +44,7 @@ bool WebContentsModalDialogManager::IsShowingDialog() const { void WebContentsModalDialogManager::FocusTopmostDialog() { DCHECK(!child_dialogs_.empty()); - native_manager_->FocusDialog(child_dialogs_.front().dialog); -} - -void WebContentsModalDialogManager::SetPreventCloseOnLoadStart( - NativeWebContentsModalDialog dialog, - bool prevent) { - WebContentsModalDialogList::iterator loc = FindDialogState(dialog); - DCHECK(loc != child_dialogs_.end()); - loc->prevent_close_on_load_start = true; + native_manager_->FocusDialog(child_dialogs_.front()); } content::WebContents* WebContentsModalDialogManager::GetWebContents() const { @@ -60,7 +53,8 @@ content::WebContents* WebContentsModalDialogManager::GetWebContents() const { void WebContentsModalDialogManager::WillClose( NativeWebContentsModalDialog dialog) { - WebContentsModalDialogList::iterator i = FindDialogState(dialog); + WebContentsModalDialogList::iterator i( + std::find(child_dialogs_.begin(), child_dialogs_.end(), dialog)); // The Views tab contents modal dialog calls WillClose twice. Ignore the // second invocation. @@ -71,7 +65,7 @@ void WebContentsModalDialogManager::WillClose( child_dialogs_.erase(i); if (!child_dialogs_.empty() && removed_topmost_dialog && !closing_all_dialogs_) - native_manager_->ShowDialog(child_dialogs_.front().dialog); + native_manager_->ShowDialog(child_dialogs_.front()); BlockWebContentsInteraction(!child_dialogs_.empty()); } @@ -80,20 +74,16 @@ void WebContentsModalDialogManager::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { - if (type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED) { - if (child_dialogs_.empty()) - return; - - bool visible = *content::Details(details).ptr(); - if (visible) - native_manager_->ShowDialog(child_dialogs_.front().dialog); - else - native_manager_->HideDialog(child_dialogs_.front().dialog); - } else if (type == content::NOTIFICATION_LOAD_START) { - if (!child_dialogs_.empty() && - !child_dialogs_.front().prevent_close_on_load_start) - native_manager_->CloseDialog(child_dialogs_.front().dialog); - } + DCHECK(type == content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED); + + if (child_dialogs_.empty()) + return; + + bool visible = *content::Details(details).ptr(); + if (visible) + native_manager_->ShowDialog(child_dialogs_.front()); + else + native_manager_->HideDialog(child_dialogs_.front()); } WebContentsModalDialogManager::WebContentsModalDialogManager( @@ -103,34 +93,11 @@ WebContentsModalDialogManager::WebContentsModalDialogManager( native_manager_(CreateNativeManager(this)), closing_all_dialogs_(false) { DCHECK(native_manager_); - content::NavigationController* controller = - &GetWebContents()->GetController(); - registrar_.Add(this, - content::NOTIFICATION_LOAD_START, - content::Source(controller)); registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED, content::Source(web_contents)); } -WebContentsModalDialogManager::DialogState::DialogState( - NativeWebContentsModalDialog dialog) - : dialog(dialog), - prevent_close_on_load_start(false) { -} - -WebContentsModalDialogManager::WebContentsModalDialogList::iterator - WebContentsModalDialogManager::FindDialogState( - NativeWebContentsModalDialog dialog) { - WebContentsModalDialogList::iterator i; - for (i = child_dialogs_.begin(); i != child_dialogs_.end(); ++i) { - if (i->dialog == dialog) - break; - } - - return i; -} - void WebContentsModalDialogManager::BlockWebContentsInteraction(bool blocked) { WebContents* contents = web_contents(); if (!contents) { @@ -151,14 +118,24 @@ void WebContentsModalDialogManager::CloseAllDialogs() { // Clear out any dialogs since we are leaving this page entirely. while (!child_dialogs_.empty()) - native_manager_->CloseDialog(child_dialogs_.front().dialog); + native_manager_->CloseDialog(child_dialogs_.front()); closing_all_dialogs_ = false; } +void WebContentsModalDialogManager::DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) { + // Close constrained windows if necessary. + if (!net::registry_controlled_domains::SameDomainOrHost( + details.previous_url, details.entry->GetURL(), + net::registry_controlled_domains::EXCLUDE_PRIVATE_REGISTRIES)) + CloseAllDialogs(); +} + void WebContentsModalDialogManager::DidGetIgnoredUIEvent() { if (!child_dialogs_.empty()) - native_manager_->FocusDialog(child_dialogs_.front().dialog); + native_manager_->FocusDialog(child_dialogs_.front()); } void WebContentsModalDialogManager::WebContentsDestroyed(WebContents* tab) { diff --git a/components/web_modal/web_contents_modal_dialog_manager.h b/components/web_modal/web_contents_modal_dialog_manager.h index 00671b2..e2548c0 100644 --- a/components/web_modal/web_contents_modal_dialog_manager.h +++ b/components/web_modal/web_contents_modal_dialog_manager.h @@ -45,11 +45,6 @@ class WebContentsModalDialogManager // calling this function. void FocusTopmostDialog(); - // Set to true to prevent closing the window when a page load starts on the - // WebContents. - void SetPreventCloseOnLoadStart(NativeWebContentsModalDialog dialog, - bool prevent); - // Overriden from NativeWebContentsModalDialogManagerDelegate: virtual content::WebContents* GetWebContents() const OVERRIDE; // Called when a WebContentsModalDialogs we own is about to be closed. @@ -81,18 +76,7 @@ class WebContentsModalDialogManager explicit WebContentsModalDialogManager(content::WebContents* web_contents); friend class content::WebContentsUserData; - struct DialogState { - explicit DialogState(NativeWebContentsModalDialog dialog); - - NativeWebContentsModalDialog dialog; - bool prevent_close_on_load_start; - }; - - typedef std::deque WebContentsModalDialogList; - - // Utility function to get the dialog state for a dialog. - WebContentsModalDialogList::iterator FindDialogState( - NativeWebContentsModalDialog dialog); + typedef std::deque WebContentsModalDialogList; // Blocks/unblocks interaction with renderer process. void BlockWebContentsInteraction(bool blocked); @@ -103,6 +87,9 @@ class WebContentsModalDialogManager void CloseAllDialogs(); // Overridden from content::WebContentsObserver: + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; virtual void DidGetIgnoredUIEvent() OVERRIDE; virtual void WebContentsDestroyed(content::WebContents* tab) OVERRIDE; -- cgit v1.1