Commit a84f590c authored by Yao Xiao's avatar Yao Xiao Committed by Commit Bot

Fix the JS dialog dismissal cause

Previously on a non-Mac platform when JS dialog is dismissed by user
closing the tab, |DIALOG_BUTTON_CLICKED| is logged. This change
separates the case of the user replying to the dialog with the buttons
vs rejecting it.

Bug: 872795
Change-Id: I4b8781daeb3cead5ee4e975a67f79f94b7fa84e5
Reviewed-on: https://chromium-review.googlesource.com/1164647Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583885}
parent d53a039c
...@@ -4,8 +4,11 @@ ...@@ -4,8 +4,11 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_command_line.h" #include "base/test/scoped_command_line.h"
#include "build/build_config.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h" #include "chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
...@@ -15,7 +18,12 @@ ...@@ -15,7 +18,12 @@
#include "content/public/test/browser_test_utils.h" #include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h" #include "content/public/test/test_utils.h"
using JavaScriptDialogTest = InProcessBrowserTest; using DismissalCause = JavaScriptDialogTabHelper::DismissalCause;
class JavaScriptDialogTest : public InProcessBrowserTest {
private:
friend class JavaScriptDialogDismissalCauseTester;
};
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, ReloadDoesntHang) { IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, ReloadDoesntHang) {
content::WebContents* tab = content::WebContents* tab =
...@@ -191,3 +199,144 @@ IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, HandleJavaScriptDialog) { ...@@ -191,3 +199,144 @@ IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, HandleJavaScriptDialog) {
ASSERT_TRUE(callback_helper.last_success()); ASSERT_TRUE(callback_helper.last_success());
ASSERT_EQ(value1, callback_helper.last_input()); ASSERT_EQ(value1, callback_helper.last_input());
} }
class JavaScriptDialogDismissalCauseTester {
public:
explicit JavaScriptDialogDismissalCauseTester(JavaScriptDialogTest* test)
: tab_(test->browser()->tab_strip_model()->GetActiveWebContents()),
frame_(tab_->GetMainFrame()),
js_helper_(JavaScriptDialogTabHelper::FromWebContents(tab_)) {}
void PopupDialog(content::JavaScriptDialogType type) {
bool did_suppress = false;
js_helper_->RunJavaScriptDialog(
tab_, frame_, type, base::ASCIIToUTF16("Label"),
base::ASCIIToUTF16("abc"), {}, &did_suppress);
}
void ClickDialogButton(bool accept, const base::string16& user_input) {
EXPECT_TRUE(js_helper_->IsShowingDialogForTesting());
js_helper_->ClickDialogButtonForTesting(accept, user_input);
}
void Reload() {
tab_->GetController().Reload(content::ReloadType::NORMAL, false);
content::WaitForLoadStop(tab_);
}
void CallHandleDialog(bool accept, const base::string16* prompt_override) {
EXPECT_TRUE(js_helper_->IsShowingDialogForTesting());
js_helper_->HandleJavaScriptDialog(tab_, accept, prompt_override);
}
void CallCancelDialogs(bool reset_state) {
EXPECT_TRUE(js_helper_->IsShowingDialogForTesting());
js_helper_->CancelDialogs(tab_, reset_state);
}
private:
content::WebContents* tab_;
content::RenderFrameHost* frame_;
JavaScriptDialogTabHelper* js_helper_;
};
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, DismissalCausePromptAcceptButton) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.ClickDialogButton(true, base::string16());
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kDialogButtonClicked, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, DismissalCausePromptCancelButton) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.ClickDialogButton(false, base::string16());
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kDialogButtonClicked, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, DismissalCausePromptHandleDialog) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.CallHandleDialog(true, nullptr);
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kHandleDialogCalled, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest,
DismissalCausePromptCancelDialogs) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.CallCancelDialogs(false);
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kCancelDialogsCalled, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest,
DismissalCausePromptTabClosedByUser) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
chrome::CloseTab(browser());
// There are differences in the implementations of Views on different platforms
// that cause different dismissal causes.
#if defined(OS_MACOSX)
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kCancelDialogsCalled, 1);
#else
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kDialogClosed, 1);
#endif
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, DismissalCausePromptTabHidden) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
chrome::NewTab(browser());
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kTabHidden, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest,
DismissalCausePromptBrowserSwitched) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
chrome::NewEmptyWindow(browser()->profile());
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kBrowserSwitched, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, DismissalCausePromptTabNavigated) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.Reload();
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kTabNavigated, 1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest,
DismissalCausePromptSubsequentDialogShown) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_PROMPT);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_ALERT);
histogram_tester.ExpectUniqueSample("JSDialogs.DismissalCause.Prompt",
DismissalCause::kSubsequentDialogShown,
1);
}
IN_PROC_BROWSER_TEST_F(JavaScriptDialogTest, NoDismissalAlertTabHidden) {
base::HistogramTester histogram_tester;
JavaScriptDialogDismissalCauseTester tester(this);
tester.PopupDialog(content::JAVASCRIPT_DIALOG_TYPE_ALERT);
chrome::NewTab(browser());
histogram_tester.ExpectTotalCount("JSDialogs.DismissalCause.Alert", 0);
}
...@@ -134,7 +134,8 @@ base::WeakPtr<JavaScriptDialog> CreateNewDialog( ...@@ -134,7 +134,8 @@ base::WeakPtr<JavaScriptDialog> CreateNewDialog(
content::JavaScriptDialogType dialog_type, content::JavaScriptDialogType dialog_type,
const base::string16& message_text, const base::string16& message_text,
const base::string16& default_prompt_text, const base::string16& default_prompt_text,
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback) { content::JavaScriptDialogManager::DialogClosedCallback dialog_callback,
base::OnceClosure dialog_closed_callback) {
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
return JavaScriptDialogAndroid::Create( return JavaScriptDialogAndroid::Create(
parent_web_contents, alerting_web_contents, title, dialog_type, parent_web_contents, alerting_web_contents, title, dialog_type,
...@@ -142,27 +143,13 @@ base::WeakPtr<JavaScriptDialog> CreateNewDialog( ...@@ -142,27 +143,13 @@ base::WeakPtr<JavaScriptDialog> CreateNewDialog(
#else #else
return JavaScriptDialogViews::Create( return JavaScriptDialogViews::Create(
parent_web_contents, alerting_web_contents, title, dialog_type, parent_web_contents, alerting_web_contents, title, dialog_type,
message_text, default_prompt_text, std::move(dialog_callback)); message_text, default_prompt_text, std::move(dialog_callback),
std::move(dialog_closed_callback));
#endif #endif
} }
} // namespace } // namespace
enum class JavaScriptDialogTabHelper::DismissalCause {
// This is used for a UMA histogram. Please never alter existing values, only
// append new ones.
TAB_HELPER_DESTROYED = 0,
SUBSEQUENT_DIALOG_SHOWN = 1,
HANDLE_DIALOG_CALLED = 2,
CANCEL_DIALOGS_CALLED = 3,
TAB_HIDDEN = 4,
BROWSER_SWITCHED = 5,
DIALOG_BUTTON_CLICKED = 6,
TAB_NAVIGATED = 7,
TAB_SWITCHED_OUT = 8,
COUNT,
};
JavaScriptDialogTabHelper::JavaScriptDialogTabHelper( JavaScriptDialogTabHelper::JavaScriptDialogTabHelper(
content::WebContents* web_contents) content::WebContents* web_contents)
: content::WebContentsObserver(web_contents) { : content::WebContentsObserver(web_contents) {
...@@ -172,7 +159,7 @@ JavaScriptDialogTabHelper::~JavaScriptDialogTabHelper() { ...@@ -172,7 +159,7 @@ JavaScriptDialogTabHelper::~JavaScriptDialogTabHelper() {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
DCHECK(!tab_strip_model_being_observed_); DCHECK(!tab_strip_model_being_observed_);
#endif #endif
CloseDialog(DismissalCause::TAB_HELPER_DESTROYED, false, base::string16()); CloseDialog(DismissalCause::kTabHelperDestroyed, false, base::string16());
} }
void JavaScriptDialogTabHelper::SetDialogShownCallbackForTesting( void JavaScriptDialogTabHelper::SetDialogShownCallbackForTesting(
...@@ -184,6 +171,13 @@ bool JavaScriptDialogTabHelper::IsShowingDialogForTesting() const { ...@@ -184,6 +171,13 @@ bool JavaScriptDialogTabHelper::IsShowingDialogForTesting() const {
return !!dialog_; return !!dialog_;
} }
void JavaScriptDialogTabHelper::ClickDialogButtonForTesting(
bool accept,
const base::string16& user_input) {
DCHECK(!!dialog_);
CloseDialog(DismissalCause::kDialogButtonClicked, accept, user_input);
}
void JavaScriptDialogTabHelper::RunJavaScriptDialog( void JavaScriptDialogTabHelper::RunJavaScriptDialog(
content::WebContents* alerting_web_contents, content::WebContents* alerting_web_contents,
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
...@@ -232,7 +226,7 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog( ...@@ -232,7 +226,7 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
} }
// Close any dialog already showing. // Close any dialog already showing.
CloseDialog(DismissalCause::SUBSEQUENT_DIALOG_SHOWN, false, base::string16()); CloseDialog(DismissalCause::kSubsequentDialogShown, false, base::string16());
bool make_pending = false; bool make_pending = false;
if (!IsWebContentsForemost(parent_web_contents) && if (!IsWebContentsForemost(parent_web_contents) &&
...@@ -300,7 +294,10 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog( ...@@ -300,7 +294,10 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
dialog_type, truncated_message_text, truncated_default_prompt_text, dialog_type, truncated_message_text, truncated_default_prompt_text,
base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog, base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog,
base::Unretained(this), base::Unretained(this),
DismissalCause::DIALOG_BUTTON_CLICKED)); DismissalCause::kDialogButtonClicked),
base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog,
base::Unretained(this), DismissalCause::kDialogClosed,
false, base::string16()));
} else { } else {
DCHECK(!pending_dialog_); DCHECK(!pending_dialog_);
dialog_ = CreateNewDialog( dialog_ = CreateNewDialog(
...@@ -308,7 +305,10 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog( ...@@ -308,7 +305,10 @@ void JavaScriptDialogTabHelper::RunJavaScriptDialog(
truncated_message_text, truncated_default_prompt_text, truncated_message_text, truncated_default_prompt_text,
base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog, base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog,
base::Unretained(this), base::Unretained(this),
DismissalCause::DIALOG_BUTTON_CLICKED)); DismissalCause::kDialogButtonClicked),
base::BindOnce(&JavaScriptDialogTabHelper::CloseDialog,
base::Unretained(this), DismissalCause::kDialogClosed,
false, base::string16()));
} }
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
...@@ -370,7 +370,7 @@ bool JavaScriptDialogTabHelper::HandleJavaScriptDialog( ...@@ -370,7 +370,7 @@ bool JavaScriptDialogTabHelper::HandleJavaScriptDialog(
bool accept, bool accept,
const base::string16* prompt_override) { const base::string16* prompt_override) {
if (dialog_ || pending_dialog_) { if (dialog_ || pending_dialog_) {
CloseDialog(DismissalCause::HANDLE_DIALOG_CALLED, accept, CloseDialog(DismissalCause::kHandleDialogCalled, accept,
prompt_override ? *prompt_override : dialog_->GetUserInput()); prompt_override ? *prompt_override : dialog_->GetUserInput());
return true; return true;
} }
...@@ -383,7 +383,7 @@ bool JavaScriptDialogTabHelper::HandleJavaScriptDialog( ...@@ -383,7 +383,7 @@ bool JavaScriptDialogTabHelper::HandleJavaScriptDialog(
void JavaScriptDialogTabHelper::CancelDialogs( void JavaScriptDialogTabHelper::CancelDialogs(
content::WebContents* web_contents, content::WebContents* web_contents,
bool reset_state) { bool reset_state) {
CloseDialog(DismissalCause::CANCEL_DIALOGS_CALLED, false, base::string16()); CloseDialog(DismissalCause::kCancelDialogsCalled, false, base::string16());
// Cancel any app-modal dialogs being run by the app-modal dialog system. // Cancel any app-modal dialogs being run by the app-modal dialog system.
return AppModalDialogManager()->CancelDialogs(web_contents, reset_state); return AppModalDialogManager()->CancelDialogs(web_contents, reset_state);
...@@ -392,7 +392,7 @@ void JavaScriptDialogTabHelper::CancelDialogs( ...@@ -392,7 +392,7 @@ void JavaScriptDialogTabHelper::CancelDialogs(
void JavaScriptDialogTabHelper::OnVisibilityChanged( void JavaScriptDialogTabHelper::OnVisibilityChanged(
content::Visibility visibility) { content::Visibility visibility) {
if (visibility == content::Visibility::HIDDEN) { if (visibility == content::Visibility::HIDDEN) {
HandleTabSwitchAway(DismissalCause::TAB_HIDDEN); HandleTabSwitchAway(DismissalCause::kTabHidden);
} else if (pending_dialog_) { } else if (pending_dialog_) {
dialog_ = std::move(pending_dialog_).Run(); dialog_ = std::move(pending_dialog_).Run();
pending_dialog_.Reset(); pending_dialog_.Reset();
...@@ -408,7 +408,7 @@ void JavaScriptDialogTabHelper::DidStartNavigation( ...@@ -408,7 +408,7 @@ void JavaScriptDialogTabHelper::DidStartNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
// Close the dialog if the user started a new navigation. This allows reloads // Close the dialog if the user started a new navigation. This allows reloads
// and history navigations to proceed. // and history navigations to proceed.
CloseDialog(DismissalCause::TAB_NAVIGATED, false, base::string16()); CloseDialog(DismissalCause::kTabNavigated, false, base::string16());
} }
// This function handles the case where browser-side navigation (PlzNavigate) is // This function handles the case where browser-side navigation (PlzNavigate) is
...@@ -420,7 +420,7 @@ void JavaScriptDialogTabHelper::DidStartNavigationToPendingEntry( ...@@ -420,7 +420,7 @@ void JavaScriptDialogTabHelper::DidStartNavigationToPendingEntry(
content::ReloadType reload_type) { content::ReloadType reload_type) {
// Close the dialog if the user started a new navigation. This allows reloads // Close the dialog if the user started a new navigation. This allows reloads
// and history navigations to proceed. // and history navigations to proceed.
CloseDialog(DismissalCause::TAB_NAVIGATED, false, base::string16()); CloseDialog(DismissalCause::kTabNavigated, false, base::string16());
} }
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
...@@ -428,7 +428,7 @@ void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) { ...@@ -428,7 +428,7 @@ void JavaScriptDialogTabHelper::OnBrowserSetLastActive(Browser* browser) {
if (IsWebContentsForemost(web_contents())) { if (IsWebContentsForemost(web_contents())) {
OnVisibilityChanged(content::Visibility::VISIBLE); OnVisibilityChanged(content::Visibility::VISIBLE);
} else { } else {
HandleTabSwitchAway(DismissalCause::BROWSER_SWITCHED); HandleTabSwitchAway(DismissalCause::kBrowserSwitched);
} }
} }
...@@ -443,7 +443,7 @@ void JavaScriptDialogTabHelper::TabReplacedAt( ...@@ -443,7 +443,7 @@ void JavaScriptDialogTabHelper::TabReplacedAt(
// must be done here. // must be done here.
SetTabNeedsAttentionImpl(false, tab_strip_model, index); SetTabNeedsAttentionImpl(false, tab_strip_model, index);
CloseDialog(DismissalCause::TAB_SWITCHED_OUT, false, base::string16()); CloseDialog(DismissalCause::kTabSwitchedOut, false, base::string16());
} }
} }
...@@ -461,29 +461,23 @@ void JavaScriptDialogTabHelper::TabDetachedAt(content::WebContents* contents, ...@@ -461,29 +461,23 @@ void JavaScriptDialogTabHelper::TabDetachedAt(content::WebContents* contents,
DCHECK(tab_strip_model_being_observed_); DCHECK(tab_strip_model_being_observed_);
tab_strip_model_being_observed_->RemoveObserver(this); tab_strip_model_being_observed_->RemoveObserver(this);
tab_strip_model_being_observed_ = nullptr; tab_strip_model_being_observed_ = nullptr;
CloseDialog(DismissalCause::TAB_HELPER_DESTROYED, false, base::string16()); CloseDialog(DismissalCause::kTabHelperDestroyed, false, base::string16());
} }
} }
#endif #endif
void JavaScriptDialogTabHelper::LogDialogDismissalCause( void JavaScriptDialogTabHelper::LogDialogDismissalCause(DismissalCause cause) {
JavaScriptDialogTabHelper::DismissalCause cause) { // Log to UMA
switch (dialog_type_) { switch (dialog_type_) {
case content::JAVASCRIPT_DIALOG_TYPE_ALERT: case content::JAVASCRIPT_DIALOG_TYPE_ALERT:
UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Alert", UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Alert", cause);
static_cast<int>(cause),
static_cast<int>(DismissalCause::COUNT));
break; break;
case content::JAVASCRIPT_DIALOG_TYPE_CONFIRM: case content::JAVASCRIPT_DIALOG_TYPE_CONFIRM:
UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Confirm", UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Confirm", cause);
static_cast<int>(cause),
static_cast<int>(DismissalCause::COUNT));
break; break;
case content::JAVASCRIPT_DIALOG_TYPE_PROMPT: case content::JAVASCRIPT_DIALOG_TYPE_PROMPT:
UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Prompt", UMA_HISTOGRAM_ENUMERATION("JSDialogs.DismissalCause.Prompt", cause);
static_cast<int>(cause),
static_cast<int>(DismissalCause::COUNT));
break; break;
} }
} }
...@@ -522,7 +516,8 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause, ...@@ -522,7 +516,8 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause,
// //
// Using the |cause| to distinguish a call from JavaScriptDialog vs from // Using the |cause| to distinguish a call from JavaScriptDialog vs from
// within JavaScriptDialogTabHelper is a bit hacky, but is the simplest way. // within JavaScriptDialogTabHelper is a bit hacky, but is the simplest way.
if (dialog_ && cause != DismissalCause::DIALOG_BUTTON_CLICKED) if (dialog_ && cause != DismissalCause::kDialogButtonClicked &&
cause != DismissalCause::kDialogClosed)
dialog_->CloseDialogWithoutCallback(); dialog_->CloseDialogWithoutCallback();
// If there is a callback, call it. There might not be one, if a tab-modal // If there is a callback, call it. There might not be one, if a tab-modal
...@@ -534,8 +529,8 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause, ...@@ -534,8 +529,8 @@ void JavaScriptDialogTabHelper::CloseDialog(DismissalCause cause,
// state; clear it out. However, if the tab was switched out, the turning off // state; clear it out. However, if the tab was switched out, the turning off
// of the "needs attention" state was done in TabReplacedAt() or // of the "needs attention" state was done in TabReplacedAt() or
// TabDetachedAt() because SetTabNeedsAttention won't work, so don't call it. // TabDetachedAt() because SetTabNeedsAttention won't work, so don't call it.
if (pending_dialog_ && cause != DismissalCause::TAB_SWITCHED_OUT && if (pending_dialog_ && cause != DismissalCause::kTabSwitchedOut &&
cause != DismissalCause::TAB_HELPER_DESTROYED) { cause != DismissalCause::kTabHelperDestroyed) {
SetTabNeedsAttention(false); SetTabNeedsAttention(false);
} }
......
...@@ -48,11 +48,61 @@ class JavaScriptDialogTabHelper ...@@ -48,11 +48,61 @@ class JavaScriptDialogTabHelper
#endif #endif
public content::WebContentsUserData<JavaScriptDialogTabHelper> { public content::WebContentsUserData<JavaScriptDialogTabHelper> {
public: public:
enum class DismissalCause {
// This is used for a UMA histogram. Please never alter existing values,
// only append new ones.
// The tab helper is destroyed. By current design, the dialog is always torn
// down before the tab helper is destroyed, so we never see the
// |kTabHelperDestroyed| enum. However, that might not always be the case.
// It's better to have a simple rule in the code of "when you close a dialog
// you must provide a UMA enum reason why" and have some enums that never
// happen than have haphazard code that enforces no rules.
kTabHelperDestroyed = 0,
// Subsequent dialog pops up.
kSubsequentDialogShown = 1,
// HandleJavaScriptDialog() is called. In practice, this can happen whenever
// browser choose to accept/cancel the dialog without user's interaction.
kHandleDialogCalled = 2,
// CancelDialogs() is called. In practice, this can happen whenever browser
// choose to close the dialog without user's interaction. Besides, this can
// also happen when tab is closed by user on a Mac platform.
kCancelDialogsCalled = 3,
// Tab is made hidden by opening a new tab, switching to another tab, etc.
// Note that only Prompt() and Confirm() can be dismissed for this cause;
// it won't affect Alert().
kTabHidden = 4,
// Another browser instance is made active.
kBrowserSwitched = 5,
// Accept/Cancel button is clicked by user.
kDialogButtonClicked = 6,
// Navigation occurs.
kTabNavigated = 7,
// TabReplacedAt() is called.
kTabSwitchedOut = 8,
// CloseDialog() is called. In practice, this happens when tab is closed by
// user on a non-Mac platform.
kDialogClosed = 9,
kMaxValue = kDialogClosed,
};
explicit JavaScriptDialogTabHelper(content::WebContents* web_contents); explicit JavaScriptDialogTabHelper(content::WebContents* web_contents);
~JavaScriptDialogTabHelper() override; ~JavaScriptDialogTabHelper() override;
void SetDialogShownCallbackForTesting(base::OnceClosure callback); void SetDialogShownCallbackForTesting(base::OnceClosure callback);
bool IsShowingDialogForTesting() const; bool IsShowingDialogForTesting() const;
void ClickDialogButtonForTesting(bool accept,
const base::string16& user_input);
// JavaScriptDialogManager: // JavaScriptDialogManager:
void RunJavaScriptDialog(content::WebContents* web_contents, void RunJavaScriptDialog(content::WebContents* web_contents,
...@@ -96,7 +146,6 @@ class JavaScriptDialogTabHelper ...@@ -96,7 +146,6 @@ class JavaScriptDialogTabHelper
private: private:
friend class content::WebContentsUserData<JavaScriptDialogTabHelper>; friend class content::WebContentsUserData<JavaScriptDialogTabHelper>;
enum class DismissalCause;
// Logs the cause of a dialog dismissal in UMA. // Logs the cause of a dialog dismissal in UMA.
void LogDialogDismissalCause(DismissalCause cause); void LogDialogDismissalCause(DismissalCause cause);
......
...@@ -19,15 +19,18 @@ base::WeakPtr<JavaScriptDialogViews> JavaScriptDialogViews::Create( ...@@ -19,15 +19,18 @@ base::WeakPtr<JavaScriptDialogViews> JavaScriptDialogViews::Create(
content::JavaScriptDialogType dialog_type, content::JavaScriptDialogType dialog_type,
const base::string16& message_text, const base::string16& message_text,
const base::string16& default_prompt_text, const base::string16& default_prompt_text,
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback) { content::JavaScriptDialogManager::DialogClosedCallback dialog_callback,
base::OnceClosure dialog_force_closed_callback) {
return (new JavaScriptDialogViews( return (new JavaScriptDialogViews(
parent_web_contents, alerting_web_contents, title, dialog_type, parent_web_contents, alerting_web_contents, title, dialog_type,
message_text, default_prompt_text, std::move(dialog_callback))) message_text, default_prompt_text, std::move(dialog_callback),
std::move(dialog_force_closed_callback)))
->weak_factory_.GetWeakPtr(); ->weak_factory_.GetWeakPtr();
} }
void JavaScriptDialogViews::CloseDialogWithoutCallback() { void JavaScriptDialogViews::CloseDialogWithoutCallback() {
dialog_callback_.Reset(); dialog_callback_.Reset();
dialog_force_closed_callback_.Reset();
GetWidget()->Close(); GetWidget()->Close();
} }
...@@ -61,8 +64,8 @@ bool JavaScriptDialogViews::Accept() { ...@@ -61,8 +64,8 @@ bool JavaScriptDialogViews::Accept() {
} }
bool JavaScriptDialogViews::Close() { bool JavaScriptDialogViews::Close() {
if (dialog_callback_) if (dialog_force_closed_callback_)
std::move(dialog_callback_).Run(false, base::string16()); std::move(dialog_force_closed_callback_).Run();
return true; return true;
} }
...@@ -102,12 +105,14 @@ JavaScriptDialogViews::JavaScriptDialogViews( ...@@ -102,12 +105,14 @@ JavaScriptDialogViews::JavaScriptDialogViews(
content::JavaScriptDialogType dialog_type, content::JavaScriptDialogType dialog_type,
const base::string16& message_text, const base::string16& message_text,
const base::string16& default_prompt_text, const base::string16& default_prompt_text,
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback) content::JavaScriptDialogManager::DialogClosedCallback dialog_callback,
base::OnceClosure dialog_force_closed_callback)
: title_(title), : title_(title),
dialog_type_(dialog_type), dialog_type_(dialog_type),
message_text_(message_text), message_text_(message_text),
default_prompt_text_(default_prompt_text), default_prompt_text_(default_prompt_text),
dialog_callback_(std::move(dialog_callback)), dialog_callback_(std::move(dialog_callback)),
dialog_force_closed_callback_(std::move(dialog_force_closed_callback)),
weak_factory_(this) { weak_factory_(this) {
int options = views::MessageBoxView::DETECT_DIRECTIONALITY; int options = views::MessageBoxView::DETECT_DIRECTIONALITY;
if (dialog_type == content::JAVASCRIPT_DIALOG_TYPE_PROMPT) if (dialog_type == content::JAVASCRIPT_DIALOG_TYPE_PROMPT)
......
...@@ -24,6 +24,9 @@ class JavaScriptDialogViews : public JavaScriptDialog, ...@@ -24,6 +24,9 @@ class JavaScriptDialogViews : public JavaScriptDialog,
public: public:
~JavaScriptDialogViews() override; ~JavaScriptDialogViews() override;
// Creates a new JS dialog. Note the two callbacks; |dialog_callback| is for
// user responses, while |dialog_force_closed_callback| is for when Views
// forces the dialog closed without a user reply.
static base::WeakPtr<JavaScriptDialogViews> Create( static base::WeakPtr<JavaScriptDialogViews> Create(
content::WebContents* parent_web_contents, content::WebContents* parent_web_contents,
content::WebContents* alerting_web_contents, content::WebContents* alerting_web_contents,
...@@ -31,7 +34,8 @@ class JavaScriptDialogViews : public JavaScriptDialog, ...@@ -31,7 +34,8 @@ class JavaScriptDialogViews : public JavaScriptDialog,
content::JavaScriptDialogType dialog_type, content::JavaScriptDialogType dialog_type,
const base::string16& message_text, const base::string16& message_text,
const base::string16& default_prompt_text, const base::string16& default_prompt_text,
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback); content::JavaScriptDialogManager::DialogClosedCallback dialog_callback,
base::OnceClosure dialog_force_closed_callback);
// JavaScriptDialog: // JavaScriptDialog:
void CloseDialogWithoutCallback() override; void CloseDialogWithoutCallback() override;
...@@ -62,13 +66,15 @@ class JavaScriptDialogViews : public JavaScriptDialog, ...@@ -62,13 +66,15 @@ class JavaScriptDialogViews : public JavaScriptDialog,
content::JavaScriptDialogType dialog_type, content::JavaScriptDialogType dialog_type,
const base::string16& message_text, const base::string16& message_text,
const base::string16& default_prompt_text, const base::string16& default_prompt_text,
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback); content::JavaScriptDialogManager::DialogClosedCallback dialog_callback,
base::OnceClosure dialog_force_closed_callback);
base::string16 title_; base::string16 title_;
content::JavaScriptDialogType dialog_type_; content::JavaScriptDialogType dialog_type_;
base::string16 message_text_; base::string16 message_text_;
base::string16 default_prompt_text_; base::string16 default_prompt_text_;
content::JavaScriptDialogManager::DialogClosedCallback dialog_callback_; content::JavaScriptDialogManager::DialogClosedCallback dialog_callback_;
base::OnceClosure dialog_force_closed_callback_;
// The message box view whose commands we handle. // The message box view whose commands we handle.
views::MessageBoxView* message_box_view_; views::MessageBoxView* message_box_view_;
......
...@@ -26053,6 +26053,11 @@ Called by update_gpu_driver_bug_workaround_entries.py.--> ...@@ -26053,6 +26053,11 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
The page displaying the dialog was switched out of its tab (e.g. tab The page displaying the dialog was switched out of its tab (e.g. tab
discarding) discarding)
</int> </int>
<int value="9" label="Dialog closed">
The UI toolkit forced the dialog to close (at root caused by the user
closing the owning tab but going through a different path due to platform
differences)
</int>
</enum> </enum>
<enum name="JumplisticonsDeleteCategory"> <enum name="JumplisticonsDeleteCategory">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment