Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
Make JavaScript dialog suppression more aggressive.
Browse files Browse the repository at this point in the history
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 2460c76)

[email protected]

Review URL: https://codereview.chromium.org/1089873005

Cr-Commit-Position: refs/branch-heads/2357@{#172}
Cr-Branched-From: 59d4494-refs/heads/master@{#323860}
  • Loading branch information
Avi Drissman committed Apr 20, 2015
1 parent 655a170 commit 0345c8f
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 15 deletions.
2 changes: 1 addition & 1 deletion android_webview/browser/aw_javascript_dialog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void AwJavaScriptDialogManager::CancelActiveAndPendingDialogs(
content::WebContents* web_contents) {
}

void AwJavaScriptDialogManager::WebContentsDestroyed(
void AwJavaScriptDialogManager::ResetDialogState(
content::WebContents* web_contents) {
}

Expand Down
2 changes: 1 addition & 1 deletion android_webview/browser/aw_javascript_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 11 additions & 1 deletion components/app_modal/javascript_dialog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion components/app_modal/javascript_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions content/public/browser/javascript_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
};
Expand Down
3 changes: 1 addition & 2 deletions content/shell/browser/shell_javascript_dialog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion content/shell/browser/shell_javascript_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ void JavaScriptDialogHelper::CancelActiveAndPendingDialogs(
content::WebContents* web_contents) {
}

void JavaScriptDialogHelper::WebContentsDestroyed(
void JavaScriptDialogHelper::ResetDialogState(
content::WebContents* web_contents) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 0345c8f

Please sign in to comment.