CrOS Gallery: Add video file types to the Gallery manifest.

This allows them to be opened via chrome.fileManagerPrivate.executeTask.
Otherwise, they get filtered out in apps::LaunchPlatformAppWithFileHandler(..)

Note this doesn't change the default app for opening video files from
the file manager. It just allows the files to be passed via the
executeTask API. Adds FileTasksBrowserTest.DefaultHandlerChangeDetector
to ensure default handlers do not change unless it is intended to do so
in future.

Bug: 867705
Change-Id: I758b8599b6fa1f0b60f72bf3466d796ec0aedaa3
Reviewed-on: https://chromium-review.googlesource.com/1151039Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579326}

chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

@@ -23,13 +23,11 @@
 #include "chrome/browser/chromeos/crostini/crostini_manager.h"
 #include "chrome/browser/chromeos/crostini/crostini_pref_names.h"
 #include "chrome/browser/chromeos/drive/file_system_util.h"
-#include "chrome/browser/chromeos/file_manager/app_id.h"
+#include "chrome/browser/chromeos/file_manager/file_manager_test_util.h"
 #include "chrome/browser/chromeos/file_manager/mount_test_util.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/extensions/component_loader.h"
-#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_features.h"
@@ -44,11 +42,11 @@
 #include "components/drive/chromeos/file_system_interface.h"
 #include "components/drive/drive_pref_names.h"
 #include "components/drive/service/fake_drive_service.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/api/test/test_api.h"
-#include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "google_apis/drive/drive_api_parser.h"
 #include "google_apis/drive/test_util.h"
@@ -1019,22 +1017,7 @@ void FileManagerBrowserTestBase::SetUpOnMainThread() {
 
   // The test resources are setup: enable and add default ChromeOS component
   // extensions now and not before: crbug.com/831074, crbug.com/804413
-  extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
-  extensions::ExtensionService* service =
-      extensions::ExtensionSystem::Get(profile())->extension_service();
-  service->component_loader()->AddDefaultComponentExtensions(false);
-
-  // The File Manager component extension should have been added for loading
-  // into the user profile, but not into the sign-in profile.
-  CHECK(extensions::ExtensionSystem::Get(profile())
-            ->extension_service()
-            ->component_loader()
-            ->Exists(kFileManagerAppId));
-  CHECK(!extensions::ExtensionSystem::Get(
-             chromeos::ProfileHelper::GetSigninProfile())
-             ->extension_service()
-             ->component_loader()
-             ->Exists(kFileManagerAppId));
+  test::AddDefaultComponentExtensionsOnMainThread(profile());
 }
 
 bool FileManagerBrowserTestBase::GetEnableDriveFs() const {

chrome/browser/chromeos/file_manager/file_manager_test_util.cc

+// Copyright 2018 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/file_manager/file_manager_test_util.h"
+
+#include "chrome/browser/chromeos/file_manager/app_id.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/extensions/component_loader.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "extensions/browser/extension_system.h"
+
+namespace file_manager {
+namespace test {
+
+void AddDefaultComponentExtensionsOnMainThread(Profile* profile) {
+  CHECK(profile);
+
+  extensions::ComponentLoader::EnableBackgroundExtensionsForTesting();
+  extensions::ExtensionService* service =
+      extensions::ExtensionSystem::Get(profile)->extension_service();
+  service->component_loader()->AddDefaultComponentExtensions(false);
+
+  // The File Manager component extension should have been added for loading
+  // into the user profile, but not into the sign-in profile.
+  CHECK(extensions::ExtensionSystem::Get(profile)
+            ->extension_service()
+            ->component_loader()
+            ->Exists(kFileManagerAppId));
+  CHECK(!extensions::ExtensionSystem::Get(
+             chromeos::ProfileHelper::GetSigninProfile())
+             ->extension_service()
+             ->component_loader()
+             ->Exists(kFileManagerAppId));
+}
+
+}  // namespace test
+}  // namespace file_manager

chrome/browser/chromeos/file_manager/file_manager_test_util.h

+// Copyright 2018 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_FILE_MANAGER_FILE_MANAGER_TEST_UTIL_H_
+#define CHROME_BROWSER_CHROMEOS_FILE_MANAGER_FILE_MANAGER_TEST_UTIL_H_
+
+class Profile;
+
+namespace file_manager {
+namespace test {
+
+// Load the default set of component extensions used on ChromeOS. This should be
+// done in an override of InProcessBrowserTest::SetUpOnMainThread().
+void AddDefaultComponentExtensionsOnMainThread(Profile* profile);
+
+}  // namespace test
+}  // namespace file_manager
+
+#endif  // CHROME_BROWSER_CHROMEOS_FILE_MANAGER_FILE_MANAGER_TEST_UTIL_H_

chrome/browser/chromeos/file_manager/file_tasks_browsertest.cc

+// Copyright 2018 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/file_manager/file_tasks.h"
+
+#include "chrome/browser/chromeos/file_manager/app_id.h"
+#include "chrome/browser/chromeos/file_manager/file_manager_test_util.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "extensions/browser/entry_info.h"
+#include "net/base/mime_util.h"
+
+namespace file_manager {
+namespace file_tasks {
+
+class FileTasksBrowserTest : public InProcessBrowserTest {
+ public:
+  void SetUpOnMainThread() override {
+    test::AddDefaultComponentExtensionsOnMainThread(browser()->profile());
+  }
+};
+
+// Tests the default handlers for various file types in ChromeOS. This test
+// exists to ensure the default app that launches when you open a file in the
+// ChromeOS file manager does not change unexpectedly. Multiple default apps are
+// allowed to register a handler for the same file type. Without that, it is not
+// possible for an app to open that type even when given explicit direction via
+// the chrome.fileManagerPrivate.executeTask API. The current conflict
+// 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) {
+  constexpr struct {
+    const char* file_extension;
+    const char* app_id;
+    bool has_mime = true;
+  } kExpectations[] = {
+      // Images.
+      {"bmp", kGalleryAppId},
+      {"gif", kGalleryAppId},
+      {"ico", kGalleryAppId},
+      {"jpg", kGalleryAppId},
+      {"jpeg", kGalleryAppId},
+      {"png", kGalleryAppId},
+      {"webp", kGalleryAppId},
+
+      // Raw.
+      {"arw", kGalleryAppId, false},
+      {"cr2", kGalleryAppId, false},
+      {"dng", kGalleryAppId, false},
+      {"nef", kGalleryAppId, false},
+      {"nrw", kGalleryAppId, false},
+      {"orf", kGalleryAppId, false},
+      {"raf", kGalleryAppId, false},
+      {"rw2", kGalleryAppId, false},
+
+      // Video.
+      {"3gp", kVideoPlayerAppId, false},
+      {"avi", kVideoPlayerAppId, false},
+      {"m4v", kVideoPlayerAppId},
+      {"mkv", kVideoPlayerAppId, false},
+      {"mov", kVideoPlayerAppId, false},
+      {"mp4", kVideoPlayerAppId},
+      {"mpeg", kVideoPlayerAppId},
+      {"mpeg4", kVideoPlayerAppId, false},
+      {"mpg", kVideoPlayerAppId},
+      {"mpg4", kVideoPlayerAppId, false},
+      {"ogm", kVideoPlayerAppId},
+      {"ogv", kVideoPlayerAppId},
+      {"ogx", kVideoPlayerAppId, false},
+      {"webm", kVideoPlayerAppId},
+
+      // Audio.
+      {"amr", kAudioPlayerAppId, false},
+      {"flac", kAudioPlayerAppId},
+      {"m4a", kAudioPlayerAppId},
+      {"mp3", kAudioPlayerAppId},
+      {"oga", kAudioPlayerAppId},
+      {"ogg", kAudioPlayerAppId},
+      {"wav", kAudioPlayerAppId},
+  };
+
+  base::RunLoop run_loop;
+  int remaining = base::size(kExpectations);
+  auto callback = base::BindRepeating(
+      [](base::RepeatingClosure quit, int* remaining,
+         const char* file_extension, const char* expected_app_id,
+         std::unique_ptr<std::vector<FullTaskDescriptor>> result) {
+        ASSERT_TRUE(result) << file_extension;
+
+        // There can be multiple handlers. The one at index 0 will be picked by
+        // ChooseAndSetDefaultTask() since prefs should be empty in the test.
+        ASSERT_LE(1u, result->size()) << file_extension;
+
+        EXPECT_EQ(expected_app_id, result->at(0).task_descriptor().app_id)
+            << " for extension: " << file_extension;
+
+        // Verify expected behavior of ChooseAndSetDefaultTask().
+        EXPECT_TRUE(result->at(0).is_default()) << file_extension;
+        for (size_t i = 1; i < result->size(); ++i)
+          EXPECT_FALSE(result->at(i).is_default()) << file_extension;
+
+        if (--*remaining == 0)
+          quit.Run();
+      },
+      run_loop.QuitClosure(), &remaining);
+
+  const drive::DriveAppRegistry* const kDriveAppRegistry = nullptr;
+  const base::FilePath prefix = base::FilePath().AppendASCII("file");
+
+  for (const auto& test : kExpectations) {
+    base::FilePath path = prefix.AddExtension(test.file_extension);
+
+    // Fetching a mime type is part of the default app determination, but it
+    // doesn't need to succeed.
+    std::string mime_type;
+    EXPECT_EQ(test.has_mime, net::GetMimeTypeFromFile(path, &mime_type))
+        << test.file_extension;
+
+    std::vector<extensions::EntryInfo> entries = {{path, mime_type, false}};
+    std::vector<GURL> file_urls{GURL()};
+    FindAllTypesOfTasks(
+        browser()->profile(), kDriveAppRegistry, entries, file_urls,
+        base::BindRepeating(callback, test.file_extension, test.app_id));
+  }
+  run_loop.Run();
+  EXPECT_EQ(0, remaining);
+}
+
+}  // namespace file_tasks
+}  // namespace file_manager

chrome/test/BUILD.gn

@@ -1587,7 +1587,10 @@ test("browser_tests") {
         "../browser/chromeos/file_manager/file_manager_jstest.cc",
         "../browser/chromeos/file_manager/file_manager_jstest_base.cc",
         "../browser/chromeos/file_manager/file_manager_jstest_base.h",
+        "../browser/chromeos/file_manager/file_manager_test_util.cc",
+        "../browser/chromeos/file_manager/file_manager_test_util.h",
         "../browser/chromeos/file_manager/file_manager_uitest.cc",
+        "../browser/chromeos/file_manager/file_tasks_browsertest.cc",
         "../browser/chromeos/file_manager/gallery_browsertest.cc",
         "../browser/chromeos/file_manager/gallery_jstest.cc",
         "../browser/chromeos/file_manager/image_loader_jstest.cc",

ui/file_manager/gallery/manifest.json

@@ -53,7 +53,22 @@
         "nrw",
         "orf",
         "raf",
-        "rw2"
+        "rw2",
+        // Video.
+        "3gp",
+        "avi",
+        "m4v",
+        "mkv",
+        "mov",
+        "mp4",
+        "mpeg",
+        "mpeg4",
+        "mpg",
+        "mpg4",
+        "ogm",
+        "ogv",
+        "ogx",
+        "webm"
       ]
     }
   },