Skip to content

Commit

Permalink
File Handling: Add extension types to permission prompt.
Browse files Browse the repository at this point in the history
Add file extensions sourced from the registrar, to the permission prompt.
The permission prompt should now inform users which file extensions the
web app would like to access file handlers for. Different translation
strings exist for each possible amount of handlers, to allow for better
translation between languages with different sentence structure.

File Handler information is retrieved from the WebAppRegistrar (instead
of from the blink manifest), to match what's registered in the
operating system.

Previous UI: https://storage.cloud.google.com/chromium-translation-screenshots/55ddb345d69ee2187e402f22579fc4459b9563d1
New UI: https://storage.cloud.google.com/chromium-translation-screenshots/aa286efbaee111ce1a8e37854ff384fa397f929e

Bug: 1198339
Change-Id: Iad350f2f5c753af3a1315f3813d5fda505b93549
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2824363
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Andy Paicu <andypaicu@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Chase Phillips <cmp@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#877746}
  • Loading branch information
Darwin Huang authored and Chromium LUCI CQ committed Apr 30, 2021
1 parent b63f81c commit 2aedfd5
Show file tree
Hide file tree
Showing 19 changed files with 244 additions and 45 deletions.
10 changes: 10 additions & 0 deletions chrome/app/generated_resources.grd
Expand Up @@ -10461,6 +10461,16 @@ Please help our engineers fix this problem. Tell us what happened right before y
</message>
</if>

