Delete Video Player app entry points.

Delete the Video Player Hidden feature flag and other entry points into the Video Player app. This will prepare the app for deletion, which will happen in a future milestone.

See https://chromium-review.googlesource.com/c/chromium/src/+/2780996 for a similar example with the deletion Gallery app entry points.

Bug: b/171154148, b/186168810
Change-Id: I5bbc3ba64168fd4c8711aa8efb3a1e5295fd10a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2936336
Commit-Queue: Mitchell McDermott <mcdermottm@google.com>
Reviewed-by: Patti <patricialor@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#891511}

ash/constants/ash_features.cc

@@ -339,11 +339,6 @@ const base::Feature kExoLockNotification{"ExoLockNotification",
 const base::Feature kFamilyLinkOnSchoolDevice{"FamilyLinkOnSchoolDevice",
                                               base::FEATURE_ENABLED_BY_DEFAULT};
 
-// Whether to "hide" the legacy video player chrome app. Videos will instead be
-// handled by the media app. See https://crbug.com/1158531.
-const base::Feature kVideoPlayerAppHidden{"VideoPlayerAppHidden",
-                                          base::FEATURE_ENABLED_BY_DEFAULT};
-
 // Enables partitioning of removable disks in file manager.
 const base::Feature kFilesSinglePartitionFormat{
     "FilesSinglePartitionFormat", base::FEATURE_DISABLED_BY_DEFAULT};

ash/constants/ash_features.h

@@ -160,8 +160,6 @@ extern const base::Feature kExoLockNotification;
 COMPONENT_EXPORT(ASH_CONSTANTS)
 extern const base::Feature kFamilyLinkOnSchoolDevice;
 COMPONENT_EXPORT(ASH_CONSTANTS)
-extern const base::Feature kVideoPlayerAppHidden;
-COMPONENT_EXPORT(ASH_CONSTANTS)
 extern const base::Feature kFilesSinglePartitionFormat;
 COMPONENT_EXPORT(ASH_CONSTANTS) extern const base::Feature kFilesSWA;
 COMPONENT_EXPORT(ASH_CONSTANTS) extern const base::Feature kFilesTrash;

chrome/browser/about_flags.cc

@@ -6037,9 +6037,6 @@ const FeatureEntry kFeatureEntries[] = {
     {"media-app-handles-pdf", flag_descriptions::kMediaAppHandlesPdfName,
      flag_descriptions::kMediaAppHandlesPdfDescription, kOsCrOS,
      FEATURE_VALUE_TYPE(chromeos::features::kMediaAppHandlesPdf)},
-    {"media-app-video", flag_descriptions::kMediaAppVideoName,
-     flag_descriptions::kMediaAppVideoDescription, kOsCrOS,
-     FEATURE_VALUE_TYPE(ash::features::kVideoPlayerAppHidden)},
     {"release-notes-notification",
      flag_descriptions::kReleaseNotesNotificationName,
      flag_descriptions::kReleaseNotesNotificationDescription, kOsCrOS,

chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

@@ -313,11 +313,11 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("fileDisplayStartupError")));
 
 WRAPPED_INSTANTIATE_TEST_SUITE_P(
-    OpenVideoFiles, /* open_video_files.js */
+    OpenVideoMediaApp, /* open_video_media_app.js */
     FilesAppBrowserTest,
-    ::testing::Values(TestCase("videoOpenDownloads").InGuestMode(),
-                      TestCase("videoOpenDownloads"),
-                      TestCase("videoOpenDrive")));
+    ::testing::Values(TestCase("videoOpenDownloads").MediaSwa().InGuestMode(),
+                      TestCase("videoOpenDownloads").MediaSwa(),
+                      TestCase("videoOpenDrive").MediaSwa()));
 
 WRAPPED_INSTANTIATE_TEST_SUITE_P(
     OpenAudioFiles, /* open_audio_files.js */
@@ -542,7 +542,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
         TestCase("openQuickViewTabIndexText"),
         TestCase("openQuickViewTabIndexHtml"),
         TestCase("openQuickViewTabIndexAudio"),
-        TestCase("openQuickViewTabIndexVideo"),
+        TestCase("openQuickViewTabIndexVideo").MediaSwa(),
         TestCase("openQuickViewTabIndexDeleteDialog"),
         TestCase("openQuickViewTabIndexDeleteDialog").EnableTrash(),
         TestCase("openQuickViewToggleInfoButtonKeyboard"),

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -1694,10 +1694,6 @@ void FileManagerBrowserTestBase::SetUpCommandLine(
   // Make sure to run the ARC storage UI toast tests.
   enabled_features.push_back(arc::kUsbStorageUIFeature);
 
-  // FileManager tests exist for the deprecated video player app, which will be
-  // removed, along with the kVideoPlayerAppHidden flag by m93.
-  disabled_features.push_back(ash::features::kVideoPlayerAppHidden);
-
   if (options.files_swa) {
     enabled_features.push_back(chromeos::features::kFilesSWA);
   } else {

chrome/browser/chromeos/file_manager/file_manager_test_util.cc

@@ -115,6 +115,12 @@ void AddDefaultComponentExtensionsOnMainThread(Profile* profile) {
   extensions::ExtensionService* service =
       extensions::ExtensionSystem::Get(profile)->extension_service();
   service->component_loader()->AddDefaultComponentExtensions(false);
+  // AddDefaultComponentExtensions() is normally invoked during
+  // ExtensionService::Init() which also invokes UninstallMigratedExtensions().
+  // Invoke it here as well, otherwise migrated extensions will remain installed
+  // for the duration of the test. Note this may result in immediately
+  // uninstalling an extension just installed above.
+  service->UninstallMigratedExtensionsForTest();
 
   // The File Manager component extension should have been added for loading
   // into the user profile, but not into the sign-in profile.

chrome/browser/chromeos/file_manager/file_tasks.cc

@@ -187,18 +187,9 @@ void AdjustTasksForMediaApp(const std::vector<extensions::EntryInfo>& entries,
   if (media_app_task == tasks->end())
     return;
 
-  // If the video player is still available, check if it was offered and early
-  // exit. Unfortunately because obscure videos must be handled with extension
-  // matches, both the MediaApp and Video have equal priority for those. That
-  // causes them to be ordered by extension ID. Video is "jhd.." and Media is
-  // "jcg..". So to ensure the Video app takes precedence, we pretend the media
-  // app is actually a wildcard match. Note the video player will only be
-  // offered if _all_ the files in the selection are videos, and it is not
-  // already hidden by flags.
-  if (task_for_app(kVideoPlayerAppId) != tasks->end()) {
-    media_app_task->set_is_generic_file_handler(true);
-    return;
-  }
+  // Video Player app was replaced by media app in m91, deprecated in m93 and
+  // will be deleted m94.
+  DCHECK(task_for_app(kVideoPlayerAppId) == tasks->end());
 
   // TOOD(crbug/1071289): For a while is_file_extension_match() would always be
   // false for System Web App manifests, even when specifying extension matches.
@@ -242,7 +233,6 @@ bool IsFallbackFileHandler(const FullTaskDescriptor& task) {
   // an app other than kMediaAppId to be the default (b/153387960).
   constexpr const char* kBuiltInApps[] = {
       kFileManagerAppId,
-      kVideoPlayerAppId,
       kTextEditorAppId,
       kAudioPlayerAppId,
       extension_misc::kQuickOfficeComponentExtensionId,
@@ -677,16 +667,9 @@ void FindFileHandlerTasks(Profile* profile,
         !extensions::util::IsIncognitoEnabled(extension->id(), profile))
       continue;
 
-    if (base::FeatureList::IsEnabled(ash::features::kVideoPlayerAppHidden) &&
-        extension->id() == kVideoPlayerAppId) {
-      // "Hide" the video player component extension (i.e. skip the rest of this
-      // loop which would add it as a handler). Note this is not achieved by
-      // preventing the extension install in component_loader.cc. The component
-      // extension must first be properly "uninstalled" in a milestone after all
-      // entrypoints are removed.
-      // TODO(crbug/1158531): Remove this when the video player is uninstalled.
-      continue;
-    }
+    // Video player should no longer install itself or its file handlers
+    // starting in m93.
+    DCHECK_NE(kVideoPlayerAppId, extension->id());
 
     typedef std::vector<extensions::FileHandlerMatch> FileHandlerMatchList;
     FileHandlerMatchList file_handlers =

chrome/browser/chromeos/file_manager/file_tasks_browsertest.cc

@@ -141,22 +141,9 @@ class FileTasksBrowserTestBase
 class FileTasksBrowserTest : public FileTasksBrowserTestBase {
  public:
   FileTasksBrowserTest() {
-    // Enable Media App Video, but no PDF support.
-    scoped_feature_list_.InitWithFeatures(
-        {chromeos::features::kVideoPlayerAppHidden},
-        {ash::features::kMediaAppHandlesPdf});
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-class FileTasksBrowserTestNoVideo : public FileTasksBrowserTestBase {
- public:
-  FileTasksBrowserTestNoVideo() {
-    // Disable Media App video support.
-    scoped_feature_list_.InitWithFeatures(
-        {}, {chromeos::features::kVideoPlayerAppHidden});
+    // Enable Media App without PDF support.
+    scoped_feature_list_.InitWithFeatures({},
+                                          {ash::features::kMediaAppHandlesPdf});
   }
 
  private:
@@ -187,22 +174,6 @@ class FileTasksBrowserTestWithPdf : public FileTasksBrowserTestBase {
 // The "deprecated" lists are those that use the old ChromeApps as handlers and
 // can be removed when those are gone.
 
-constexpr Expectation kVideoDeprecatedExpectations[] = {
-    {"3gp", kVideoPlayerAppId, "application/octet-stream"},
-    {"avi", kVideoPlayerAppId, "application/octet-stream"},
-    {"m4v", kVideoPlayerAppId},
-    {"mkv", kVideoPlayerAppId, "video/webm"},
-    {"mov", kVideoPlayerAppId, "application/octet-stream"},
-    {"mp4", kVideoPlayerAppId},
-    {"mpeg", kVideoPlayerAppId},
-    {"mpeg4", kVideoPlayerAppId, "video/mpeg"},
-    {"mpg", kVideoPlayerAppId},
-    {"mpg4", kVideoPlayerAppId, "video/mpeg"},
-    {"ogm", kVideoPlayerAppId},
-    {"ogv", kVideoPlayerAppId},
-    {"ogx", kVideoPlayerAppId, "video/ogg"},
-    {"webm", kVideoPlayerAppId}};
-
 constexpr Expectation kAudioDeprecatedExpectations[] = {
     {"amr", kAudioPlayerAppId, "application/octet-stream"},
     {"flac", kAudioPlayerAppId},
@@ -343,22 +314,13 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, DefaultHandlerChangeDetector) {
   TestExpectationsAgainstDefaultTasks(expectations);
 }
 
-// Tests the default handlers that are different with Video support enabled.
+// Test to ensure the media app handle known video file extensions.
 IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, VideoHandlerChangeDetector) {
   std::vector<Expectation> expectations(std::begin(kVideoExpectations),
                                         std::end(kVideoExpectations));
   TestExpectationsAgainstDefaultTasks(expectations);
 }
 
-// Tests the default handlers that are different with Video support disabled.
-IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestNoVideo,
-                       VideoHandlerChangeDetector) {
-  std::vector<Expectation> expectations(
-      std::begin(kVideoDeprecatedExpectations),
-      std::end(kVideoDeprecatedExpectations));
-  TestExpectationsAgainstDefaultTasks(expectations);
-}
-
 // Tests the default handlers that are different with PDF support enabled.
 IN_PROC_BROWSER_TEST_P(FileTasksBrowserTestWithPdf, PdfHandlerChangeDetector) {
   std::vector<Expectation> expectations(std::begin(kMediaAppPdfExpectations),
@@ -458,13 +420,6 @@ INSTANTIATE_TEST_SUITE_P(All,
                                            TestProfileType::kGuest),
                          TestProfileTypeToString);
 
-INSTANTIATE_TEST_SUITE_P(All,
-                         FileTasksBrowserTestNoVideo,
-                         ::testing::Values(TestProfileType::kRegular,
-                                           TestProfileType::kIncognito,
-                                           TestProfileType::kGuest),
-                         TestProfileTypeToString);
-
 INSTANTIATE_TEST_SUITE_P(All,
                          FileTasksBrowserTestWithPdf,
                          ::testing::Values(TestProfileType::kRegular,

chrome/browser/extensions/component_loader.cc

@@ -377,8 +377,14 @@ void ComponentLoader::AddFileManagerExtension() {
 }
 
 void ComponentLoader::AddVideoPlayerExtension() {
-  Add(IDR_VIDEO_PLAYER_MANIFEST,
-      base::FilePath(FILE_PATH_LITERAL("video_player")));
+  // TODO(b/186168810): Delete this entirely around M96 when it has has a
+  // chance to be cleaned up.
+  if (extensions::ExtensionPrefs::Get(profile_)
+          ->ShouldInstallObsoleteComponentExtension(
+              file_manager::kVideoPlayerAppId)) {
+    Add(IDR_VIDEO_PLAYER_MANIFEST,
+        base::FilePath(FILE_PATH_LITERAL("video_player")));
+  }
 }
 
 void ComponentLoader::AddAudioPlayerExtension() {

chrome/browser/extensions/extension_service.cc

@@ -149,7 +149,11 @@ const char* const kMigratedExtensionIds[] = {
 // Note: We preserve at least one entry here for continued testing coverage.
 const char* const kObsoleteComponentExtensionIds[] = {
     // Obsolete since M91.
-    "nlkncpkkdoccmpiclbokaimcnedabhhm"  // Gallery
+    "nlkncpkkdoccmpiclbokaimcnedabhhm",  // Gallery
+    // The Video Player chrome app became obsolete in m93. This entry can be
+    // removed after references to kVideoPlayerAppId in component_loader.cc
+    // are removed (and it's no longer used for regression tests).
+    "jcgeabjmjgoblfofpppfkcoakmfobdko"  // Video Player
 };
 
 }  // namespace