Commit 3bcbe725 authored by Mugdha Lakhani's avatar Mugdha Lakhani Committed by Commit Bot

[Background Fetch] Part 1: Take Background Sync

content setting out of the decision on whether we can start Background
Fetch.

For details, see this doc:
https://docs.google.com/document/d/1rPYSlbzScw_6PLUJ3m96ZLIkxXVAWyBSL75-VDJEMso/edit?usp=sharing

Bug: 886896
Change-Id: Ie6ba2ad6b906ab136eb65b846e67d34bad41e7e6
Reviewed-on: https://chromium-review.googlesource.com/1245799
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: default avatarRayan Kanso <rayankans@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594403}
parent 63ac6f12
...@@ -658,7 +658,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, FetchFromServiceWorker) { ...@@ -658,7 +658,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, FetchFromServiceWorker) {
// Give the needed permissions. // Give the needed permissions.
SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
SetPermission(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, CONTENT_SETTING_ALLOW);
// The fetch should succeed. // The fetch should succeed.
ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage( ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage(
...@@ -671,15 +670,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, FetchFromServiceWorker) { ...@@ -671,15 +670,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, FetchFromServiceWorker) {
// This should fail without the Automatic Downloads permission. // This should fail without the Automatic Downloads permission.
ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage( ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage(
"StartFetchFromServiceWorker()", "permissionerror")); "StartFetchFromServiceWorker()", "permissionerror"));
// Reset Automatic Downloads permission and remove Background Sync permission
SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
CONTENT_SETTING_ALLOW);
SetPermission(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, CONTENT_SETTING_BLOCK);
// This should also fail now.
ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage(
"StartFetchFromServiceWorker()", "permissionerror"));
} }
IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
...@@ -687,7 +677,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, ...@@ -687,7 +677,6 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
// Give the needed permissions. // Give the needed permissions.
SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
SetPermission(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, CONTENT_SETTING_ALLOW);
ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage( ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage(
"StartFetchFromIframe()", "backgroundfetchsuccess")); "StartFetchFromIframe()", "backgroundfetchsuccess"));
} }
...@@ -695,9 +684,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, ...@@ -695,9 +684,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
FetchFromChildFrameWithMissingPermissions) { FetchFromChildFrameWithMissingPermissions) {
SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, SetPermission(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_BLOCK);
// Revoke Background Sync permission.
SetPermission(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, CONTENT_SETTING_BLOCK);
ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage( ASSERT_NO_FATAL_FAILURE(RunScriptAndCheckResultingMessage(
"StartFetchFromIframe()", "permissionerror")); "StartFetchFromIframe()", "permissionerror"));
} }
...@@ -177,8 +177,12 @@ void BackgroundFetchDelegateImpl::GetPermissionForOrigin( ...@@ -177,8 +177,12 @@ void BackgroundFetchDelegateImpl::GetPermissionForOrigin(
// The fetch should be thought of as one download. So the origin will be // The fetch should be thought of as one download. So the origin will be
// used as the URL, and the |request_method| is set to GET. // used as the URL, and the |request_method| is set to GET.
limiter->CanDownload(wc_getter, origin.GetURL(), "GET", limiter->CanDownload(
base::AdaptCallbackForRepeating(std::move(callback))); wc_getter, origin.GetURL(), "GET",
base::AdaptCallbackForRepeating(base::BindOnce(
&BackgroundFetchDelegateImpl::
DidGetPermissionFromDownloadRequestLimiter,
weak_ptr_factory_.GetWeakPtr(), std::move(callback))));
return; return;
} }
...@@ -186,26 +190,39 @@ void BackgroundFetchDelegateImpl::GetPermissionForOrigin( ...@@ -186,26 +190,39 @@ void BackgroundFetchDelegateImpl::GetPermissionForOrigin(
HostContentSettingsMapFactory::GetForProfile(profile_); HostContentSettingsMapFactory::GetForProfile(profile_);
DCHECK(host_content_settings_map); DCHECK(host_content_settings_map);
// This is running from a worker context, use the Automatic Downloads // This is running from a non-top level frame, use the Automatic Downloads
// permission. // content setting.
ContentSetting content_setting = host_content_settings_map->GetContentSetting( ContentSetting content_setting = host_content_settings_map->GetContentSetting(
origin.GetURL(), origin.GetURL(), origin.GetURL(), origin.GetURL(),
CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
std::string() /* resource_identifier */); std::string() /* resource_identifier */);
if (content_setting != CONTENT_SETTING_ALLOW) { // The set of valid settings for automatic downloads is set to
std::move(callback).Run(/* has_permission= */ false); // {CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK}.
return; switch (content_setting) {
case CONTENT_SETTING_ALLOW:
std::move(callback).Run(content::BackgroundFetchPermission::ALLOWED);
return;
case CONTENT_SETTING_ASK:
std::move(callback).Run(content::BackgroundFetchPermission::ASK);
return;
case CONTENT_SETTING_BLOCK:
std::move(callback).Run(content::BackgroundFetchPermission::BLOCKED);
return;
case CONTENT_SETTING_DEFAULT:
case CONTENT_SETTING_SESSION_ONLY:
case CONTENT_SETTING_DETECT_IMPORTANT_CONTENT:
case CONTENT_SETTING_NUM_SETTINGS:
NOTREACHED();
} }
}
// Also make sure that Background Sync has permission. void BackgroundFetchDelegateImpl::DidGetPermissionFromDownloadRequestLimiter(
// TODO(crbug.com/616321): Remove this check after Automatic Downloads GetPermissionForOriginCallback callback,
// permissions can be modified from Android. bool has_permission) {
content_setting = host_content_settings_map->GetContentSetting( std::move(callback).Run(has_permission
origin.GetURL(), origin.GetURL(), CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, ? content::BackgroundFetchPermission::ALLOWED
std::string() /* resource_identifier */); : content::BackgroundFetchPermission::BLOCKED);
std::move(callback).Run(content_setting == CONTENT_SETTING_ALLOW);
} }
void BackgroundFetchDelegateImpl::CreateDownloadJob( void BackgroundFetchDelegateImpl::CreateDownloadJob(
......
...@@ -155,6 +155,11 @@ class BackgroundFetchDelegateImpl ...@@ -155,6 +155,11 @@ class BackgroundFetchDelegateImpl
void OnDownloadReceived(const std::string& guid, void OnDownloadReceived(const std::string& guid,
download::DownloadParams::StartResult result); download::DownloadParams::StartResult result);
// The callback passed to DownloadRequestLimiter::CanDownload().
void DidGetPermissionFromDownloadRequestLimiter(
GetPermissionForOriginCallback callback,
bool has_permission);
// The profile this service is being created for. // The profile this service is being created for.
Profile* profile_; Profile* profile_;
......
...@@ -183,16 +183,17 @@ void BackgroundFetchContext::DidGetPermission( ...@@ -183,16 +183,17 @@ void BackgroundFetchContext::DidGetPermission(
const SkBitmap& icon, const SkBitmap& icon,
blink::mojom::BackgroundFetchUkmDataPtr ukm_data, blink::mojom::BackgroundFetchUkmDataPtr ukm_data,
int frame_tree_node_id, int frame_tree_node_id,
bool has_permission) { BackgroundFetchPermission permission) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
base::PostTaskWithTraits( base::PostTaskWithTraits(
FROM_HERE, {BrowserThread::UI}, FROM_HERE, {BrowserThread::UI},
base::BindOnce(&background_fetch::RecordBackgroundFetchUkmEvent, base::BindOnce(&background_fetch::RecordBackgroundFetchUkmEvent,
registration_id.origin(), requests, options, icon, registration_id.origin(), requests, options, icon,
std::move(ukm_data), frame_tree_node_id, has_permission)); std::move(ukm_data), frame_tree_node_id, permission));
if (has_permission) { if (permission != BackgroundFetchPermission::BLOCKED) {
// TODO(crbug.com/886896): Passed paused flag to CreateRegistration.
data_manager_->BackgroundFetchDataManager::CreateRegistration( data_manager_->BackgroundFetchDataManager::CreateRegistration(
registration_id, requests, options, icon, registration_id, requests, options, icon,
base::BindOnce(&BackgroundFetchContext::DidCreateRegistration, base::BindOnce(&BackgroundFetchContext::DidCreateRegistration,
......
...@@ -151,7 +151,8 @@ class CONTENT_EXPORT BackgroundFetchContext ...@@ -151,7 +151,8 @@ class CONTENT_EXPORT BackgroundFetchContext
void OnStorageWiped() override; void OnStorageWiped() override;
private: private:
using GetPermissionCallback = base::OnceCallback<void(bool)>; using GetPermissionCallback =
base::OnceCallback<void(BackgroundFetchPermission)>;
FRIEND_TEST_ALL_PREFIXES(BackgroundFetchServiceTest, FRIEND_TEST_ALL_PREFIXES(BackgroundFetchServiceTest,
JobsInitializedOnBrowserRestart); JobsInitializedOnBrowserRestart);
...@@ -278,7 +279,7 @@ class CONTENT_EXPORT BackgroundFetchContext ...@@ -278,7 +279,7 @@ class CONTENT_EXPORT BackgroundFetchContext
const SkBitmap& icon, const SkBitmap& icon,
blink::mojom::BackgroundFetchUkmDataPtr ukm_data, blink::mojom::BackgroundFetchUkmDataPtr ukm_data,
int frame_tree_node_id, int frame_tree_node_id,
bool has_permission); BackgroundFetchPermission permission);
// |this| is owned, indirectly, by the BrowserContext. // |this| is owned, indirectly, by the BrowserContext.
BrowserContext* browser_context_; BrowserContext* browser_context_;
......
...@@ -45,11 +45,10 @@ class BackgroundFetchDelegateProxy::Core ...@@ -45,11 +45,10 @@ class BackgroundFetchDelegateProxy::Core
void ForwardGetPermissionForOriginCallbackToIO( void ForwardGetPermissionForOriginCallbackToIO(
BackgroundFetchDelegate::GetPermissionForOriginCallback callback, BackgroundFetchDelegate::GetPermissionForOriginCallback callback,
bool has_permission) { BackgroundFetchPermission permission) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
base::PostTaskWithTraits( base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO},
FROM_HERE, {BrowserThread::IO}, base::BindOnce(std::move(callback), permission));
base::BindOnce(std::move(callback), has_permission));
} }
void GetPermissionForOrigin( void GetPermissionForOrigin(
...@@ -62,7 +61,7 @@ class BackgroundFetchDelegateProxy::Core ...@@ -62,7 +61,7 @@ class BackgroundFetchDelegateProxy::Core
base::BindOnce(&Core::ForwardGetPermissionForOriginCallbackToIO, base::BindOnce(&Core::ForwardGetPermissionForOriginCallbackToIO,
weak_ptr_factory_.GetWeakPtr(), std::move(callback))); weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
} else { } else {
std::move(callback).Run(false /* has_permission */); std::move(callback).Run(BackgroundFetchPermission::BLOCKED);
} }
} }
......
...@@ -40,7 +40,7 @@ class FakeBackgroundFetchDelegate : public BackgroundFetchDelegate { ...@@ -40,7 +40,7 @@ class FakeBackgroundFetchDelegate : public BackgroundFetchDelegate {
const url::Origin& origin, const url::Origin& origin,
const ResourceRequestInfo::WebContentsGetter& wc_getter, const ResourceRequestInfo::WebContentsGetter& wc_getter,
GetPermissionForOriginCallback callback) override { GetPermissionForOriginCallback callback) override {
std::move(callback).Run(true /* has_permission */); std::move(callback).Run(BackgroundFetchPermission::ALLOWED);
} }
void CreateDownloadJob( void CreateDownloadJob(
std::unique_ptr<BackgroundFetchDescription> fetch_description) override {} std::unique_ptr<BackgroundFetchDescription> fetch_description) override {}
......
...@@ -43,7 +43,7 @@ void RecordBackgroundFetchUkmEvent( ...@@ -43,7 +43,7 @@ void RecordBackgroundFetchUkmEvent(
const SkBitmap& icon, const SkBitmap& icon,
blink::mojom::BackgroundFetchUkmDataPtr ukm_data, blink::mojom::BackgroundFetchUkmDataPtr ukm_data,
int frame_tree_node_id, int frame_tree_node_id,
bool has_permission) { BackgroundFetchPermission permission) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
// Only record UKM data if there's a frame associated. // Only record UKM data if there's a frame associated.
...@@ -67,7 +67,8 @@ void RecordBackgroundFetchUkmEvent( ...@@ -67,7 +67,8 @@ void RecordBackgroundFetchUkmEvent(
options.download_total, kUkmEventDataBucketSpacing)) options.download_total, kUkmEventDataBucketSpacing))
.SetNumRequestsInFetch(ukm::GetExponentialBucketMin( .SetNumRequestsInFetch(ukm::GetExponentialBucketMin(
requests.size(), kUkmEventDataBucketSpacing)) requests.size(), kUkmEventDataBucketSpacing))
.SetDeniedDueToPermissions(!has_permission) .SetDeniedDueToPermissions(permission ==
BackgroundFetchPermission::BLOCKED)
.Record(ukm::UkmRecorder::Get()); .Record(ukm::UkmRecorder::Get());
} }
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#ifndef CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_METRICS_H_ #ifndef CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_METRICS_H_
#define CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_METRICS_H_ #define CONTENT_BROWSER_BACKGROUND_FETCH_BACKGROUND_FETCH_METRICS_H_
#include "content/public/browser/background_fetch_delegate.h"
#include "third_party/blink/public/platform/modules/background_fetch/background_fetch.mojom.h" #include "third_party/blink/public/platform/modules/background_fetch/background_fetch.mojom.h"
namespace content { namespace content {
...@@ -38,7 +39,7 @@ void RecordBackgroundFetchUkmEvent( ...@@ -38,7 +39,7 @@ void RecordBackgroundFetchUkmEvent(
const SkBitmap& icon, const SkBitmap& icon,
blink::mojom::BackgroundFetchUkmDataPtr ukm_data, blink::mojom::BackgroundFetchUkmDataPtr ukm_data,
int frame_tree_node_id, int frame_tree_node_id,
bool has_permission); BackgroundFetchPermission permission);
} // namespace background_fetch } // namespace background_fetch
......
...@@ -63,7 +63,7 @@ void MockBackgroundFetchDelegate::GetPermissionForOrigin( ...@@ -63,7 +63,7 @@ void MockBackgroundFetchDelegate::GetPermissionForOrigin(
GetPermissionForOriginCallback callback) { GetPermissionForOriginCallback callback) {
base::SequencedTaskRunnerHandle::Get()->PostTask( base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(std::move(callback), true /* has_permission */)); base::BindOnce(std::move(callback), BackgroundFetchPermission::ALLOWED));
} }
void MockBackgroundFetchDelegate::GetIconDisplaySize( void MockBackgroundFetchDelegate::GetIconDisplaySize(
......
...@@ -39,6 +39,18 @@ struct BackgroundFetchResponse; ...@@ -39,6 +39,18 @@ struct BackgroundFetchResponse;
struct BackgroundFetchResult; struct BackgroundFetchResult;
struct BackgroundFetchDescription; struct BackgroundFetchDescription;
enum class BackgroundFetchPermission {
// Background Fetch is allowed.
ALLOWED,
// Background Fetch should be started in a paused state, to let the user
// decide whether to continue.
ASK,
// Background Fetch is blocked.
BLOCKED,
};
// Interface for launching background fetches. Implementing classes would // Interface for launching background fetches. Implementing classes would
// generally interface with the DownloadService or DownloadManager. // generally interface with the DownloadService or DownloadManager.
// Must only be used on the UI thread and generally expected to be called by the // Must only be used on the UI thread and generally expected to be called by the
...@@ -46,7 +58,8 @@ struct BackgroundFetchDescription; ...@@ -46,7 +58,8 @@ struct BackgroundFetchDescription;
class CONTENT_EXPORT BackgroundFetchDelegate { class CONTENT_EXPORT BackgroundFetchDelegate {
public: public:
using GetIconDisplaySizeCallback = base::OnceCallback<void(const gfx::Size&)>; using GetIconDisplaySizeCallback = base::OnceCallback<void(const gfx::Size&)>;
using GetPermissionForOriginCallback = base::OnceCallback<void(bool)>; using GetPermissionForOriginCallback =
base::OnceCallback<void(BackgroundFetchPermission)>;
// Client interface that a BackgroundFetchDelegate would use to signal the // Client interface that a BackgroundFetchDelegate would use to signal the
// progress of a background fetch. // progress of a background fetch.
......
...@@ -186,7 +186,7 @@ void LayoutTestBackgroundFetchDelegate::GetPermissionForOrigin( ...@@ -186,7 +186,7 @@ void LayoutTestBackgroundFetchDelegate::GetPermissionForOrigin(
const ResourceRequestInfo::WebContentsGetter& wc_getter, const ResourceRequestInfo::WebContentsGetter& wc_getter,
GetPermissionForOriginCallback callback) { GetPermissionForOriginCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI); DCHECK_CURRENTLY_ON(BrowserThread::UI);
std::move(callback).Run(true /* has_permission */); std::move(callback).Run(BackgroundFetchPermission::ALLOWED);
} }
void LayoutTestBackgroundFetchDelegate::CreateDownloadJob( void LayoutTestBackgroundFetchDelegate::CreateDownloadJob(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment