Parameterize file_tasks_browsertest.cc on Guest/Incognito profile types.

There have been recent crashes while trying to launch apps when these
profile types make their way through various APIs.

Although this test file doesn't deal with _launching_ the tasks, there
are planned changes to where file handlers may be stored, and we don't
want these expectations to change due to the profile type that is
passed to the file manager, and other APIs.

This CL introduces a "mixin" to allow any InProcessBrowserTest harness
to set up a command line with the de-facto settings used to run
browser tests under incognito and guest profiles.

Bug: 1122379
Change-Id: I16e8099be24043427a1ecc4b73d6fae8422f0e71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377041Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802167}

chrome/browser/chromeos/file_manager/file_tasks_browsertest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/chromeos/file_manager/filesystem_api_util.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/chromeos/web_applications/default_web_app_ids.h"
+#include "chrome/browser/chromeos/web_applications/test/profile_test_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/web_applications/system_web_app_manager.h"
@@ -99,7 +100,8 @@ scoped_refptr<const extensions::Extension> InstallTiffHandlerChromeApp(
       profile, "extensions/api_test/file_browser/app_file_handler");
 }
 
-class FileTasksBrowserTestBase : public InProcessBrowserTest {
+class FileTasksBrowserTestBase
+    : public TestProfileTypeMixin<InProcessBrowserTest> {
  public:
   void SetUpOnMainThread() override {
     test::AddDefaultComponentExtensionsOnMainThread(browser()->profile());
@@ -217,7 +219,7 @@ constexpr Expectation kUnchangedExpectations[] = {
 }  // namespace
 
 // Test file extensions correspond to mime types where expected.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, ExtensionToMimeMapping) {
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ExtensionToMimeMapping) {
   constexpr struct {
     const char* file_extension;
     bool has_mime = true;
@@ -287,7 +289,7 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, ExtensionToMimeMapping) {
 // resolution mechanism is "sort by extension ID", which has the desired result.
 // If desires change, we'll need to update ChooseAndSetDefaultTask() with some
 // additional logic.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, DefaultHandlerChangeDetector) {
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, DefaultHandlerChangeDetector) {
   //  With the Media App disabled, all images should be handled by Gallery.
   std::vector<Expectation> expectations = {
       // Images.
@@ -303,7 +305,7 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, DefaultHandlerChangeDetector) {
 
 // Spot test the default handlers for selections that include multiple different
 // file types. Only tests combinations of interest to the Media App.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, MultiSelectDefaultHandler) {
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, MultiSelectDefaultHandler) {
   std::vector<Expectation> expectations = {
       {"jpg/gif", kGalleryAppId},
       {"jpg/mp4", kGalleryAppId},
@@ -313,7 +315,7 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, MultiSelectDefaultHandler) {
 }
 
 // Tests the default handlers with the Media App installed.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestWithMediaApp,
                        DefaultHandlerChangeDetector) {
   // With the Media App enabled, images should be handled by it by default (but
   // video, which it also handles should be unchanged).
@@ -331,7 +333,7 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
 
 // Spot test the default handlers for selections that include multiple different
 // file types with the Media App installed.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestWithMediaApp,
                        MultiSelectDefaultHandler) {
   std::vector<Expectation> expectations = {
       {"jpg/gif", kMediaAppId},
@@ -350,27 +352,56 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
 
 // Sanity check: the tiff-specific file handler is preferred when MediaApp is
 // not enabled.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTest, InstalledAppsAreImplicitDefaults) {
-  auto extension = InstallTiffHandlerChromeApp(browser()->profile());
-  TestExpectationsAgainstDefaultTasks({{"tiff", extension->id().c_str()}});
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, InstalledAppsAreImplicitDefaults) {
+  if (GetParam() == TestProfileType::kRegular) {
+    auto extension = InstallTiffHandlerChromeApp(browser()->profile());
+    TestExpectationsAgainstDefaultTasks({{"tiff", extension->id().c_str()}});
+  } else if (GetParam() == TestProfileType::kGuest) {
+    // The extension can't be installed in guest mode. (FATAL check under
+    // ExtensionService::AddNewOrUpdatedExtension()). There should be no tiff
+    // handler.
+    TestExpectationsAgainstDefaultTasks({{"tiff", nullptr}});
+  } else {
+    // The extension installs in incognito, but doesn't register a file handler.
+    // So, also no tiff handler.
+    InstallTiffHandlerChromeApp(browser()->profile());
+    TestExpectationsAgainstDefaultTasks({{"tiff", nullptr}});
+  }
 }
 
 // If the media app is enabled, it will be preferred over a chrome app with a
 // specific extension, unless that app is set default via prefs.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestWithMediaApp,
                        MediaAppPreferredOverChromeApps) {
+  if (GetParam() == TestProfileType::kGuest) {
+    // The provided file system can't install in guest mode. Just check that
+    // MediaApp handles tiff.
+    TestExpectationsAgainstDefaultTasks({{"tiff", kMediaAppId}});
+    return;
+  }
   Profile* profile = browser()->profile();
   auto extension = InstallTiffHandlerChromeApp(profile);
   TestExpectationsAgainstDefaultTasks({{"tiff", kMediaAppId}});
 
   UpdateDefaultTask(profile->GetPrefs(), extension->id() + "|app|tiffAction",
                     {"tiff"}, {"image/tiff"});
-  TestExpectationsAgainstDefaultTasks({{"tiff", extension->id().c_str()}});
+  if (GetParam() == TestProfileType::kIncognito) {
+    // In incognito, the provided file system can exist, but the file handler
+    // preference can't be changed.
+    TestExpectationsAgainstDefaultTasks({{"tiff", kMediaAppId}});
+  } else {
+    TestExpectationsAgainstDefaultTasks({{"tiff", extension->id().c_str()}});
+  }
 }
 
 // Test expectations for files coming from provided file systems.
-IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
+IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestWithMediaApp,
                        ProvidedFileSystemFileSource) {
+  if (GetParam() == TestProfileType::kGuest) {
+    // Provided file systems don't exist in guest. This test seems to work OK in
+    // incognito mode though.
+    return;
+  }
   // The current test expectation: a GIF file in the provided file system called
   // "readwrite.gif" should open with the MediaApp.
   const char kTestFile[] = "readwrite.gif";
@@ -415,5 +446,19 @@ IN_PROC_BROWSER_TEST_F(FileTasksBrowserTestWithMediaApp,
   EXPECT_EQ(remaining_expectations, 0);
 }
 
+INSTANTIATE_TEST_SUITE_P(All,
+                         FileTasksBrowserTest,
+                         ::testing::Values(TestProfileType::kRegular,
+                                           TestProfileType::kIncognito,
+                                           TestProfileType::kGuest),
+                         TestProfileTypeToString);
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         FileTasksBrowserTestWithMediaApp,
+                         ::testing::Values(TestProfileType::kRegular,
+                                           TestProfileType::kIncognito,
+                                           TestProfileType::kGuest),
+                         TestProfileTypeToString);
+
 }  // namespace file_tasks
 }  // namespace file_manager

chrome/browser/chromeos/web_applications/test/profile_test_helper.cc

+// Copyright 2020 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 "chrome/browser/chromeos/web_applications/test/profile_test_helper.h"
+
+#include "chromeos/constants/chromeos_switches.h"
+#include "components/account_id/account_id.h"
+#include "components/user_manager/user_names.h"
+
+std::string TestProfileTypeToString(
+    const ::testing::TestParamInfo<TestProfileType>& info) {
+  switch (info.param) {
+    case TestProfileType::kRegular:
+      return "Regular";
+    case TestProfileType::kIncognito:
+      return "Incognito";
+    case TestProfileType::kGuest:
+      return "Guest";
+  }
+}
+
+void ConfigureCommandLineForGuestMode(base::CommandLine* command_line) {
+  command_line->AppendSwitch(chromeos::switches::kGuestSession);
+  command_line->AppendSwitch(::switches::kIncognito);
+  command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "hash");
+  command_line->AppendSwitchASCII(
+      chromeos::switches::kLoginUser,
+      user_manager::GuestAccountId().GetUserEmail());
+}

