From 0345c8f5b8f53b98f866b60e67fa8aa9b00bb10f Mon Sep 17 00:00:00 2001 From: Avi Drissman Date: Mon, 20 Apr 2015 19:28:26 -0400 Subject: [PATCH] Make JavaScript dialog suppression more aggressive. 1. If the "suppress" checkbox was selected for a site, suppress beforeunload dialogs as well. 2. Re-enable dialogs only on cross-site navigation. BUG=473903 TEST=as in bug Review URL: https://codereview.chromium.org/1091073002 Cr-Commit-Position: refs/heads/master@{#325637} (cherry picked from commit 2460c760ed65fe9af0c0c58d6524749a897508af) TBR=ben@chromium.org Review URL: https://codereview.chromium.org/1089873005 Cr-Commit-Position: refs/branch-heads/2357@{#172} Cr-Branched-From: 59d4494849b405682265ed5d3f5164573b9a939b-refs/heads/master@{#323860} --- .../browser/aw_javascript_dialog_manager.cc | 2 +- .../browser/aw_javascript_dialog_manager.h | 2 +- components/app_modal/javascript_dialog_manager.cc | 12 +++++++++++- components/app_modal/javascript_dialog_manager.h | 2 +- content/browser/web_contents/web_contents_impl.cc | 8 +++++--- content/public/browser/javascript_dialog_manager.h | 5 ++--- .../shell/browser/shell_javascript_dialog_manager.cc | 3 +-- .../shell/browser/shell_javascript_dialog_manager.h | 2 +- .../guest_view/web_view/javascript_dialog_helper.cc | 2 +- .../guest_view/web_view/javascript_dialog_helper.h | 2 +- 10 files changed, 25 insertions(+), 15 deletions(-) diff --git a/android_webview/browser/aw_javascript_dialog_manager.cc b/android_webview/browser/aw_javascript_dialog_manager.cc index 476935a53c806..afe9bda215eed 100644 --- a/android_webview/browser/aw_javascript_dialog_manager.cc +++ b/android_webview/browser/aw_javascript_dialog_manager.cc @@ -58,7 +58,7 @@ void AwJavaScriptDialogManager::CancelActiveAndPendingDialogs( content::WebContents* web_contents) { } -void AwJavaScriptDialogManager::WebContentsDestroyed( +void AwJavaScriptDialogManager::ResetDialogState( content::WebContents* web_contents) { } diff --git a/android_webview/browser/aw_javascript_dialog_manager.h b/android_webview/browser/aw_javascript_dialog_manager.h index 8713e3daca5e1..1c4254231b228 100644 --- a/android_webview/browser/aw_javascript_dialog_manager.h +++ b/android_webview/browser/aw_javascript_dialog_manager.h @@ -29,7 +29,7 @@ class AwJavaScriptDialogManager : public content::JavaScriptDialogManager { const DialogClosedCallback& callback) override; void CancelActiveAndPendingDialogs( content::WebContents* web_contents) override; - void WebContentsDestroyed(content::WebContents* web_contents) override; + void ResetDialogState(content::WebContents* web_contents) override; private: DISALLOW_COPY_AND_ASSIGN(AwJavaScriptDialogManager); diff --git a/components/app_modal/javascript_dialog_manager.cc b/components/app_modal/javascript_dialog_manager.cc index 5bf42fd4bfcdb..6d72439ef9397 100644 --- a/components/app_modal/javascript_dialog_manager.cc +++ b/components/app_modal/javascript_dialog_manager.cc @@ -128,6 +128,16 @@ void JavaScriptDialogManager::RunBeforeUnloadDialog( const base::string16& message_text, bool is_reload, const DialogClosedCallback& callback) { + ChromeJavaScriptDialogExtraData* extra_data = + &javascript_dialog_extra_data_[web_contents]; + + if (extra_data->suppress_javascript_messages_) { + // If a site harassed the user enough for them to put it on mute, then it + // lost its privilege to deny unloading. + callback.Run(true, base::string16()); + return; + } + const base::string16 title = l10n_util::GetStringUTF16(is_reload ? IDS_BEFORERELOAD_MESSAGEBOX_TITLE : IDS_BEFOREUNLOAD_MESSAGEBOX_TITLE); const base::string16 footer = l10n_util::GetStringUTF16(is_reload ? @@ -174,7 +184,7 @@ bool JavaScriptDialogManager::HandleJavaScriptDialog( return true; } -void JavaScriptDialogManager::WebContentsDestroyed( +void JavaScriptDialogManager::ResetDialogState( content::WebContents* web_contents) { CancelActiveAndPendingDialogs(web_contents); javascript_dialog_extra_data_.erase(web_contents); diff --git a/components/app_modal/javascript_dialog_manager.h b/components/app_modal/javascript_dialog_manager.h index 83720151b5ebb..7978cddbcadbe 100644 --- a/components/app_modal/javascript_dialog_manager.h +++ b/components/app_modal/javascript_dialog_manager.h @@ -58,7 +58,7 @@ class JavaScriptDialogManager : public content::JavaScriptDialogManager { const base::string16* prompt_override) override; void CancelActiveAndPendingDialogs( content::WebContents* web_contents) override; - void WebContentsDestroyed(content::WebContents* web_contents) override; + void ResetDialogState(content::WebContents* web_contents) override; base::string16 GetTitle(content::WebContents* web_contents, const GURL& origin_url, diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 24122cdae08c3..353d761b92f43 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -372,7 +372,7 @@ WebContentsImpl::~WebContentsImpl() { // Clear out any JavaScript state. if (dialog_manager_) - dialog_manager_->WebContentsDestroyed(this); + dialog_manager_->ResetDialogState(this); if (color_chooser_info_.get()) color_chooser_info_->chooser->End(); @@ -4206,9 +4206,11 @@ void WebContentsImpl::UpdateRenderViewSizeForRenderManager() { void WebContentsImpl::CancelModalDialogsForRenderManager() { // We need to cancel modal dialogs when doing a process swap, since the load - // deferrer would prevent us from swapping out. + // deferrer would prevent us from swapping out. We also clear the state + // because this is a cross-process navigation, which means that it's a new + // site that should not have to pay for the sins of its predecessor. if (dialog_manager_) - dialog_manager_->CancelActiveAndPendingDialogs(this); + dialog_manager_->ResetDialogState(this); } void WebContentsImpl::NotifySwappedFromRenderManager(RenderFrameHost* old_host, diff --git a/content/public/browser/javascript_dialog_manager.h b/content/public/browser/javascript_dialog_manager.h index cdc628b5bc5e5..114418cf51a75 100644 --- a/content/public/browser/javascript_dialog_manager.h +++ b/content/public/browser/javascript_dialog_manager.h @@ -55,9 +55,8 @@ class CONTENT_EXPORT JavaScriptDialogManager { // Cancels all active and pending dialogs for the given WebContents. virtual void CancelActiveAndPendingDialogs(WebContents* web_contents) = 0; - // The given WebContents is being destroyed; discards any saved state tied - // to it. - virtual void WebContentsDestroyed(WebContents* web_contents) = 0; + // Reset any saved state tied to the given WebContents. + virtual void ResetDialogState(WebContents* web_contents) = 0; virtual ~JavaScriptDialogManager() {} }; diff --git a/content/shell/browser/shell_javascript_dialog_manager.cc b/content/shell/browser/shell_javascript_dialog_manager.cc index 2f8f5cccd063b..e8554aaa7488a 100644 --- a/content/shell/browser/shell_javascript_dialog_manager.cc +++ b/content/shell/browser/shell_javascript_dialog_manager.cc @@ -114,8 +114,7 @@ void ShellJavaScriptDialogManager::CancelActiveAndPendingDialogs( #endif } -void ShellJavaScriptDialogManager::WebContentsDestroyed( - WebContents* web_contents) { +void ShellJavaScriptDialogManager::ResetDialogState(WebContents* web_contents) { } void ShellJavaScriptDialogManager::DialogClosed(ShellJavaScriptDialog* dialog) { diff --git a/content/shell/browser/shell_javascript_dialog_manager.h b/content/shell/browser/shell_javascript_dialog_manager.h index 1e1f07b05e0b2..331db22291474 100644 --- a/content/shell/browser/shell_javascript_dialog_manager.h +++ b/content/shell/browser/shell_javascript_dialog_manager.h @@ -36,7 +36,7 @@ class ShellJavaScriptDialogManager : public JavaScriptDialogManager { void CancelActiveAndPendingDialogs(WebContents* web_contents) override; - void WebContentsDestroyed(WebContents* web_contents) override; + void ResetDialogState(WebContents* web_contents) override; // Called by the ShellJavaScriptDialog when it closes. void DialogClosed(ShellJavaScriptDialog* dialog); diff --git a/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc b/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc index bbf581844c599..0d3c2b980cf9c 100644 --- a/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc +++ b/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc @@ -91,7 +91,7 @@ void JavaScriptDialogHelper::CancelActiveAndPendingDialogs( content::WebContents* web_contents) { } -void JavaScriptDialogHelper::WebContentsDestroyed( +void JavaScriptDialogHelper::ResetDialogState( content::WebContents* web_contents) { } diff --git a/extensions/browser/guest_view/web_view/javascript_dialog_helper.h b/extensions/browser/guest_view/web_view/javascript_dialog_helper.h index 50c89efd44e93..adf11cd34f976 100644 --- a/extensions/browser/guest_view/web_view/javascript_dialog_helper.h +++ b/extensions/browser/guest_view/web_view/javascript_dialog_helper.h @@ -35,7 +35,7 @@ class JavaScriptDialogHelper : public content::JavaScriptDialogManager { const base::string16* prompt_override) override; void CancelActiveAndPendingDialogs( content::WebContents* web_contents) override; - void WebContentsDestroyed(content::WebContents* web_contents) override; + void ResetDialogState(content::WebContents* web_contents) override; private: void OnPermissionResponse(