<!-- File Handling permission prompt -->
<if expr="not is_android">
<message name="IDS_WEB_APP_FILE_HANDLING_PERMISSION_TEXT" desc="Permission request shown if the user is opening a file using a PWA with registered file handlers. Follows a prompt: 'This site would like to:'">
Open <ph name="FILE_EXTENSIONS">$1<ex>.txt, .csv, .md</ex></ph> files
</message>
<message name="IDS_WEB_APP_FILE_HANDLING_EXTENSION_LIST_SEPARATOR" desc="Text/punctuation that separates a list of file types/extensions, such as .txt or .csv." meaning="List separator for file extensions.">
, '''
</message>
</if>

<!-- Font Access chooser -->
<if expr="not is_android">
<message name="IDS_FONT_ACCESS_CHOOSER_PROMPT_ORIGIN" desc="The label that is used to introduce Local Font Access chooser details to the user in a popup when it is from a website.">
Expand Down
@@ -0,0 +1 @@
aa286efbaee111ce1a8e37854ff384fa397f929e
@@ -0,0 +1 @@
aa286efbaee111ce1a8e37854ff384fa397f929e
4 changes: 3 additions & 1 deletion chrome/browser/permissions/permission_manager_factory.cc
Expand Up @@ -32,7 +32,6 @@
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/permissions/contexts/clipboard_read_write_permission_context.h"
#include "components/permissions/contexts/clipboard_sanitized_write_permission_context.h"
#include "components/permissions/contexts/file_handling_permission_context.h"
#include "components/permissions/contexts/font_access_permission_context.h"
#include "components/permissions/contexts/midi_permission_context.h"
#include "components/permissions/contexts/midi_sysex_permission_context.h"
Expand All @@ -51,6 +50,7 @@
#include "components/permissions/contexts/nfc_permission_context_android.h"
#else
#include "chrome/browser/geolocation/geolocation_permission_context_delegate.h"
#include "chrome/browser/web_applications/components/file_handling_permission_context.h"
#include "components/permissions/contexts/geolocation_permission_context.h"
#include "components/permissions/contexts/nfc_permission_context.h"
#endif
Expand Down Expand Up @@ -141,8 +141,10 @@ permissions::PermissionManager::PermissionContextMap CreatePermissionContexts(
std::make_unique<FontAccessPermissionContext>(profile);
permission_contexts[ContentSettingsType::DISPLAY_CAPTURE] =
std::make_unique<DisplayCapturePermissionContext>(profile);
#if !defined(OS_ANDROID) // File Handling is not available on Android.
permission_contexts[ContentSettingsType::FILE_HANDLING] =
std::make_unique<FileHandlingPermissionContext>(profile);
#endif // !defined(OS_ANDROID)
return permission_contexts;
}
} // namespace
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/web_applications/components/BUILD.gn
Expand Up @@ -24,6 +24,10 @@ source_set("components") {
"externally_managed_app_manager.h",
"file_handler_manager.cc",
"file_handler_manager.h",
"file_handling_permission_context.cc",
"file_handling_permission_context.h",
"file_handling_permission_request_impl.cc",
"file_handling_permission_request_impl.h",
"install_bounce_metric.cc",
"install_bounce_metric.h",
"install_finalizer.cc",
Expand Down Expand Up @@ -181,6 +185,7 @@ source_set("components") {
"//chrome/common",
"//components/crx_file",
"//components/keyed_service/content",
"//components/permissions:permissions",
"//components/pref_registry",
"//components/services/app_service/public/cpp:protocol_handling",
"//components/services/app_service/public/mojom",
Expand Down
@@ -0,0 +1,40 @@
// Copyright 2021 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/file_handling_permission_context.h"

#include <string>

#include "chrome/browser/web_applications/components/file_handling_permission_request_impl.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "components/permissions/permission_context_base.h"
#include "content/public/browser/web_contents.h"
#include "third_party/blink/public/mojom/permissions_policy/permissions_policy.mojom.h"

FileHandlingPermissionContext::FileHandlingPermissionContext(
content::BrowserContext* browser_context)
: PermissionContextBase(browser_context,
ContentSettingsType::FILE_HANDLING,
blink::mojom::PermissionsPolicyFeature::kNotFound) {
}

FileHandlingPermissionContext::~FileHandlingPermissionContext() = default;

bool FileHandlingPermissionContext::IsRestrictedToSecureOrigins() const {
return true;
}

std::unique_ptr<permissions::PermissionRequest>
FileHandlingPermissionContext::CreatePermissionRequest(
const GURL& request_origin,
ContentSettingsType content_settings_type,
bool has_gesture,
content::WebContents* web_contents,
permissions::PermissionRequestImpl::PermissionDecidedCallback
permission_decided_callback,
base::OnceClosure delete_callback) const {
return std::make_unique<permissions::FileHandlingPermissionRequestImpl>(
request_origin, content_settings_type, has_gesture, web_contents,
std::move(permission_decided_callback), std::move(delete_callback));
}
Expand Up @@ -2,11 +2,15 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef COMPONENTS_PERMISSIONS_CONTEXTS_FILE_HANDLING_PERMISSION_CONTEXT_H_
#define COMPONENTS_PERMISSIONS_CONTEXTS_FILE_HANDLING_PERMISSION_CONTEXT_H_
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_CONTEXT_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_CONTEXT_H_

#include "components/permissions/permission_context_base.h"

namespace content {
class WebContents;
} // namespace content

class FileHandlingPermissionContext
: public permissions::PermissionContextBase {
public:
Expand All @@ -21,6 +25,14 @@ class FileHandlingPermissionContext
protected:
// PermissionContextBase:
bool IsRestrictedToSecureOrigins() const override;
std::unique_ptr<permissions::PermissionRequest> CreatePermissionRequest(
const GURL& request_origin,
ContentSettingsType content_settings_type,
bool has_gesture,
content::WebContents* web_contents,
permissions::PermissionRequestImpl::PermissionDecidedCallback
permission_decided_callback,
base::OnceClosure delete_callback) const override;
};

#endif // COMPONENTS_PERMISSIONS_CONTEXTS_FILE_HANDLING_PERMISSION_CONTEXT_H_
#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_CONTEXT_H_
@@ -0,0 +1,51 @@
// Copyright 2021 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/file_handling_permission_request_impl.h"

#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/components/web_app_utils.h"
#include "chrome/grit/generated_resources.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
#include "ui/base/l10n/l10n_util.h"

namespace permissions {

FileHandlingPermissionRequestImpl::FileHandlingPermissionRequestImpl(
const GURL& request_origin,
ContentSettingsType content_settings_type,
bool has_gesture,
content::WebContents* web_contents,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback)
: PermissionRequestImpl(request_origin,
content_settings_type,
has_gesture,
std::move(permission_decided_callback),
std::move(delete_callback)) {
auto* browser = web_contents->GetBrowserContext();
if (!browser) {
return;
}
Profile* profile = Profile::FromBrowserContext(browser);
DCHECK(profile);

std::u16string extensions_list =
web_app::GetFileExtensionsHandledByWebAppDisplayedAsList(
profile, web_contents->GetLastCommittedURL());
message_text_fragment_ = l10n_util::GetStringFUTF16(
IDS_WEB_APP_FILE_HANDLING_PERMISSION_TEXT, extensions_list);
}

FileHandlingPermissionRequestImpl::~FileHandlingPermissionRequestImpl() =
default;

std::u16string FileHandlingPermissionRequestImpl::GetMessageTextFragment()
const {
return message_text_fragment_;
}

} // namespace permissions
@@ -0,0 +1,36 @@
// Copyright 2021 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_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_REQUEST_IMPL_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_REQUEST_IMPL_H_

#include <string>
#include "components/permissions/permission_request_impl.h"

namespace content {
class WebContents;
}

namespace permissions {

// Provides a custom message text fragment that differs based on file handlers
// requested by a web app.
class FileHandlingPermissionRequestImpl : public PermissionRequestImpl {
public:
FileHandlingPermissionRequestImpl(
const GURL& request_origin,
ContentSettingsType content_settings_type,
bool has_gesture,
content::WebContents* web_contents,
PermissionDecidedCallback permission_decided_callback,
base::OnceClosure delete_callback);
~FileHandlingPermissionRequestImpl() override;

private:
std::u16string GetMessageTextFragment() const override;
std::u16string message_text_fragment_;
};
} // namespace permissions

#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_FILE_HANDLING_PERMISSION_REQUEST_IMPL_H_
29 changes: 29 additions & 0 deletions chrome/browser/web_applications/components/web_app_utils.cc
Expand Up @@ -5,10 +5,18 @@
#include "chrome/browser/web_applications/components/web_app_utils.h"

#include "base/files/file_path.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/web_applications/components/app_registrar.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/grit/generated_resources.h"
#include "components/services/app_service/public/cpp/file_handler.h"
#include "components/site_engagement/content/site_engagement_service.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "base/feature_list.h"
Expand Down Expand Up @@ -131,4 +139,25 @@ bool IsChromeOs() {
#endif
}

std::vector<std::string> GetFileExtensionsHandledByWebApp(Profile* profile,
const GURL& url) {
auto* provider = WebAppProviderBase::GetProviderBase(profile);
const AppRegistrar& registrar = provider->registrar();
const apps::FileHandlers* handlers =
registrar.GetAppFileHandlers(*registrar.FindAppWithUrlInScope(url));
DCHECK(handlers);
std::set<std::string> extensions =
apps::GetFileExtensionsFromFileHandlers(*handlers);
return std::vector<std::string>(extensions.begin(), extensions.end());
}

std::u16string GetFileExtensionsHandledByWebAppDisplayedAsList(
Profile* profile,
const GURL& url) {
return base::UTF8ToUTF16(base::JoinString(
GetFileExtensionsHandledByWebApp(profile, url),
l10n_util::GetStringUTF8(
IDS_WEB_APP_FILE_HANDLING_EXTENSION_LIST_SEPARATOR)));
}

} // namespace web_app
14 changes: 14 additions & 0 deletions chrome/browser/web_applications/components/web_app_utils.h
Expand Up @@ -6,9 +6,11 @@
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_UTILS_H_

#include <string>
#include <vector>

#include "chrome/browser/web_applications/components/web_app_id.h"

class GURL;
class Profile;

namespace base {
Expand Down Expand Up @@ -69,6 +71,18 @@ std::string GetProfileCategoryForLogging(Profile* profile);
// Returns true if the WebApp should have `web_app::WebAppChromeOsData()`.
bool IsChromeOs();

// Returns a vector of file extensions of the form ".csv" which are handled by
// the app for `url`. It is an error to call this with a URL that doesn't
// correspond to an app in `profile`.
std::vector<std::string> GetFileExtensionsHandledByWebApp(Profile* profile,
const GURL& url);

// Returns a display-ready string that holds all the file extensions handled by
// the app for `url`. It is an error to call this with a URL that doesn't
// correspond to an app in `profile`.
std::u16string GetFileExtensionsHandledByWebAppDisplayedAsList(Profile* profile,
const GURL& url);

} // namespace web_app

#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_UTILS_H_
2 changes: 0 additions & 2 deletions components/permissions/BUILD.gn
Expand Up @@ -21,8 +21,6 @@ source_set("permissions") {
"contexts/clipboard_read_write_permission_context.h",
"contexts/clipboard_sanitized_write_permission_context.cc",
"contexts/clipboard_sanitized_write_permission_context.h",
"contexts/file_handling_permission_context.cc",
"contexts/file_handling_permission_context.h",
"contexts/font_access_permission_context.cc",
"contexts/font_access_permission_context.h",
"contexts/geolocation_permission_context.cc",
Expand Down

This file was deleted.

29 changes: 21 additions & 8 deletions components/permissions/permission_context_base.cc
Expand Up @@ -219,6 +219,20 @@ void PermissionContextBase::UserMadePermissionDecision(
const GURL& embedding_origin,
ContentSetting content_setting) {}

std::unique_ptr<PermissionRequest>
PermissionContextBase::CreatePermissionRequest(
const GURL& request_origin,
ContentSettingsType content_settings_type,
bool has_gesture,
content::WebContents* web_contents,
PermissionRequestImpl::PermissionDecidedCallback
permission_decided_callback,
base::OnceClosure delete_callback) const {
return std::make_unique<PermissionRequestImpl>(
request_origin, content_settings_type, has_gesture,
std::move(permission_decided_callback), std::move(delete_callback));
}

PermissionResult PermissionContextBase::GetPermissionStatus(
content::RenderFrameHost* render_frame_host,
const GURL& requesting_origin,
Expand Down Expand Up @@ -373,14 +387,13 @@ void PermissionContextBase::DecidePermission(
if (!permission_request_manager)
return;

std::unique_ptr<PermissionRequest> request_ptr =
std::make_unique<PermissionRequestImpl>(
requesting_origin, content_settings_type_, user_gesture,
base::BindOnce(&PermissionContextBase::PermissionDecided,
weak_factory_.GetWeakPtr(), id, requesting_origin,
embedding_origin, std::move(callback)),
base::BindOnce(&PermissionContextBase::CleanUpRequest,
weak_factory_.GetWeakPtr(), id));
std::unique_ptr<PermissionRequest> request_ptr = CreatePermissionRequest(
requesting_origin, content_settings_type_, user_gesture, web_contents,
base::BindOnce(&PermissionContextBase::PermissionDecided,
weak_factory_.GetWeakPtr(), id, requesting_origin,
embedding_origin, std::move(callback)),
base::BindOnce(&PermissionContextBase::CleanUpRequest,
weak_factory_.GetWeakPtr(), id));
PermissionRequest* request = request_ptr.get();

bool inserted =
Expand Down

0 comments on commit 2aedfd5

Please sign in to comment.