Make CWS Report Abuse page the active tab after report abuse and uninstall

chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc

Index: chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
diff --git a/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc b/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
index 5e0fbcb4aef207f297d91e469e800ca8a3134369..62f7dbb9b52b9dc2cecb7e005c62f0988c656855 100644
--- a/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
+++ b/chrome/browser/ui/views/extensions/extension_uninstall_dialog_view_browsertest.cc
@@ -8,15 +8,23 @@
 #include "chrome/browser/extensions/extension_uninstall_dialog.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/extension_dialog_auto_confirm.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
+#include "extensions/common/extension_urls.h"
 #include "extensions/common/value_builder.h"
 
 namespace {
 
+const char kReferrerId[] = "chrome-remove-extension-dialog";
+
+// A preference key storing the url loaded when an extension is uninstalled.
+const char kUninstallUrl[] = "uninstall_url";
+
 scoped_refptr<extensions::Extension> BuildTestExtension() {
   return extensions::ExtensionBuilder()
       .SetManifest(extensions::DictionaryBuilder()
@@ -26,6 +34,17 @@ scoped_refptr<extensions::Extension> BuildTestExtension() {
       .Build();
 }
 
+const GURL& GetActiveUrl(Browser* browser) {
+  return browser->tab_strip_model()->GetActiveWebContents()->GetVisibleURL();

[Devlin, 2017/05/15 23:28:04]

We should use GetLastCommittedURL() (everywhere in this file)

[catmullings, 2017/05/18 18:26:00]

GetLastCommitedURL returns an empty string when used. 

[Devlin, 2017/05/18 19:14:20]

Ah, I see.  We're checking this a bit too soon in the flow.  We should wait for
the web contents to stop loading before checking the URL.

[catmullings, 2017/05/19 22:04:50]

Done.

+}
+
+void SetUninstallURL(extensions::ExtensionPrefs* prefs,
+                     const std::string& extension_id,
+                     const std::string& url_string) {
+  prefs->UpdateExtensionPref(extension_id, kUninstallUrl,
+                             base::MakeUnique<base::Value>(url_string));
+}
+
 class TestExtensionUninstallDialogDelegate
     : public extensions::ExtensionUninstallDialog::Delegate {
  public:
@@ -107,3 +126,108 @@ IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
   run_loop.Run();
   EXPECT_TRUE(delegate.canceled());
 }
+
+// Test that when the user clicks Uninstall on the ExtensionUninstallDialog, the
+// extension's uninstall url (when it is specified) should open and be the
+// active tab.
+IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
+                       EnsureExtensionUninstallURLIsActiveTabAfterUninstall) {
+  scoped_refptr<extensions::Extension> extension(BuildTestExtension());
+  ExtensionService* extension_service =
+      extensions::ExtensionSystem::Get(browser()->profile())
+          ->extension_service();
+  std::string uninstall_url = "https://www.google.com/";
+  SetUninstallURL(
+      extensions::ExtensionPrefs::Get(extension_service->GetBrowserContext()),

[Devlin, 2017/05/15 23:28:04]

Since we always set the same uninstall url, we could actually just pass a
profile and extension id to SetUninstallUrl() to make this code a little
shorter:
SetUninstallUrl(browser()->profile(), extension->id());

[catmullings, 2017/05/18 18:26:00]

Did you mean profile or extension prefs?

[Devlin, 2017/05/18 19:14:20]

I meant profile so that SetUninstallUrl() could get the prefs and avoid
duplicating that code, but either is fine.

[catmullings, 2017/05/19 22:04:50]

Sg. I will keep the prefs as a param.

[catmullings, 2017/05/19 22:04:50]

Done.

+      extension->id(), uninstall_url);
+  extension_service->AddExtension(extension.get());

[Devlin, 2017/05/15 23:28:04]

nit: let's AddExtension() before setting its uninstall url, just since that's
more representative of the real flow.

[catmullings, 2017/05/18 18:26:00]

Done.

+
+  // Auto-confirm the uninstall dialog.
+  extensions::ScopedTestDialogAutoConfirm auto_confirm(
+      extensions::ScopedTestDialogAutoConfirm::ACCEPT);
+
+  base::RunLoop run_loop;
+  TestExtensionUninstallDialogDelegate delegate(run_loop.QuitClosure());
+  std::unique_ptr<extensions::ExtensionUninstallDialog> dialog(
+      extensions::ExtensionUninstallDialog::Create(
+          browser()->profile(), browser()->window()->GetNativeWindow(),
+          &delegate));
+  content::RunAllPendingInMessageLoop();
+
+  dialog->ConfirmUninstall(extension,
+                           // UNINSTALL_REASON_USER_INITIATED is used to trigger
+                           // complete uninstallation.
+                           extensions::UNINSTALL_REASON_USER_INITIATED,
+                           extensions::UNINSTALL_SOURCE_FOR_TESTING);
+
+  content::RunAllPendingInMessageLoop();
+
+  // There should be 2 tabs open: chrome://about and the extension's uninstall
+  // url.
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  // Verifying that the extension's uninstall url is the active tab.
+  EXPECT_EQ(GetActiveUrl(browser()), uninstall_url);
+
+  // Kill parent window.

[Devlin, 2017/05/15 23:28:04]

Is this necessary?

[catmullings, 2017/05/18 18:26:00]

The comment? Or the code (browser()->window()->Close()) ?
The comment is just copy/paste from the previous tests, but I can remove it
here.

[Devlin, 2017/05/18 19:14:20]

The code.  The previous tests were checking destruction patterns, so they kill
the parent window.  This test doesn't need to do that, I think?

[catmullings, 2017/05/19 22:04:50]

Done.

+  browser()->window()->Close();
+  run_loop.Run();
+  // The delegate should not be canceled because the user chose to uninstall the
+  // extension, which should be successful.
+  EXPECT_TRUE(!delegate.canceled());
+}
+
+// Test that when the user clicks the Report Abuse checkbox and clicks Uninstall
+// on the ExtensionUninstallDialog, the extension's uninstall url (when it is
+// specified) and the CWS Report Abuse survey are opened in the browser, also
+// testing that the CWS survey is the active tab.
+IN_PROC_BROWSER_TEST_F(ExtensionUninstallDialogViewBrowserTest,
+                       EnsureCWSReportAbusePageIsActiveTabAfterUninstall) {
+  scoped_refptr<extensions::Extension> extension(BuildTestExtension());
+  ExtensionService* extension_service =
+      extensions::ExtensionSystem::Get(browser()->profile())
+          ->extension_service();
+  std::string uninstall_url = "https://www.google.com/";
+  SetUninstallURL(
+      extensions::ExtensionPrefs::Get(extension_service->GetBrowserContext()),
+      extension->id(), uninstall_url);
+  extension_service->AddExtension(extension.get());
+
+  // Auto-confirm the uninstall dialog.
+  extensions::ScopedTestDialogAutoConfirm auto_confirm(
+      extensions::ScopedTestDialogAutoConfirm::ACCEPT_AND_OPTION);
+
+  base::RunLoop run_loop;
+  TestExtensionUninstallDialogDelegate delegate(run_loop.QuitClosure());
+  std::unique_ptr<extensions::ExtensionUninstallDialog> dialog(
+      extensions::ExtensionUninstallDialog::Create(
+          browser()->profile(), browser()->window()->GetNativeWindow(),
+          &delegate));
+  content::RunAllPendingInMessageLoop();
+
+  dialog->ConfirmUninstall(extension,
+                           // UNINSTALL_REASON_USER_INITIATED is used to trigger
+                           // complete uninstallation.
+                           extensions::UNINSTALL_REASON_USER_INITIATED,
+                           extensions::UNINSTALL_SOURCE_FOR_TESTING);
+
+  content::RunAllPendingInMessageLoop();
+
+  // There should be 3 tabs open: chrome://about, the extension's uninstall url,
+  // and the CWS Report Abuse survey.
+  EXPECT_EQ(3, browser()->tab_strip_model()->count());
+  // Verifying that the extension's uninstall url was opened. It should not be
+  // the active tab.
+  EXPECT_EQ(browser()->tab_strip_model()->GetWebContentsAt(1)->GetVisibleURL(),
+            uninstall_url);
+  // The CWS Report Abuse survey should be the active tab.
+  EXPECT_EQ(
+      extension_urls::GetWebstoreReportAbuseUrl(extension->id(), kReferrerId),
+      GetActiveUrl(browser()));
+
+  // Kill parent window.
+  browser()->window()->Close();
+  run_loop.Run();
+  // The delegate should not be canceled because the user chose to uninstall the
+  // extension, which should be successful.
+  EXPECT_TRUE(!delegate.canceled());
+}