WebApps: Renames web app file handler registration methods.

Previously, the names of the methods in
web_app_file_handler_registration implied it was the sole place where
file handler registrations occurred. This was confusing, as Chrome OS
directly interfaces with the registry of installed apps, so these
methods were no-ops while it seemed like they should have been doing
something.

Functionally, this also reverts https://crrev.com/c/1935296, as it
doesn't align with new aim of the interface.

Bug: 1028448
Change-Id: I8630f31ee7829165ffaef39374ea286cc85caaf1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1941528Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720079}

chrome/browser/web_applications/components/BUILD.gn

@@ -79,10 +79,7 @@ source_set("components") {
   ]
 
   if (is_chromeos) {
-    sources += [
-      "web_app_file_handler_registration_chromeos.cc",
-      "web_app_shortcut_chromeos.cc",
-    ]
+    sources += [ "web_app_shortcut_chromeos.cc" ]
   }
 
   if (is_desktop_linux) {

chrome/browser/web_applications/components/file_handler_manager.cc

@@ -31,15 +31,15 @@ void FileHandlerManager::Start() {
 
 void FileHandlerManager::OnWebAppUninstalled(const AppId& installed_app_id) {
   if (base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI) &&
-      OsSupportsWebAppFileHandling()) {
-    UnregisterFileHandlersForWebApp(installed_app_id, profile_);
+      ShouldRegisterFileHandlersWithOs()) {
+    UnregisterFileHandlersWithOs(installed_app_id, profile_);
   }
 }
 
 void FileHandlerManager::OnWebAppProfileWillBeDeleted(const AppId& app_id) {
   if (base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI) &&
-      OsSupportsWebAppFileHandling()) {
-    UnregisterFileHandlersForWebApp(app_id, profile_);
+      ShouldRegisterFileHandlersWithOs()) {
+    UnregisterFileHandlersWithOs(app_id, profile_);
   }
 }
 
