MacViews: Enable views based Download recovery dialog behind secondary-ui-md flag.

chrome/browser/download/download_danger_prompt_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/download/download_danger_prompt.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/safe_browsing_db/database_manager.h"
 #include "content/public/test/mock_download_item.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ui_base_switches.h"
 #include "url/gurl.h"
 
 using ::testing::_;
 using ::testing::ByRef;
 using ::testing::Eq;
 using ::testing::Return;
 using ::testing::ReturnRef;
 using ::testing::SaveArg;
-using safe_browsing::ClientDownloadResponse;
-using safe_browsing::ClientSafeBrowsingReportRequest;
-using safe_browsing::SafeBrowsingService;
 
 namespace safe_browsing {
 
+namespace {
+
 const char kTestDownloadUrl[] = "http://evildownload.com";
 const char kDownloadResponseToken[] = "default_token";
 
-class DownloadDangerPromptTest : public InProcessBrowserTest {
+// An enum to parameterize the tests so that the tests can be run with and
+// without the secondary-ui-md flag.
+enum class SecondaryUiMd {
+  ENABLED,
+  DISABLED,
+};
+
+// Functor to generate the test name suffix depending on the value of the

[tapted, 2017/01/25 00:32:17]

Can this be a regular function? See e.g.
https://codereview.chromium.org/2394123002/diff/370001/ui/views/controls/menu...

[karandeepb, 2017/01/25 00:51:51]

Done.

+// SecondaryUiMd param.
+struct PrintSecondaryUiMdStatus {
+  template <class ParamType>
+  std::string operator()(const testing::TestParamInfo<ParamType>& info) const {
+    switch (info.param) {
+      case SecondaryUiMd::ENABLED:
+        return "SecondaryUiMdEnabled";
+      case SecondaryUiMd::DISABLED:
+        return "SecondaryUiMdDisabled";
+    }
+    NOTREACHED();
+    return std::string();
+  }
+};
+
+}  // namespace
+
+class DownloadDangerPromptTest
+    : public InProcessBrowserTest,
+      public ::testing::WithParamInterface<SecondaryUiMd> {
  public:
   DownloadDangerPromptTest()
       : prompt_(nullptr),
         expected_action_(DownloadDangerPrompt::CANCEL),
         did_receive_callback_(false),
-        test_safe_browsing_factory_(
-            new safe_browsing::TestSafeBrowsingServiceFactory()) {}
+        test_safe_browsing_factory_(new TestSafeBrowsingServiceFactory()) {}

[tapted, 2017/01/25 00:32:17]

nit: (since you're changing it), I think the preference is for

base::MakeUnique<TestSafeBrowsingServiceFactory>()

[karandeepb, 2017/01/25 00:51:51]

Done.

 
   ~DownloadDangerPromptTest() override {}
 
   void SetUp() override {
     SafeBrowsingService::RegisterFactory(test_safe_browsing_factory_.get());
     InProcessBrowserTest::SetUp();
   }
 
   void TearDown() override {
     SafeBrowsingService::RegisterFactory(nullptr);
     InProcessBrowserTest::TearDown();
   }
 
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    // TODO(crbug.com/630357): Remove when secondary-ui-md is enabled by default

[tapted, 2017/01/25 00:32:17]

I don't think we need the TODO. When that happens,
switches::kExtendMdToSecondaryUi will disappear, so it will "automatically" get
cleaned up.

But! What might be missed is that the whole WithParamInterface can go too. So
perhaps update the TODO to mention that. Then when someone gets a compile error
here, hopefully they see the TODO and remember to remove WithParamInterface as
well as just delete the SetUpCommandLine override.

[karandeepb, 2017/01/25 00:51:51]

Done.

+    // on all platforms.
+    if (GetParam() == SecondaryUiMd::ENABLED)
+      command_line->AppendSwitch(switches::kExtendMdToSecondaryUi);

[karandeepb, 2017/01/24 09:17:21]

Tried using MaterialDesignControllerTestApi but it didn't work. It's difficult
to hook into the point MaterialDesignController is initialized for a browser
test.

[tapted, 2017/01/25 00:32:17]

This is good - that's how I've been doing it

+  }
+
   // Opens a new tab and waits for navigations to finish. If there are pending
   // navigations, the constrained prompt might be dismissed when the navigation
   // completes.
   void OpenNewTab() {
     ui_test_utils::NavigateToURLWithDisposition(
         browser(), GURL("about:blank"),
         WindowOpenDisposition::NEW_FOREGROUND_TAB,
         ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB |
             ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   }
@@ skipping 44 matching lines @@
 
   DownloadDangerPrompt* prompt() { return prompt_; }
 
  private:
   void SetUpDownloadItemExpectations(
       const content::DownloadDangerType& danger_type,
       const std::string& token) {
     EXPECT_CALL(download_, GetFileNameToReportUser()).WillRepeatedly(Return(
         base::FilePath(FILE_PATH_LITERAL("evil.exe"))));
     EXPECT_CALL(download_, GetDangerType()).WillRepeatedly(Return(danger_type));
-    safe_browsing::DownloadProtectionService::DownloadPingToken* token_obj
-        = new safe_browsing::DownloadProtectionService::DownloadPingToken(
-            token);
-    download_.SetUserData(
-        safe_browsing::DownloadProtectionService::kDownloadPingTokenKey,
-        token_obj);
+    DownloadProtectionService::DownloadPingToken* token_obj =
+        new DownloadProtectionService::DownloadPingToken(token);
+    download_.SetUserData(DownloadProtectionService::kDownloadPingTokenKey,
+                          token_obj);
   }
 
   void SetUpSafeBrowsingReportExpectations(
       bool did_proceed,
       const ClientDownloadResponse::Verdict& download_verdict,
       const std::string& token,
       bool from_download_api) {
     ClientSafeBrowsingReportRequest expected_report;
     expected_report.set_url(GURL(kTestDownloadUrl).spec());
     if (from_download_api)
@@ skipping 23 matching lines @@
     EXPECT_FALSE(did_receive_callback_);
     EXPECT_EQ(expected_action_, action);
     did_receive_callback_ = true;
     prompt_ = nullptr;
   }
 
   content::MockDownloadItem download_;
   DownloadDangerPrompt* prompt_;
   DownloadDangerPrompt::Action expected_action_;
   bool did_receive_callback_;
-  std::unique_ptr<safe_browsing::TestSafeBrowsingServiceFactory>
-      test_safe_browsing_factory_;
+  std::unique_ptr<TestSafeBrowsingServiceFactory> test_safe_browsing_factory_;
   std::string expected_serialized_report_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadDangerPromptTest);
 };
 
 // Disabled for flaky timeouts on Windows. crbug.com/446696
 #if defined(OS_WIN)
 #define MAYBE_TestAll DISABLED_TestAll
 #else
 #define MAYBE_TestAll TestAll
 #endif
-IN_PROC_BROWSER_TEST_F(DownloadDangerPromptTest, MAYBE_TestAll) {
+IN_PROC_BROWSER_TEST_P(DownloadDangerPromptTest, MAYBE_TestAll) {
   // ExperienceSampling: Set default actions for DownloadItem methods we need.
   GURL download_url(kTestDownloadUrl);
   ON_CALL(download(), GetURL()).WillByDefault(ReturnRef(download_url));
   ON_CALL(download(), GetReferrerUrl())
       .WillByDefault(ReturnRef(GURL::EmptyGURL()));
   ON_CALL(download(), GetBrowserContext())
       .WillByDefault(Return(browser()->profile()));
   base::FilePath empty_file_path;
   ON_CALL(download(), GetTargetFilePath())
       .WillByDefault(ReturnRef(empty_file_path));
@@ skipping 96 matching lines @@
   SetUpExpectations(DownloadDangerPrompt::CANCEL,
                     content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT,
                     ClientDownloadResponse::UNCOMMON,
                     std::string(),
                     true);
   EXPECT_CALL(download(), IsDangerous()).WillRepeatedly(Return(true));
   SimulatePromptAction(DownloadDangerPrompt::CANCEL);
   VerifyExpectations(true);
 }
 
+// Prefix for test instantiations intentionally left blank since the test
+// fixture class has a single parameterization.
+INSTANTIATE_TEST_CASE_P(,
+                        DownloadDangerPromptTest,
+                        ::testing::Values(SecondaryUiMd::ENABLED,
+                                          SecondaryUiMd::DISABLED),
+                        PrintSecondaryUiMdStatus());
+
 }  // namespace safe_browsing