Commit d0373dd2 authored by tmdiep's avatar tmdiep Committed by Commit bot

Ensure app install is aborted when the install dialog is closed

If the user initiates an app install via the app launcher, it is
possible to close the install dialog by clicking on the background.
The app launcher window and modal dialog are destroyed without
cancelling the dialog, therefore keeping the install active.

This patch ensures that ExtensionInstallPrompt::Delegate::
InstallUIAbort() is called in this scenario to allow the delegate
(WebstoreStandaloneInstaller) to abort the install.

BUG=409616
TEST=browser_tests

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

Cr-Commit-Position: refs/heads/master@{#294505}
parent 3ac3cdcf
......@@ -195,7 +195,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
scrollable_header_only_(NULL),
show_details_link_(NULL),
checkbox_info_label_(NULL),
unchecked_boxes_(0) {
unchecked_boxes_(0),
handled_result_(false) {
// Possible grid layouts without ExtensionPermissionDialog experiment:
// Inline install
// w/ permissions no permissions
......@@ -588,7 +589,10 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
sampling_event_ = ExperienceSamplingEvent::Create(event_name);
}
ExtensionInstallDialogView::~ExtensionInstallDialogView() {}
ExtensionInstallDialogView::~ExtensionInstallDialogView() {
if (!handled_result_)
delegate_->InstallUIAbort(true);
}
views::GridLayout* ExtensionInstallDialogView::CreateLayout(
views::View* parent,
......@@ -714,16 +718,23 @@ int ExtensionInstallDialogView::GetDefaultDialogButton() const {
}
bool ExtensionInstallDialogView::Cancel() {
if (handled_result_)
return true;
handled_result_ = true;
UpdateInstallResultHistogram(false);
if (sampling_event_.get())
if (sampling_event_)
sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kDeny);
delegate_->InstallUIAbort(true);
return true;
}
bool ExtensionInstallDialogView::Accept() {
DCHECK(!handled_result_);
handled_result_ = true;
UpdateInstallResultHistogram(true);
if (sampling_event_.get())
if (sampling_event_)
sampling_event_->CreateUserDecisionEvent(ExperienceSamplingEvent::kProceed);
delegate_->InstallUIProceed();
return true;
......
......@@ -149,6 +149,10 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
// ExperienceSampling: Track this UI event.
scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_;
// Set to true once the user's selection has been received and the
// |delegate_| has been notified.
bool handled_result_;
DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogView);
};
......
......@@ -69,35 +69,43 @@ class MockExtensionInstallPrompt : public ExtensionInstallPrompt {
ExtensionInstallPrompt::Prompt* prompt_;
};
class ScrollbarTest : public ExtensionBrowserTest {
class ExtensionInstallDialogViewTestBase : public ExtensionBrowserTest {
protected:
ScrollbarTest();
virtual ~ScrollbarTest() {}
explicit ExtensionInstallDialogViewTestBase(
ExtensionInstallPrompt::PromptType prompt_type);
virtual ~ExtensionInstallDialogViewTestBase() {}
virtual void SetUpOnMainThread() OVERRIDE;
ExtensionInstallPrompt::Prompt* prompt() { return prompt_.get(); }
content::WebContents* web_contents() { return web_contents_; }
MockExtensionInstallPromptDelegate* delegate() { return &delegate_; }
void SetPromptPermissions(std::vector<base::string16> permissions);
void SetPromptDetails(std::vector<base::string16> details);
void SetPromptRetainedFiles(std::vector<base::FilePath> files);
bool IsScrollbarVisible();
private:
const extensions::Extension* extension_;
MockExtensionInstallPrompt* install_prompt_;
scoped_refptr<ExtensionInstallPrompt::Prompt> prompt_;
content::WebContents* web_contents_;
MockExtensionInstallPromptDelegate delegate_;
DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogViewTestBase);
};
ScrollbarTest::ScrollbarTest() :
extension_(NULL),
install_prompt_(NULL),
prompt_(new ExtensionInstallPrompt::Prompt(
ExtensionInstallPrompt::PERMISSIONS_PROMPT)),
web_contents_(NULL) {}
ExtensionInstallDialogViewTestBase::ExtensionInstallDialogViewTestBase(
ExtensionInstallPrompt::PromptType prompt_type)
: extension_(NULL),
install_prompt_(NULL),
prompt_(new ExtensionInstallPrompt::Prompt(prompt_type)),
web_contents_(NULL) {
}
void ScrollbarTest::SetUpOnMainThread() {
void ExtensionInstallDialogViewTestBase::SetUpOnMainThread() {
ExtensionBrowserTest::SetUpOnMainThread();
extension_ = ExtensionBrowserTest::LoadExtension(test_data_dir_.AppendASCII(
"install_prompt/permissions_scrollbar_regression"));
......@@ -118,26 +126,41 @@ void ScrollbarTest::SetUpOnMainThread() {
this->SetPromptRetainedFiles(std::vector<base::FilePath>());
}
void ScrollbarTest::SetPromptPermissions(
void ExtensionInstallDialogViewTestBase::SetPromptPermissions(
std::vector<base::string16> permissions) {
prompt_->SetPermissions(permissions);
}
void ScrollbarTest::SetPromptDetails(
void ExtensionInstallDialogViewTestBase::SetPromptDetails(
std::vector<base::string16> details) {
prompt_->SetPermissionsDetails(details);
}
void ScrollbarTest::SetPromptRetainedFiles(
void ExtensionInstallDialogViewTestBase::SetPromptRetainedFiles(
std::vector<base::FilePath> files) {
prompt_->set_retained_files(files);
}
class ScrollbarTest : public ExtensionInstallDialogViewTestBase {
protected:
ScrollbarTest();
virtual ~ScrollbarTest() {}
bool IsScrollbarVisible();
private:
DISALLOW_COPY_AND_ASSIGN(ScrollbarTest);
};
ScrollbarTest::ScrollbarTest()
: ExtensionInstallDialogViewTestBase(
ExtensionInstallPrompt::PERMISSIONS_PROMPT) {
}
bool ScrollbarTest::IsScrollbarVisible() {
ExtensionInstallPrompt::ShowParams show_params(web_contents_);
MockExtensionInstallPromptDelegate delegate;
ExtensionInstallDialogView* dialog =
new ExtensionInstallDialogView(show_params.navigator, &delegate, prompt_);
ExtensionInstallPrompt::ShowParams show_params(web_contents());
ExtensionInstallDialogView* dialog = new ExtensionInstallDialogView(
show_params.navigator, delegate(), prompt());
// Create the modal view around the install dialog view.
views::Widget* modal =
......@@ -178,3 +201,61 @@ IN_PROC_BROWSER_TEST_F(ScrollbarTest, ScrollbarRegression) {
this->SetPromptDetails(details);
ASSERT_FALSE(IsScrollbarVisible()) << "Scrollbar is visible";
}
class ExtensionInstallDialogViewTest
: public ExtensionInstallDialogViewTestBase {
protected:
ExtensionInstallDialogViewTest()
: ExtensionInstallDialogViewTestBase(
ExtensionInstallPrompt::INSTALL_PROMPT) {}
virtual ~ExtensionInstallDialogViewTest() {}
private:
DISALLOW_COPY_AND_ASSIGN(ExtensionInstallDialogViewTest);
};
// Verifies that the delegate is notified when the user selects to accept or
// cancel the install.
IN_PROC_BROWSER_TEST_F(ExtensionInstallDialogViewTest, NotifyDelegate) {
{
// The user confirms the install.
MockExtensionInstallPromptDelegate delegate;
scoped_ptr<ExtensionInstallDialogView> dialog(
new ExtensionInstallDialogView(web_contents(), &delegate, prompt()));
views::DialogDelegateView* delegate_view = dialog.get();
delegate_view->Accept();
delegate_view->OnClosed();
dialog.reset();
EXPECT_EQ(0, delegate.abort_count());
EXPECT_EQ(1, delegate.proceed_count());
}
{
// The user cancels the install.
MockExtensionInstallPromptDelegate delegate;
scoped_ptr<ExtensionInstallDialogView> dialog(
new ExtensionInstallDialogView(web_contents(), &delegate, prompt()));
views::DialogDelegateView* delegate_view = dialog.get();
delegate_view->Cancel();
delegate_view->OnClosed();
dialog.reset();
EXPECT_EQ(1, delegate.abort_count());
EXPECT_EQ(0, delegate.proceed_count());
}
{
// Corner case: Dialog is closed without the user explicitly choosing to
// proceed or cancel.
MockExtensionInstallPromptDelegate delegate;
scoped_ptr<ExtensionInstallDialogView> dialog(
new ExtensionInstallDialogView(web_contents(), &delegate, prompt()));
dialog.reset();
EXPECT_EQ(1, delegate.abort_count());
EXPECT_EQ(0, delegate.proceed_count());
}
}
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