Add an NTP promo command to open the Safety Check

Bug: 1015841
Change-Id: I4927a2ad0d7e1d4ca61c48efb557694dd6d03866
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339994Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Moe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797525}

chrome/browser/about_flags.cc

@@ -1378,9 +1378,15 @@ const FeatureEntry::FeatureVariation kMarkHttpAsFeatureVariations[] = {
 
 const FeatureEntry::FeatureParam kPromoBrowserCommandUnknownCommandParam[] = {
     {features::kPromoBrowserCommandIdParam, "0"}};
+const FeatureEntry::FeatureParam
+    kPromoBrowserCommandOpenSafetyCheckCommandParam[] = {
+        {features::kPromoBrowserCommandIdParam, "1"}};
 const FeatureEntry::FeatureVariation kPromoBrowserCommandsVariations[] = {
     {"- Unknown Command", kPromoBrowserCommandUnknownCommandParam,
      base::size(kPromoBrowserCommandUnknownCommandParam),
+     "t4237555" /* variation_id */},
+    {"- Open Safety Check", kPromoBrowserCommandOpenSafetyCheckCommandParam,
+     base::size(kPromoBrowserCommandOpenSafetyCheckCommandParam),
      "t4237555" /* variation_id */}};
 
 #if defined(OS_ANDROID)

chrome/browser/promo_browser_command/promo_browser_command.mojom

@@ -15,6 +15,7 @@ module promo_browser_command.mojom;
 // Please update enums.xml upon addition of new commands.
 enum Command {
   kUnknownCommand = 0,
+  kOpenSafetyCheck = 1,
 };
 
 // Click information needed to determine user's desired window disposition using

chrome/browser/ui/webui/new_tab_page/promo_browser_command/promo_browser_command_handler.cc

@@ -11,6 +11,10 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/promos/promo_service.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_navigator.h"
+#include "chrome/browser/ui/chrome_pages.h"
+#include "chrome/common/webui_url_constants.h"
+#include "ui/base/page_transition_types.h"
 #include "ui/base/window_open_disposition.h"
 
 using promo_browser_command::mojom::ClickInfoPtr;
@@ -29,10 +33,7 @@ PromoBrowserCommandHandler::PromoBrowserCommandHandler(
       page_handler_(this, std::move(pending_page_handler)) {
   if (!base::FeatureList::IsEnabled(features::kPromoBrowserCommands))
     return;
-
-  // Explicitly enable supported commands.
-  command_updater_->UpdateCommandEnabled(
-      static_cast<int>(Command::kUnknownCommand), true);
+  EnableCommands();
 }
 
 PromoBrowserCommandHandler::~PromoBrowserCommandHandler() = default;
@@ -44,8 +45,9 @@ void PromoBrowserCommandHandler::ExecuteCommand(
   const auto disposition = ui::DispositionFromClick(
       click_info->middle_button, click_info->alt_key, click_info->ctrl_key,
       click_info->meta_key, click_info->shift_key);
-  const bool command_executed = command_updater_->ExecuteCommandWithDisposition(
-      static_cast<int>(command_id), disposition);
+  const bool command_executed =
+      GetCommandUpdater()->ExecuteCommandWithDisposition(
+          static_cast<int>(command_id), disposition);
   std::move(callback).Run(command_executed);
 }
 
@@ -59,13 +61,32 @@ void PromoBrowserCommandHandler::ExecuteCommandWithDisposition(
     case Command::kUnknownCommand:
       // Nothing to do.
       break;
+    case Command::kOpenSafetyCheck:
+      NavigateToURL(GURL(chrome::GetSettingsUrl(chrome::kSafetyCheckSubPage)),
+                    disposition);
+      break;
     default:
       NOTREACHED() << "Unspecified behavior for command " << id;
       break;
   }
 }
 
-void PromoBrowserCommandHandler::SetCommandUpdaterForTesting(
-    std::unique_ptr<CommandUpdater> command_updater) {
-  command_updater_ = std::move(command_updater);
+void PromoBrowserCommandHandler::EnableCommands() {
+  // Explicitly enable supported commands.
+  GetCommandUpdater()->UpdateCommandEnabled(
+      static_cast<int>(Command::kUnknownCommand), true);
+  GetCommandUpdater()->UpdateCommandEnabled(
+      static_cast<int>(Command::kOpenSafetyCheck), true);
+}
+
+CommandUpdater* PromoBrowserCommandHandler::GetCommandUpdater() {
+  return command_updater_.get();
+}
+
+void PromoBrowserCommandHandler::NavigateToURL(
+    const GURL& url,
+    WindowOpenDisposition disposition) {
+  NavigateParams params(profile_, url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = disposition;
+  Navigate(&params);
 }

chrome/browser/ui/webui/new_tab_page/promo_browser_command/promo_browser_command_handler.h

@@ -14,8 +14,8 @@
 #include "ui/base/window_open_disposition.h"
 
 class CommandUpdater;
+class GURL;
 class Profile;
-class PromoBrowserCommandHandlerTest;
 
 // Handles promo browser commands send from JS.
 class PromoBrowserCommandHandler
@@ -40,12 +40,14 @@ class PromoBrowserCommandHandler
       int command_id,
       WindowOpenDisposition disposition) override;
 
- private:
-  friend class PromoBrowserCommandHandlerTest;
+ protected:
+  void EnableCommands();
+
+  virtual CommandUpdater* GetCommandUpdater();
 
-  void SetCommandUpdaterForTesting(
-      std::unique_ptr<CommandUpdater> command_updater);
-  CommandUpdater* command_updater() { return command_updater_.get(); }
+ private:
+  virtual void NavigateToURL(const GURL& url,
+                             WindowOpenDisposition disposition);
 
   Profile* profile_;
   std::unique_ptr<CommandUpdater> command_updater_;

chrome/browser/ui/webui/new_tab_page/promo_browser_command/promo_browser_command_handler_unittest.cc

@@ -11,14 +11,57 @@
 #include "chrome/browser/browser_features.h"
 #include "chrome/browser/command_updater_impl.h"
 #include "chrome/browser/promo_browser_command/promo_browser_command.mojom.h"
+#include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/webui/new_tab_page/promo_browser_command/promo_browser_command_handler.h"
+#include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/browser_task_environment.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/window_open_disposition.h"
+
+using promo_browser_command::mojom::ClickInfo;
+using promo_browser_command::mojom::ClickInfoPtr;
+using promo_browser_command::mojom::Command;
+using promo_browser_command::mojom::CommandHandler;
 
 namespace {
 
+class TestCommandHandler : public PromoBrowserCommandHandler {
+ public:
+  explicit TestCommandHandler(Profile* profile)
+      : PromoBrowserCommandHandler(mojo::PendingReceiver<CommandHandler>(),
+                                   profile) {}
+  ~TestCommandHandler() override = default;
+
+  void NavigateToURL(const GURL&, WindowOpenDisposition) override {
+    // The functionality of opening a URL is removed, as it cannot be executed
+    // in a unittest.
+  }
+
+  CommandUpdater* GetCommandUpdater() override {
+    if (command_updater_)
+      return command_updater_.get();
+    return PromoBrowserCommandHandler::GetCommandUpdater();
+  }
+
+  void SetCommandUpdater(std::unique_ptr<CommandUpdater> command_updater) {
+    command_updater_ = std::move(command_updater);
+    // Ensure that all commands are also updated in the new |command_updater|.
+    EnableCommands();
+  }
+
+  std::unique_ptr<CommandUpdater> command_updater_;
+};
+
+class MockCommandHandler : public TestCommandHandler {
+ public:
+  explicit MockCommandHandler(Profile* profile) : TestCommandHandler(profile) {}
+  ~MockCommandHandler() override = default;
+
+  MOCK_METHOD2(NavigateToURL, void(const GURL&, WindowOpenDisposition));
+};
+
 class MockCommandUpdater : public CommandUpdaterImpl {
  public:
   explicit MockCommandUpdater(CommandUpdaterDelegate* delegate)
@@ -37,12 +80,13 @@ void ExecuteCommandCallback(base::OnceClosure quit_closure,
   std::move(quit_closure).Run();
 }
 
-}  // namespace
+// A shorthand for conversion between ClickInfo and WindowOpenDisposition.
+WindowOpenDisposition DispositionFromClick(const ClickInfo& info) {
+  return ui::DispositionFromClick(info.middle_button, info.alt_key,
+                                  info.ctrl_key, info.meta_key, info.shift_key);
+}
 
-using promo_browser_command::mojom::ClickInfo;
-using promo_browser_command::mojom::ClickInfoPtr;
-using promo_browser_command::mojom::Command;
-using promo_browser_command::mojom::CommandHandler;
+}  // namespace
 
 class PromoBrowserCommandHandlerTest : public testing::Test {
  public:
@@ -50,24 +94,21 @@ class PromoBrowserCommandHandlerTest : public testing::Test {
   ~PromoBrowserCommandHandlerTest() override = default;
 
   void SetUp() override {
-    command_handler_ = std::make_unique<PromoBrowserCommandHandler>(
-        mojo::PendingReceiver<CommandHandler>(), &profile_);
-    command_handler_->SetCommandUpdaterForTesting(
-        std::make_unique<MockCommandUpdater>(command_handler_.get()));
+    command_handler_ = std::make_unique<MockCommandHandler>(&profile_);
   }
 
   void TearDown() override { testing::Test::TearDown(); }
 
   MockCommandUpdater* mock_command_updater() {
     return static_cast<MockCommandUpdater*>(
-        command_handler_->command_updater());
+        command_handler_->GetCommandUpdater());
   }
 
   bool ExecuteCommand(Command command_id, ClickInfoPtr click_info) {
     base::RunLoop run_loop;
     bool command_executed = false;
     command_handler_->ExecuteCommand(
-        Command::kUnknownCommand, ClickInfo::New(),
+        command_id, std::move(click_info),
         base::BindOnce(&ExecuteCommandCallback, run_loop.QuitClosure(),
                        &command_executed));
     run_loop.Run();
@@ -77,12 +118,16 @@ class PromoBrowserCommandHandlerTest : public testing::Test {
  protected:
   content::BrowserTaskEnvironment task_environment_;
   TestingProfile profile_;
-  std::unique_ptr<PromoBrowserCommandHandler> command_handler_;
+  std::unique_ptr<MockCommandHandler> command_handler_;
 };
 
 TEST_F(PromoBrowserCommandHandlerTest, SupportedCommands) {
   base::HistogramTester histogram_tester;
 
+  // Mock out the command updater to test enabling and disabling commands.
+  command_handler_->SetCommandUpdater(
+      std::make_unique<MockCommandUpdater>(command_handler_.get()));
+
   // Unsupported commands do not get executed and no histogram is logged.
   EXPECT_CALL(*mock_command_updater(),
               SupportsCommand(static_cast<int>(Command::kUnknownCommand)))
@@ -128,10 +173,22 @@ TEST_F(PromoBrowserCommandHandlerTest, DisableHandlingCommands) {
 
   // The PromoBrowserCommandHandler instance needs to be recreated for the
   // feature to take effect.
-  command_handler_ = std::make_unique<PromoBrowserCommandHandler>(
-      mojo::PendingReceiver<CommandHandler>(), &profile_);
+  command_handler_ = std::make_unique<MockCommandHandler>(&profile_);
 
   EXPECT_FALSE(ExecuteCommand(Command::kUnknownCommand, ClickInfo::New()));
   histogram_tester.ExpectTotalCount(
       PromoBrowserCommandHandler::kPromoBrowserCommandHistogramName, 0);
 }
+
+TEST_F(PromoBrowserCommandHandlerTest, OpenSafetyCheckCommand) {
+  // The OpenSafetyCheck command opens a new settings window with the Safety
+  // Check, and the correct disposition.
+  ClickInfoPtr info = ClickInfo::New();
+  info->middle_button = true;
+  info->meta_key = true;
+  EXPECT_CALL(
+      *command_handler_,
+      NavigateToURL(GURL(chrome::GetSettingsUrl(chrome::kSafetyCheckSubPage)),
+                    DispositionFromClick(*info)));
+  EXPECT_TRUE(ExecuteCommand(Command::kOpenSafetyCheck, std::move(info)));
+}

chrome/common/webui_url_constants.cc

@@ -402,6 +402,7 @@ const char kPrintingSettingsSubPage[] = "printing";
 const char kPrivacySubPage[] = "privacy";
 const char kResetSubPage[] = "reset";
 const char kResetProfileSettingsSubPage[] = "resetProfileSettings";
+const char kSafetyCheckSubPage[] = "safetyCheck";
 const char kSearchSubPage[] = "search";
 const char kSearchEnginesSubPage[] = "searchEngines";
 const char kSignOutSubPage[] = "signOut";

chrome/common/webui_url_constants.h

@@ -356,6 +356,7 @@ extern const char kPrintingSettingsSubPage[];
 extern const char kPrivacySubPage[];
 extern const char kResetSubPage[];
 extern const char kResetProfileSettingsSubPage[];
+extern const char kSafetyCheckSubPage[];
 extern const char kSearchSubPage[];
 extern const char kSearchEnginesSubPage[];
 extern const char kSignOutSubPage[];