Skip to content

Commit

Permalink
Remove File Handling permissions code.
Browse files Browse the repository at this point in the history
Remove kDesktopPWAsFileHandlingSettingsGated which is enabled by
default everywhere. Remove all code that is no longer reachable.
There should be no behavioral change.[1]

Removed code includes:

* permission types in permissions code
* permissions logic in PWA code
* bespoke permission prompt and strings, including code to control
  showing this after the PWA window opens
* now-obsolete tests
* settings Web UI

[1] The File Handling permission page, i.e.
chrome://settings/content/fileHandlers, will no longer be visible,
however this should have already been hidden since it was
non-functional.

Bug: 1245301, 1272000
Change-Id: I39b163deaa1640a1c8f3aeb2dd5027af593e23b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3293841
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948176}
  • Loading branch information
Evan Stade authored and Chromium LUCI CQ committed Dec 3, 2021
1 parent de4a42d commit 66ef2a8
Show file tree
Hide file tree
Showing 111 changed files with 146 additions and 2,103 deletions.
2 changes: 0 additions & 2 deletions android_webview/browser/aw_permission_manager.cc
Expand Up @@ -343,7 +343,6 @@ void AwPermissionManager::RequestPermissions(
case PermissionType::WINDOW_PLACEMENT:
case PermissionType::FONT_ACCESS:
case PermissionType::DISPLAY_CAPTURE:
case PermissionType::FILE_HANDLING:
NOTIMPLEMENTED() << "RequestPermissions is not implemented for "
<< static_cast<int>(permissions[i]);
pending_request_raw->SetPermissionStatus(permissions[i],
Expand Down Expand Up @@ -549,7 +548,6 @@ void AwPermissionManager::CancelPermissionRequest(int request_id) {
case PermissionType::WINDOW_PLACEMENT:
case PermissionType::FONT_ACCESS:
case PermissionType::DISPLAY_CAPTURE:
case PermissionType::FILE_HANDLING:
NOTIMPLEMENTED() << "CancelPermission not implemented for "
<< static_cast<int>(permission);
break;
Expand Down
2 changes: 0 additions & 2 deletions ash/webui/camera_app_ui/camera_app_ui.cc
Expand Up @@ -197,8 +197,6 @@ CameraAppUI::CameraAppUI(content::WebUI* web_ui,
host_origin, ContentSettingsType::MEDIASTREAM_MIC);
allowlist->RegisterAutoGrantedPermission(
host_origin, ContentSettingsType::MEDIASTREAM_CAMERA);
allowlist->RegisterAutoGrantedPermission(host_origin,
ContentSettingsType::FILE_HANDLING);
allowlist->RegisterAutoGrantedPermission(
host_origin, ContentSettingsType::FILE_SYSTEM_READ_GUARD);
allowlist->RegisterAutoGrantedPermission(
Expand Down
1 change: 0 additions & 1 deletion ash/webui/media_app_ui/media_app_ui.cc
Expand Up @@ -108,7 +108,6 @@ MediaAppUI::MediaAppUI(content::WebUI* web_ui,
allowlist->RegisterAutoGrantedPermissions(
host_origin, {
ContentSettingsType::COOKIES,
ContentSettingsType::FILE_HANDLING,
ContentSettingsType::FILE_SYSTEM_READ_GUARD,
ContentSettingsType::FILE_SYSTEM_WRITE_GUARD,
ContentSettingsType::IMAGES,
Expand Down
Expand Up @@ -493,7 +493,7 @@ public void testFetchAllPreferencesForSingleOrigin() {
// If the ContentSettingsType.NUM_TYPES value changes *and* a new value has been exposed on
// Android, then please update this code block to include a test for your new type.
// Otherwise, just update count in the assert.
Assert.assertEquals(76, ContentSettingsType.NUM_TYPES);
Assert.assertEquals(75, ContentSettingsType.NUM_TYPES);
websitePreferenceBridge.addContentSettingException(
new ContentSettingException(ContentSettingsType.COOKIES, googleOrigin,
ContentSettingValues.DEFAULT, preferenceSource));
Expand Down
8 changes: 1 addition & 7 deletions chrome/app/generated_resources.grd
Expand Up @@ -10995,17 +10995,11 @@ Please help our engineers fix this problem. Tell us what happened right before y
</message>
</if>

<!-- File Handling permission prompt -->
<!-- File Handling launch prompt -->
<if expr="not is_android">
<message name="IDS_WEB_APP_FILE_HANDLING_LIST_SEPARATOR" desc="Text/punctuation that separates items in a lists. For example, this is used in a list of files, such as 'todo_list.txt, shopping_list.txt', and file types/extensions, such as '.txt, .csv.'" meaning="List separator for files.">
, '''
</message>
<message name="IDS_WEB_APP_FILE_HANDLING_PERMISSION_TITLE" desc="Title of permission request dialog that is when the user is opening a file using a PWA.">
Open file?
</message>
<message name="IDS_WEB_APP_FILE_HANDLING_PERMISSION_TITLE_MULTIPLE" desc="Title of permission request dialog that is shown when the user is opening multiple files using a PWA.">
Open files?
</message>
<message name="IDS_WEB_APP_FILE_HANDLING_PERMISSION_DESCRIPTION" desc="Descriptive text in permission request dialog that is shown when the user is opening a file using a PWA.">
<ph name="APP_ORIGIN">$1<ex>example.com</ex></ph> wants to open this file:
</message>
Expand Down

This file was deleted.

This file was deleted.

21 changes: 0 additions & 21 deletions chrome/app/settings_strings.grdp
Expand Up @@ -2193,21 +2193,6 @@
<message name="IDS_SETTINGS_SITE_SETTINGS_DEVICE_USE_BLOCKED_EXCEPTIONS" desc="Label for the blocked exceptions site list of the device use content setting.">
Not allowed to know when you're actively using your device
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_DESCRIPTION" desc="Description of the file handling content setting.">
Web apps typically ask to open certain types of files so you can work on those files where you want, like opening documents in your preferred word processor
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_ALLOWED" desc="Label for the enabled option of the file handling content setting.">
Web apps can ask to open types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_BLOCKED" desc="Label for the disabled option of the file handling content setting.">
Don't allow web apps to open types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_ALLOWED_EXCEPTIONS" desc="Label for the allowed exceptions site list of the file handling content setting.">
Allowed to open types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_BLOCKED_EXCEPTIONS" desc="Label for the blocked exceptions site list of the file handling content setting.">
Not allowed to open types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_SYSTEM_WRITE_DESCRIPTION" desc="Description of the file system write content setting.">
Sites usually access files and folders on your device for features like automatically saving your work
</message>
Expand Down Expand Up @@ -2840,12 +2825,6 @@
<message name="IDS_SETTINGS_SITE_SETTINGS_IDLE_DETECTION_BLOCK" desc="The block label for the idle detection site settings.">
Block sites from knowing when you're actively using this device
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_ASK" desc="The ask label for the file handling site settings.">
Ask when a web app wants to open types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_FILE_HANDLING_BLOCK" desc="The block label for the file handling site settings.">
Block web apps from opening types of files
</message>
<message name="IDS_SETTINGS_SITE_SETTINGS_ALLOWED" desc="A generic Allowed label to show in Site Settings when Allow is NOT the recommended option.">
Allowed
</message>
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

50 changes: 22 additions & 28 deletions chrome/browser/apps/app_shim/web_app_shim_manager_delegate_mac.cc
Expand Up @@ -265,38 +265,32 @@ void WebAppShimManagerDelegate::LaunchApp(
}
}

if (base::FeatureList::IsEnabled(
features::kDesktopPWAsFileHandlingSettingsGated)) {
if (!launch_files.empty()) {
WebAppProvider* const provider = WebAppProvider::GetForWebApps(profile);
absl::optional<GURL> file_handler_url =
provider->os_integration_manager().GetMatchingFileHandlerURL(
app_id, launch_files);
if (!file_handler_url) {
CancelAppLaunch(profile, app_id);
return;
}

params.launch_files = launch_files;
if (!launch_files.empty()) {
WebAppProvider* const provider = WebAppProvider::GetForWebApps(profile);
absl::optional<GURL> file_handler_url =
provider->os_integration_manager().GetMatchingFileHandlerURL(
app_id, launch_files);
if (!file_handler_url) {
CancelAppLaunch(profile, app_id);
return;
}

const WebApp* web_app = provider->registrar().GetAppById(app_id);
DCHECK(web_app);
params.launch_files = launch_files;

if (web_app->file_handler_approval_state() ==
ApiApprovalState::kRequiresPrompt) {
chrome::ShowWebAppFileLaunchDialog(
launch_files, profile, app_id,
base::BindOnce(&UserChoiceDialogCompleted, std::move(params),
profile));
return;
}
const WebApp* web_app = provider->registrar().GetAppById(app_id);
DCHECK(web_app);

DCHECK_EQ(ApiApprovalState::kAllowed,
web_app->file_handler_approval_state());
if (web_app->file_handler_approval_state() ==
ApiApprovalState::kRequiresPrompt) {
chrome::ShowWebAppFileLaunchDialog(
launch_files, profile, app_id,
base::BindOnce(&UserChoiceDialogCompleted, std::move(params),
profile));
return;
}
} else {
// !features::kDesktopPWAsFileHandlingSettingsGated:
params.launch_files = launch_files;

DCHECK_EQ(ApiApprovalState::kAllowed,
web_app->file_handler_approval_state());
}

LaunchAppWithParams(profile, std::move(params));
Expand Down
Expand Up @@ -6,31 +6,17 @@

#include <memory.h>

#include "ash/webui/file_manager/url_constants.h"
#include "base/values.h"
#include "chrome/browser/ash/file_manager/file_manager_string_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h"
#include "ui/webui/webui_allowlist.h"
#include "url/gurl.h"
#include "url/origin.h"

using ash::file_manager::kChromeUIFileManagerURL;

ChromeFileManagerUIDelegate::ChromeFileManagerUIDelegate(content::WebUI* web_ui)
: web_ui_(web_ui) {
DCHECK(web_ui_);

auto* browser_context = web_ui_->GetWebContents()->GetBrowserContext();
// Register auto-granted permissions.
auto* allowlist = WebUIAllowlist::GetOrCreate(browser_context);
const url::Origin host_origin =
url::Origin::Create(GURL(kChromeUIFileManagerURL));
allowlist->RegisterAutoGrantedPermissions(
host_origin, {ContentSettingsType::FILE_HANDLING});
}

void ChromeFileManagerUIDelegate::PopulateLoadTimeData(
Expand Down
21 changes: 1 addition & 20 deletions chrome/browser/content_settings/chrome_content_settings_utils.cc
Expand Up @@ -5,17 +5,15 @@
#include "chrome/browser/content_settings/chrome_content_settings_utils.h"

#include "base/metrics/histogram_macros.h"
#include "build/build_config.h"

#if !defined(OS_ANDROID)
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/location_bar/location_bar.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
#include "ui/base/l10n/l10n_util.h"
#endif

namespace content_settings {
Expand Down Expand Up @@ -45,21 +43,4 @@ void UpdateLocationBarUiForWebContents(content::WebContents* web_contents) {
#endif
}

#if !defined(OS_ANDROID)
std::u16string GetPermissionDetailString(Profile* profile,
ContentSettingsType content_type,
const GURL& url) {
if (!url.is_valid())
return {};

switch (content_type) {
case ContentSettingsType::FILE_HANDLING:
return web_app::GetFileTypeAssociationsHandledByWebAppsForDisplay(profile,
url);
default:
return {};
}
}
#endif

} // namespace content_settings
17 changes: 0 additions & 17 deletions chrome/browser/content_settings/chrome_content_settings_utils.h
Expand Up @@ -5,17 +5,10 @@
#ifndef CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_
#define CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_

#include "build/build_config.h"
#include "components/content_settings/core/common/content_settings.h"

// Put utility functions only used by //chrome code here. If a function declared
// here would be meaningfully shared with other platforms, consider moving it to
// components/content_settings/core/browser/content_settings_utils.h.

class GURL;
class Profile;
enum class ContentSettingsType;

namespace content {
class WebContents;
} // namespace content
Expand Down Expand Up @@ -56,16 +49,6 @@ void RecordPopupsAction(PopupsAction action);
// Calls UpdateContentSettingsIcons on the |LocationBar| for |web_contents|.
void UpdateLocationBarUiForWebContents(content::WebContents* web_contents);

#if !defined(OS_ANDROID)
// Returns a string for display alongside UI that describes the given content
// setting in `profile`. This string gives extra, pertinent details about the
// content setting. `url` represents the site for which the given setting
// applies.
std::u16string GetPermissionDetailString(Profile* profile,
ContentSettingsType content_type,
const GURL& url);
#endif

} // namespace content_settings

#endif // CHROME_BROWSER_CONTENT_SETTINGS_CHROME_CONTENT_SETTINGS_UTILS_H_
Expand Up @@ -142,6 +142,8 @@ TEST_F(ContentSettingsDefaultProviderTest, DiscardObsoletePreferences) {
"profile.default_content_setting_values.plugins";
const char kObsoletePluginsDataDefaultPref[] =
"profile.default_content_setting_values.flash_data";
const char kObsoleteFileHandlingDefaultPref[] =
"profile.default_content_setting_values.file_handling";
#endif
static const char kGeolocationPrefPath[] =
"profile.default_content_setting_values.geolocation";
Expand All @@ -153,20 +155,23 @@ TEST_F(ContentSettingsDefaultProviderTest, DiscardObsoletePreferences) {
prefs->SetInteger(kMouselockPrefPath, CONTENT_SETTING_ALLOW);
prefs->SetInteger(kObsoletePluginsDefaultPref, CONTENT_SETTING_ALLOW);
prefs->SetInteger(kObsoletePluginsDataDefaultPref, CONTENT_SETTING_ALLOW);
prefs->SetInteger(kObsoleteFileHandlingDefaultPref, CONTENT_SETTING_ALLOW);
#endif
prefs->SetInteger(kGeolocationPrefPath, CONTENT_SETTING_BLOCK);

// Instantiate a new DefaultProvider; can't use |provider_| because we want to
// test the constructor's behavior after setting the above.
DefaultProvider provider(prefs, false);

// Check that fullscreen and mouselock have been deleted.
// Check that obsolete prefs have been deleted.
EXPECT_FALSE(prefs->HasPrefPath(kFullscreenPrefPath));
#if !defined(OS_ANDROID)
EXPECT_FALSE(prefs->HasPrefPath(kMouselockPrefPath));
EXPECT_FALSE(prefs->HasPrefPath(kObsoletePluginsDefaultPref));
EXPECT_FALSE(prefs->HasPrefPath(kObsoletePluginsDataDefaultPref));
EXPECT_FALSE(prefs->HasPrefPath(kObsoleteFileHandlingDefaultPref));
#endif
// Check that non-obsolete prefs have not been touched.
EXPECT_TRUE(prefs->HasPrefPath(kGeolocationPrefPath));
EXPECT_EQ(CONTENT_SETTING_BLOCK, prefs->GetInteger(kGeolocationPrefPath));
}
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/permissions/permission_manager_factory.cc
Expand Up @@ -40,8 +40,6 @@

#if defined(OS_ANDROID)
#include "chrome/browser/geolocation/geolocation_permission_context_delegate_android.h"
#else
#include "chrome/browser/web_applications/file_handling_permission_context.h"
#endif // defined(OS_ANDROID)

#if defined(OS_MAC)
Expand Down Expand Up @@ -98,13 +96,6 @@ permissions::PermissionManager::PermissionContextMap CreatePermissionContexts(
permission_contexts[ContentSettingsType::DURABLE_STORAGE] =
std::make_unique<DurableStoragePermissionContext>(profile);

#if !defined(OS_ANDROID)
// TODO(crbug.com/1101999): File Handling is not available on Android and is
// only relevant for installed PWAs.
permission_contexts[ContentSettingsType::FILE_HANDLING] =
std::make_unique<FileHandlingPermissionContext>(profile);
#endif // !defined(OS_ANDROID)

// TODO(crbug.com/1043295): Still in development for Android so we don't
// support it on WebLayer yet.
permission_contexts[ContentSettingsType::FONT_ACCESS] =
Expand Down
24 changes: 0 additions & 24 deletions chrome/browser/resources/settings/privacy_page/privacy_page.html
Expand Up @@ -968,28 +968,4 @@ <h2>$i18n{siteSettingsDefaultBehavior}</h2>
</category-setting-exceptions>
</settings-subpage>
</template>
<template is="dom-if" route-path="/content/fileHandlers" no-search>
<settings-subpage page-title="$i18n{siteSettingsFileHandling}"
search-label="$i18n{siteSettingsAllSitesSearch}"
search-term="{{searchFilter_}}">
<div class="content-settings-header secondary">
$i18n{siteSettingsFileHandlingDescription}
</div>
<settings-category-default-radio-group
category="[[contentSettingsTypesEnum_.FILE_HANDLING]]"
allow-option-label="$i18n{siteSettingsFileHandlingAllowed}"
allow-option-icon="settings:file-handling"
block-option-label="$i18n{siteSettingsFileHandlingBlocked}"
block-option-icon="settings:file-handling-off">
</settings-category-default-radio-group>
<category-setting-exceptions
category="[[contentSettingsTypesEnum_.FILE_HANDLING]]"
block-header=
"$i18n{siteSettingsFileHandlingBlockedExceptions}"
allow-header=
"$i18n{siteSettingsFileHandlingAllowedExceptions}"
search-filter="[[searchFilter_]]">
</category-setting-exceptions>
</settings-subpage>
</template>
</settings-animated-pages>
1 change: 0 additions & 1 deletion chrome/browser/resources/settings/route.ts
Expand Up @@ -87,7 +87,6 @@ function addPrivacyChildRoutes(r: SettingsRoutes) {
r.SITE_SETTINGS.createChild('windowPlacement');
r.SITE_SETTINGS_FILE_SYSTEM_WRITE = r.SITE_SETTINGS.createChild('filesystem');
r.SITE_SETTINGS_FONT_ACCESS = r.SITE_SETTINGS.createChild('fontAccess');
r.SITE_SETTINGS_FILE_HANDLING = r.SITE_SETTINGS.createChild('fileHandlers');
}

/**
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/resources/settings/settings_routes.ts
Expand Up @@ -58,7 +58,6 @@ export type SettingsRoutes = {
SITE_SETTINGS_COOKIES: Route,
SITE_SETTINGS_DATA_DETAILS: Route,
SITE_SETTINGS_FONT_ACCESS: Route,
SITE_SETTINGS_FILE_HANDLING: Route,
SITE_SETTINGS_HANDLERS: Route,
SITE_SETTINGS_HID_DEVICES: Route,
SITE_SETTINGS_IDLE_DETECTION: Route,
Expand Down

0 comments on commit 66ef2a8

Please sign in to comment.