chrome/browser/chromeos/web_applications/test/profile_test_helper.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_WEB_APPLICATIONS_TEST_PROFILE_TEST_HELPER_H_
+#define CHROME_BROWSER_CHROMEOS_WEB_APPLICATIONS_TEST_PROFILE_TEST_HELPER_H_
+
+#include <string>
+
+#include "base/command_line.h"
+#include "chrome/common/chrome_switches.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// Profile type to test. Provided to subclasses of TestProfileTypeMixin via
+// GetParam().
+enum class TestProfileType {
+  kRegular,
+  kIncognito,
+  kGuest,
+};
+
+// GTest string formatter for TestProfileType. Appends, e.g. "/Guest" to the end
+// of test names.
+std::string TestProfileTypeToString(
+    const ::testing::TestParamInfo<TestProfileType>& param);
+
+// Adds the necessary flags to |command_line| to start a browser test in guest
+// mode. Should be invoked in SetUpCommandLine(). Any test can call this: it is
+// not coupled to TestProfileTypeMixin.
+void ConfigureCommandLineForGuestMode(base::CommandLine* command_line);
+
+// "Mixin" for configuring a test harness to parameterize on different profile
+// types. To use it, inherit from
+//     : public TestProfileTypeMixin<BaseBrowserTest>
+// rather than BaseBrowserTest (e.g. a descendant of InProcessBrowserTest).
+// Then choose the profile types to test against. E.g.,
+//
+// INSTANTIATE_TEST_SUITE_P(All,
+//                          MySubclassOfTestProfileTypeMixin,
+//                          ::testing::Values(TestProfileType::kRegular,
+//                                            TestProfileType::kIncognito,
+//                                            TestProfileType::kGuest),
+//                          TestProfileTypeToString);
+//
+// Remember to use IN_PROC_BROWSER_TEST_P (not _F).
+template <class T>
+class TestProfileTypeMixin
+    : public T,
+      public ::testing::WithParamInterface<TestProfileType> {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    if (GetParam() == TestProfileType::kGuest) {
+      ConfigureCommandLineForGuestMode(command_line);
+    } else if (GetParam() == TestProfileType::kIncognito) {
+      command_line->AppendSwitch(::switches::kIncognito);
+    }
+    T::SetUpCommandLine(command_line);
+  }
+};
+
+#endif  // CHROME_BROWSER_CHROMEOS_WEB_APPLICATIONS_TEST_PROFILE_TEST_HELPER_H_

chrome/test/BUILD.gn

@@ -2545,6 +2545,8 @@ if (!is_android) {
         "../browser/chromeos/web_applications/settings_app_integration_browsertest.cc",
         "../browser/chromeos/web_applications/system_web_app_integration_test.cc",
         "../browser/chromeos/web_applications/system_web_app_integration_test.h",
+        "../browser/chromeos/web_applications/test/profile_test_helper.cc",
+        "../browser/chromeos/web_applications/test/profile_test_helper.h",
         "../browser/chromeos/wilco_dtc_supportd/wilco_dtc_supportd_web_request_service_browsertest.cc",
         "../browser/download/notification/download_notification_browsertest.cc",
         "../browser/drive/drive_notification_manager_factory_browsertest.cc",