@@ -49,7 +49,7 @@ void FileHandlerManager::OnAppRegistrarDestroyed() {
 
 void FileHandlerManager::OnShortcutsCreated(const AppId& installed_app_id) {
   if (!base::FeatureList::IsEnabled(blink::features::kFileHandlingAPI) ||
-      !OsSupportsWebAppFileHandling()) {
+      !ShouldRegisterFileHandlersWithOs()) {
     return;
   }
 
@@ -62,8 +62,8 @@ void FileHandlerManager::OnShortcutsCreated(const AppId& installed_app_id) {
       GetFileExtensionsFromFileHandlers(*file_handlers);
   std::set<std::string> mime_types =
       GetMimeTypesFromFileHandlers(*file_handlers);
-  RegisterFileHandlersForWebApp(installed_app_id, app_name, profile_,
-                                file_extensions, mime_types);
+  RegisterFileHandlersWithOs(installed_app_id, app_name, profile_,
+                             file_extensions, mime_types);
 }
 
 void FileHandlerManager::OnShortcutManagerDestroyed() {

chrome/browser/web_applications/components/web_app_file_handler_registration.cc

@@ -10,27 +10,28 @@
 namespace web_app {
 
 // This block defines stub implementations of OS specific methods for
-// FileHandling. Currently, Windows and Desktop Linux and Chrome OS have
+// FileHandling. Currently, Windows and Desktop Linux (but not Chrome OS) have
 // their own implementations.
 //
-// Note: OS_LINUX includes OS_CHROMEOS
-#if !defined(OS_WIN) && !defined(OS_LINUX)
-bool OsSupportsWebAppFileHandling() {
+// Note: Because OS_LINUX includes OS_CHROMEOS be sure to use the stub on
+// OS_CHROMEOS.
+#if !defined(OS_WIN) && !(defined(OS_LINUX) && !defined(OS_CHROMEOS))
+bool ShouldRegisterFileHandlersWithOs() {
   return false;
 }
 
-void RegisterFileHandlersForWebApp(const AppId& app_id,
-                                   const std::string& app_name,
-                                   Profile* profile,
-                                   const std::set<std::string>& file_extensions,
-                                   const std::set<std::string>& mime_types) {
-  DCHECK(OsSupportsWebAppFileHandling());
-  // Stub function for OS's that don't support Web App file handling yet.
+void RegisterFileHandlersWithOs(const AppId& app_id,
+                                const std::string& app_name,
+                                Profile* profile,
+                                const std::set<std::string>& file_extensions,
+                                const std::set<std::string>& mime_types) {
+  DCHECK(ShouldRegisterFileHandlersWithOs());
+  // Stub function for OS's which don't register file handlers with the OS.
 }
 
-void UnregisterFileHandlersForWebApp(const AppId& app_id, Profile* profile) {
-  DCHECK(OsSupportsWebAppFileHandling());
-  // Stub function for OS's that don't support Web App file handling yet.
+void UnregisterFileHandlersWithOs(const AppId& app_id, Profile* profile) {
+  DCHECK(ShouldRegisterFileHandlersWithOs());
+  // Stub function for OS's which don't register file handlers with the OS.
 }
 #endif
 

chrome/browser/web_applications/components/web_app_file_handler_registration.h

@@ -14,23 +14,27 @@ class Profile;
 
 namespace web_app {
 
-// Returns true if Chrome supports WebApp File Handling on this OS.
-bool OsSupportsWebAppFileHandling();
+// True if file handlers are managed externally by the operating system, and
+// Chrome supports file handling on this operating system.
+// In practice, this is false on Chrome OS (as Chrome OS uses Chrome's installed
+// apps to find file handlers), and on operating systems where Chrome doesn't
+// know how to register file handlers.
+bool ShouldRegisterFileHandlersWithOs();
 
 // Do OS-specific registration to handle opening files with the specified
 // |file_extensions| and |mime_types| with the PWA with the specified |app_id|.
 // This may also involve creating a shim app to launch Chrome from.
 // Note: Some operating systems (such as Chrome OS) may not need to do any work
 // here.
-void RegisterFileHandlersForWebApp(const AppId& app_id,
-                                   const std::string& app_name,
-                                   Profile* profile,
-                                   const std::set<std::string>& file_extensions,
-                                   const std::set<std::string>& mime_types);
+void RegisterFileHandlersWithOs(const AppId& app_id,
+                                const std::string& app_name,
+                                Profile* profile,
+                                const std::set<std::string>& file_extensions,
+                                const std::set<std::string>& mime_types);
 
 // Undo the file extensions registration for the PWA with specified |app_id|.
 // If a shim app was required, also removes the shim app.
-void UnregisterFileHandlersForWebApp(const AppId& app_id, Profile* profile);
+void UnregisterFileHandlersWithOs(const AppId& app_id, Profile* profile);
 
 }  // namespace web_app
 

chrome/browser/web_applications/components/web_app_file_handler_registration_chromeos.cc

-// Copyright 2019 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/web_applications/components/web_app_file_handler_registration.h"
-
-#include "base/bind.h"
-#include "base/bind_helpers.h"
-#include "chrome/browser/web_applications/components/app_shortcut_manager.h"
-#include "chrome/browser/web_applications/components/web_app_provider_base.h"
-#include "chrome/browser/web_applications/components/web_app_shortcut.h"
-
-namespace web_app {
-
-bool OsSupportsWebAppFileHandling() {
-  return true;
-}
-
-void RegisterFileHandlersForWebApp(const AppId& app_id,
-                                   const std::string& app_name,
-                                   Profile* profile,
-                                   const std::set<std::string>& file_extensions,
-                                   const std::set<std::string>& mime_types) {
-  // ChromeOS talks directly to FileHandlerManager for file handler
-  // registrations.
-}
-
-void UnregisterFileHandlersForWebApp(const AppId& app_id, Profile* profile) {
-  // ChromeOS talks directly to FileHandlerManager for file handler
-  // registrations.
-}
-
-}  // namespace web_app

chrome/browser/web_applications/components/web_app_file_handler_registration_linux.cc

@@ -32,22 +32,22 @@ void OnShortcutInfoReceived(const std::set<std::string> mime_types,
 
 }  // namespace
 
-bool OsSupportsWebAppFileHandling() {
+bool ShouldRegisterFileHandlersWithOs() {
   return true;
 }
 
-void RegisterFileHandlersForWebApp(const AppId& app_id,
-                                   const std::string& app_name,
-                                   Profile* profile,
-                                   const std::set<std::string>& file_extensions,
-                                   const std::set<std::string>& mime_types) {
+void RegisterFileHandlersWithOs(const AppId& app_id,
+                                const std::string& app_name,
+                                Profile* profile,
+                                const std::set<std::string>& file_extensions,
+                                const std::set<std::string>& mime_types) {
   AppShortcutManager& shortcut_manager =
       WebAppProviderBase::GetProviderBase(profile)->shortcut_manager();
   shortcut_manager.GetShortcutInfoForApp(
       app_id, base::BindOnce(OnShortcutInfoReceived, mime_types));
 }
 
-void UnregisterFileHandlersForWebApp(const AppId& app_id, Profile* profile) {
+void UnregisterFileHandlersWithOs(const AppId& app_id, Profile* profile) {
   // TODO(harrisjay): Add support for unregistering file handlers.
 }
 

chrome/browser/web_applications/components/web_app_file_handler_registration_win.cc

@@ -14,19 +14,19 @@ namespace web_app {
 const base::FilePath::StringPieceType kLastBrowserFile(
     FILE_PATH_LITERAL("Last Browser"));
 
-bool OsSupportsWebAppFileHandling() {
+bool ShouldRegisterFileHandlersWithOs() {
   return true;
 }
 
-void RegisterFileHandlersForWebApp(const AppId& app_id,
-                                   const std::string& app_name,
-                                   Profile* profile,
-                                   const std::set<std::string>& file_extensions,
-                                   const std::set<std::string>& mime_types) {
+void RegisterFileHandlersWithOs(const AppId& app_id,
+                                const std::string& app_name,
+                                Profile* profile,
+                                const std::set<std::string>& file_extensions,
+                                const std::set<std::string>& mime_types) {
   // TODO(davidbienvenu): Setup shim app and windows registry for this |app_id|.
 }
 
-void UnregisterFileHandlersForWebApp(const AppId& app_id, Profile* profile) {
+void UnregisterFileHandlersWithOs(const AppId& app_id, Profile* profile) {
   // TODO(davidbienvenu): Cleanup windows registry entries for this |app_id|.
 }