From b9116c101b8f58fa192d9b5af26dc4d5762a3e50 Mon Sep 17 00:00:00 2001 From: nsatragno Date: Tue, 1 Mar 2016 17:28:41 -0800 Subject: [PATCH] Add a Background Sync Permission Context This is the second in a series of patches to add a content setting to Background Sync. For a detailed plan, refer to https://goo.gl/9U8fKh This change adds the permission context for background sync. No behavoiur change happens yet, as the permission is granted by default and no UI is exposed to the user. Additionally, it has not been hooked to the Background Sync Manager. Original, 'big picture' patch: crrev.com/1673303002 BUG=564052 TEST=BackgroundSyncPermissionContextTest.* Review URL: https://codereview.chromium.org/1698323002 Cr-Commit-Position: refs/heads/master@{#378646} --- .../browser/aw_permission_manager.cc | 2 + .../background_sync_permission_context.cc | 37 +++++++ .../background_sync_permission_context.h | 41 +++++++ ...kground_sync_permission_context_factory.cc | 32 ++++++ ...ckground_sync_permission_context_factory.h | 35 ++++++ ...ground_sync_permission_context_unittest.cc | 103 ++++++++++++++++++ .../browser/permissions/permission_context.cc | 5 + .../browser/permissions/permission_manager.cc | 2 + .../permissions/permission_uma_util.cc | 5 +- chrome/browser/permissions/permission_util.cc | 4 + chrome/chrome_browser.gypi | 4 + chrome/chrome_tests_unit.gypi | 1 + .../core/browser/content_settings_registry.cc | 7 ++ .../core/common/content_settings.cc | 1 + .../core/common/content_settings_types.h | 1 + content/public/browser/permission_type.h | 1 + tools/metrics/histograms/histograms.xml | 5 + 17 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 chrome/browser/background_sync/background_sync_permission_context.cc create mode 100644 chrome/browser/background_sync/background_sync_permission_context.h create mode 100644 chrome/browser/background_sync/background_sync_permission_context_factory.cc create mode 100644 chrome/browser/background_sync/background_sync_permission_context_factory.h create mode 100644 chrome/browser/background_sync/background_sync_permission_context_unittest.cc diff --git a/android_webview/browser/aw_permission_manager.cc b/android_webview/browser/aw_permission_manager.cc index b8ddbbd5d40db9..2f3153a40335cf 100644 --- a/android_webview/browser/aw_permission_manager.cc +++ b/android_webview/browser/aw_permission_manager.cc @@ -242,6 +242,7 @@ int AwPermissionManager::RequestPermission( case PermissionType::NOTIFICATIONS: case PermissionType::PUSH_MESSAGING: case PermissionType::DURABLE_STORAGE: + case PermissionType::BACKGROUND_SYNC: NOTIMPLEMENTED() << "RequestPermission is not implemented for " << static_cast(permission); callback.Run(content::PermissionStatus::DENIED); @@ -360,6 +361,7 @@ void AwPermissionManager::CancelPermissionRequest(int request_id) { case PermissionType::DURABLE_STORAGE: case PermissionType::AUDIO_CAPTURE: case PermissionType::VIDEO_CAPTURE: + case PermissionType::BACKGROUND_SYNC: NOTIMPLEMENTED() << "CancelPermission not implemented for " << static_cast(pending_request->permission); break; diff --git a/chrome/browser/background_sync/background_sync_permission_context.cc b/chrome/browser/background_sync/background_sync_permission_context.cc new file mode 100644 index 00000000000000..2faca4508fddfd --- /dev/null +++ b/chrome/browser/background_sync/background_sync_permission_context.cc @@ -0,0 +1,37 @@ +// Copyright 2016 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/background_sync/background_sync_permission_context.h" + +#include "base/logging.h" +#include "components/content_settings/core/common/content_settings_types.h" +#include "content/public/browser/permission_type.h" + +BackgroundSyncPermissionContext::BackgroundSyncPermissionContext( + Profile* profile) + : PermissionContextBase(profile, + content::PermissionType::BACKGROUND_SYNC, + CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC) {} + +void BackgroundSyncPermissionContext::CancelPermissionRequest( + content::WebContents* web_contents, + const PermissionRequestID& id) { + // Background sync permission requests are resolved instantly without + // prompting the user, there is no way to cancel them. + NOTREACHED(); +} + +void BackgroundSyncPermissionContext::DecidePermission( + content::WebContents* web_contents, + const PermissionRequestID& id, + const GURL& requesting_origin, + const GURL& embedding_origin, + const BrowserPermissionCallback& callback) { + // The user should never be prompted to authorize background sync. + NOTREACHED(); +} + +bool BackgroundSyncPermissionContext::IsRestrictedToSecureOrigins() const { + return true; +} diff --git a/chrome/browser/background_sync/background_sync_permission_context.h b/chrome/browser/background_sync/background_sync_permission_context.h new file mode 100644 index 00000000000000..2b109ff50ec39b --- /dev/null +++ b/chrome/browser/background_sync/background_sync_permission_context.h @@ -0,0 +1,41 @@ +// Copyright 2016 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_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_H_ +#define CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_H_ + +#include "base/macros.h" +#include "chrome/browser/permissions/permission_context_base.h" + +class Profile; + +// Manages user permissions for background sync. The context is scoped to the +// requesting origin, which should always be equal to the top-level origin as +// background syncs can only be requested from top-level pages. +// The permission status is ALLOW by default and can be changed globally or on a +// per-site basis from the content settings page. The user is not prompted for +// permission. +// TODO(nsatragno): actually implement the UI to allow changing the setting. +class BackgroundSyncPermissionContext : public PermissionContextBase { + public: + explicit BackgroundSyncPermissionContext(Profile* profile); + ~BackgroundSyncPermissionContext() override = default; + + // PermissionContextBase: + void CancelPermissionRequest(content::WebContents* web_contents, + const PermissionRequestID& id) override; + + private: + // PermissionContextBase: + void DecidePermission(content::WebContents* web_contents, + const PermissionRequestID& id, + const GURL& requesting_origin, + const GURL& embedding_origin, + const BrowserPermissionCallback& callback) override; + bool IsRestrictedToSecureOrigins() const override; + + DISALLOW_COPY_AND_ASSIGN(BackgroundSyncPermissionContext); +}; + +#endif // CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_H_ diff --git a/chrome/browser/background_sync/background_sync_permission_context_factory.cc b/chrome/browser/background_sync/background_sync_permission_context_factory.cc new file mode 100644 index 00000000000000..36f00061cf378b --- /dev/null +++ b/chrome/browser/background_sync/background_sync_permission_context_factory.cc @@ -0,0 +1,32 @@ +// Copyright 2016 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/background_sync/background_sync_permission_context_factory.h" + +#include "chrome/browser/background_sync/background_sync_permission_context.h" +#include "chrome/browser/profiles/profile.h" +#include "components/keyed_service/content/browser_context_dependency_manager.h" + +// static +BackgroundSyncPermissionContext* +BackgroundSyncPermissionContextFactory::GetForProfile(Profile* profile) { + return static_cast( + GetInstance()->GetServiceForBrowserContext(profile, true /* create */)); +} + +// static +BackgroundSyncPermissionContextFactory* +BackgroundSyncPermissionContextFactory::GetInstance() { + return base::Singleton::get(); +} + +BackgroundSyncPermissionContextFactory::BackgroundSyncPermissionContextFactory() + : PermissionContextFactoryBase( + "BackgroundSyncPermissionContext", + BrowserContextDependencyManager::GetInstance()) {} + +KeyedService* BackgroundSyncPermissionContextFactory::BuildServiceInstanceFor( + content::BrowserContext* profile) const { + return new BackgroundSyncPermissionContext(static_cast(profile)); +} diff --git a/chrome/browser/background_sync/background_sync_permission_context_factory.h b/chrome/browser/background_sync/background_sync_permission_context_factory.h new file mode 100644 index 00000000000000..3dbccdf6b1ad5d --- /dev/null +++ b/chrome/browser/background_sync/background_sync_permission_context_factory.h @@ -0,0 +1,35 @@ +// Copyright 2016 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_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_FACTORY_H_ +#define CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_FACTORY_H_ + +#include "base/macros.h" +#include "base/memory/singleton.h" +#include "chrome/browser/permissions/permission_context_factory_base.h" + +class BackgroundSyncPermissionContext; +class Profile; + +class BackgroundSyncPermissionContextFactory + : public PermissionContextFactoryBase { + public: + static BackgroundSyncPermissionContext* GetForProfile(Profile* profile); + static BackgroundSyncPermissionContextFactory* GetInstance(); + + private: + friend struct base::DefaultSingletonTraits< + BackgroundSyncPermissionContextFactory>; + + BackgroundSyncPermissionContextFactory(); + ~BackgroundSyncPermissionContextFactory() override = default; + + // BrowserContextKeyedBaseFactory methods: + KeyedService* BuildServiceInstanceFor( + content::BrowserContext* profile) const override; + + DISALLOW_COPY_AND_ASSIGN(BackgroundSyncPermissionContextFactory); +}; + +#endif // CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_PERMISSION_CONTEXT_FACTORY_H_ diff --git a/chrome/browser/background_sync/background_sync_permission_context_unittest.cc b/chrome/browser/background_sync/background_sync_permission_context_unittest.cc new file mode 100644 index 00000000000000..62f9839d3f0251 --- /dev/null +++ b/chrome/browser/background_sync/background_sync_permission_context_unittest.cc @@ -0,0 +1,103 @@ +// Copyright 2016 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/background_sync/background_sync_permission_context.h" + +#include + +#include "base/bind.h" +#include "base/macros.h" +#include "base/run_loop.h" +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" +#include "chrome/browser/permissions/permission_request_id.h" +#include "chrome/test/base/chrome_render_view_host_test_harness.h" +#include "chrome/test/base/testing_profile.h" +#include "components/content_settings/core/browser/host_content_settings_map.h" +#include "components/content_settings/core/common/content_settings.h" +#include "components/content_settings/core/common/content_settings_pattern.h" +#include "components/content_settings/core/common/content_settings_types.h" +#include "content/public/browser/web_contents.h" +#include "content/public/test/mock_render_process_host.h" +#include "content/public/test/web_contents_tester.h" +#include "testing/gtest/include/gtest/gtest.h" + +class BackgroundSyncPermissionContextTest + : public ChromeRenderViewHostTestHarness { + protected: + BackgroundSyncPermissionContextTest() = default; + + ~BackgroundSyncPermissionContextTest() override = default; + + void NavigateAndRequestPermission( + const GURL& url, + BackgroundSyncPermissionContext* permission_context) { + content::WebContentsTester::For(web_contents())->NavigateAndCommit(url); + + base::RunLoop run_loop; + + const PermissionRequestID id( + web_contents()->GetRenderProcessHost()->GetID(), + web_contents()->GetMainFrame()->GetRoutingID(), -1 /* request_id */); + permission_context->RequestPermission( + web_contents(), id, url, + base::Bind( + &BackgroundSyncPermissionContextTest::TrackPermissionDecision, + base::Unretained(this), run_loop.QuitClosure())); + + run_loop.Run(); + } + + void TrackPermissionDecision(base::Closure done_closure, + ContentSetting content_setting) { + permission_granted_ = content_setting == CONTENT_SETTING_ALLOW; + done_closure.Run(); + } + + bool permission_granted() const { return permission_granted_; } + + private: + bool permission_granted_; + + DISALLOW_COPY_AND_ASSIGN(BackgroundSyncPermissionContextTest); +}; + +// Background sync permission should be allowed by default for a secure origin. +TEST_F(BackgroundSyncPermissionContextTest, TestSecureRequestingUrl) { + GURL url("https://www.example.com"); + BackgroundSyncPermissionContext permission_context(profile()); + + NavigateAndRequestPermission(url, &permission_context); + + EXPECT_TRUE(permission_granted()); +} + +// Background sync permission should be denied for an insecure origin. +TEST_F(BackgroundSyncPermissionContextTest, TestInsecureRequestingUrl) { + GURL url("http://example.com"); + BackgroundSyncPermissionContext permission_context(profile()); + + NavigateAndRequestPermission(url, &permission_context); + + EXPECT_FALSE(permission_granted()); +} + +// Tests that blocking one origin does not affect the others. +TEST_F(BackgroundSyncPermissionContextTest, TestBlockOrigin) { + GURL url1("https://www.example1.com"); + GURL url2("https://www.example2.com"); + BackgroundSyncPermissionContext permission_context(profile()); + HostContentSettingsMapFactory::GetForProfile(profile())->SetContentSetting( + ContentSettingsPattern::FromURLNoWildcard(url1), + ContentSettingsPattern::FromURLNoWildcard(url1), + CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, std::string(), + CONTENT_SETTING_BLOCK); + + NavigateAndRequestPermission(url1, &permission_context); + + EXPECT_FALSE(permission_granted()); + + NavigateAndRequestPermission(url2, &permission_context); + + EXPECT_TRUE(permission_granted()); +} diff --git a/chrome/browser/permissions/permission_context.cc b/chrome/browser/permissions/permission_context.cc index cf94b07d714809..2cb1677ac9740e 100644 --- a/chrome/browser/permissions/permission_context.cc +++ b/chrome/browser/permissions/permission_context.cc @@ -5,6 +5,8 @@ #include "chrome/browser/permissions/permission_context.h" #include "build/build_config.h" +#include "chrome/browser/background_sync/background_sync_permission_context.h" +#include "chrome/browser/background_sync/background_sync_permission_context_factory.h" #include "chrome/browser/geolocation/geolocation_permission_context.h" #include "chrome/browser/geolocation/geolocation_permission_context_factory.h" #include "chrome/browser/media/media_stream_camera_permission_context_factory.h" @@ -57,6 +59,8 @@ PermissionContextBase* PermissionContext::Get(Profile* profile, return MediaStreamMicPermissionContextFactory::GetForProfile(profile); case PermissionType::VIDEO_CAPTURE: return MediaStreamCameraPermissionContextFactory::GetForProfile(profile); + case PermissionType::BACKGROUND_SYNC: + return BackgroundSyncPermissionContextFactory::GetForProfile(profile); default: NOTREACHED() << "No PermissionContext associated with " << static_cast(permission_type); @@ -84,6 +88,7 @@ const std::list& PermissionContext::GetFactories() { factories.push_back(MediaStreamMicPermissionContextFactory::GetInstance()); factories.push_back( MediaStreamCameraPermissionContextFactory::GetInstance()); + factories.push_back(BackgroundSyncPermissionContextFactory::GetInstance()); } return factories; diff --git a/chrome/browser/permissions/permission_manager.cc b/chrome/browser/permissions/permission_manager.cc index ccc98f41c4d961..c1a719a45241bd 100644 --- a/chrome/browser/permissions/permission_manager.cc +++ b/chrome/browser/permissions/permission_manager.cc @@ -84,6 +84,8 @@ ContentSettingsType PermissionTypeToContentSetting(PermissionType permission) { return CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC; case PermissionType::VIDEO_CAPTURE: return CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA; + case PermissionType::BACKGROUND_SYNC: + return CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC; case PermissionType::NUM: // This will hit the NOTREACHED below. break; diff --git a/chrome/browser/permissions/permission_uma_util.cc b/chrome/browser/permissions/permission_uma_util.cc index 651ecfec7057b6..77b5f0930f84ce 100644 --- a/chrome/browser/permissions/permission_uma_util.cc +++ b/chrome/browser/permissions/permission_uma_util.cc @@ -149,7 +149,10 @@ void RecordPermissionAction(PermissionType permission, UMA_HISTOGRAM_ENUMERATION("Permissions.Action.VideoCapture", action, PERMISSION_ACTION_NUM); break; + // The user is not prompted for these permissions, thus there is no + // permission action recorded for them. case PermissionType::MIDI: + case PermissionType::BACKGROUND_SYNC: case PermissionType::NUM: NOTREACHED() << "PERMISSION " << PermissionUtil::GetPermissionString(permission) @@ -245,7 +248,7 @@ void RecordPermissionRequest(PermissionType permission, } } -} // namespace +} // anonymous namespace // Make sure you update histograms.xml permission histogram_suffix if you // add new permission diff --git a/chrome/browser/permissions/permission_util.cc b/chrome/browser/permissions/permission_util.cc index 12fb7bbe1b7b43..d4369c4609207a 100644 --- a/chrome/browser/permissions/permission_util.cc +++ b/chrome/browser/permissions/permission_util.cc @@ -33,6 +33,8 @@ std::string PermissionUtil::GetPermissionString( return "VideoCapture"; case content::PermissionType::MIDI: return "Midi"; + case content::PermissionType::BACKGROUND_SYNC: + return "BackgroundSync"; case content::PermissionType::NUM: break; } @@ -56,6 +58,8 @@ bool PermissionUtil::GetPermissionType(ContentSettingsType type, *out = PermissionType::VIDEO_CAPTURE; } else if (type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC) { *out = PermissionType::AUDIO_CAPTURE; + } else if (type == CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC) { + *out = PermissionType::BACKGROUND_SYNC; #if defined(OS_ANDROID) || defined(OS_CHROMEOS) } else if (type == CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER) { *out = PermissionType::PROTECTED_MEDIA_IDENTIFIER; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 0b4e00a85cab36..4811563e9e4ff7 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -40,6 +40,10 @@ 'browser/background_sync/background_sync_controller_factory.h', 'browser/background_sync/background_sync_controller_impl.cc', 'browser/background_sync/background_sync_controller_impl.h', + 'browser/background_sync/background_sync_permission_context.cc', + 'browser/background_sync/background_sync_permission_context.h', + 'browser/background_sync/background_sync_permission_context_factory.cc', + 'browser/background_sync/background_sync_permission_context_factory.h', 'browser/bad_message.cc', 'browser/bad_message.h', 'browser/banners/app_banner_data_fetcher.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 4f48f9cd101584..aef3c2f27da423 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -35,6 +35,7 @@ 'browser/autocomplete/search_provider_unittest.cc', 'browser/autocomplete/shortcuts_provider_extension_unittest.cc', 'browser/background_sync/background_sync_controller_impl_unittest.cc', + 'browser/background_sync/background_sync_permission_context_unittest.cc', 'browser/banners/app_banner_data_fetcher_unittest.cc', 'browser/banners/app_banner_settings_helper_unittest.cc', 'browser/bitmap_fetcher/bitmap_fetcher_service_unittest.cc', diff --git a/components/content_settings/core/browser/content_settings_registry.cc b/components/content_settings/core/browser/content_settings_registry.cc index ee738a6e30acdd..9c66ce35e919c8 100644 --- a/components/content_settings/core/browser/content_settings_registry.cc +++ b/components/content_settings/core/browser/content_settings_registry.cc @@ -264,6 +264,13 @@ void ContentSettingsRegistry::Init() { WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE, ContentSettingsInfo::INHERIT_IN_INCOGNITO); + Register(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, "background-sync", + CONTENT_SETTING_ALLOW, WebsiteSettingsInfo::UNSYNCABLE, + WhitelistedSchemes(), + ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK), + WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE, + ContentSettingsInfo::INHERIT_IN_INCOGNITO); + // Content settings that aren't used to store any data. TODO(raymes): use a // different mechanism rather than content settings to represent these. // Since nothing is stored in them, there is no real point in them being a diff --git a/components/content_settings/core/common/content_settings.cc b/components/content_settings/core/common/content_settings.cc index 5b0e36f0749152..18adb80be42bab 100644 --- a/components/content_settings/core/common/content_settings.cc +++ b/components/content_settings/core/common/content_settings.cc @@ -54,6 +54,7 @@ ContentSettingsType kHistogramOrder[] = { CONTENT_SETTINGS_TYPE_DURABLE_STORAGE, CONTENT_SETTINGS_TYPE_KEYGEN, CONTENT_SETTINGS_TYPE_BLUETOOTH_GUARD, + CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, }; int ContentSettingTypeToHistogramValue(ContentSettingsType content_setting, diff --git a/components/content_settings/core/common/content_settings_types.h b/components/content_settings/core/common/content_settings_types.h index 0e91e3f11e75fc..1c296c2a3d0b5c 100644 --- a/components/content_settings/core/common/content_settings_types.h +++ b/components/content_settings/core/common/content_settings_types.h @@ -46,6 +46,7 @@ enum ContentSettingsType { CONTENT_SETTINGS_TYPE_USB_CHOOSER_DATA, CONTENT_SETTINGS_TYPE_BLUETOOTH_GUARD, CONTENT_SETTINGS_TYPE_KEYGEN, + CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, // WARNING: This enum is going to be removed soon. Do not depend on NUM_TYPES. CONTENT_SETTINGS_NUM_TYPES_DO_NOT_USE, diff --git a/content/public/browser/permission_type.h b/content/public/browser/permission_type.h index d2f7b6d85a131c..ec9e8c5c082a50 100644 --- a/content/public/browser/permission_type.h +++ b/content/public/browser/permission_type.h @@ -22,6 +22,7 @@ enum class PermissionType { DURABLE_STORAGE = 7, AUDIO_CAPTURE = 8, VIDEO_CAPTURE = 9, + BACKGROUND_SYNC = 10, // Always keep this at the end. NUM, diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 4446fd0789baed..b03657f4587f0a 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -61190,6 +61190,10 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + + + @@ -76482,6 +76486,7 @@ To add a new entry, add it with any value and run test to compute valid value. +