Index: chrome/browser/permissions/permission_context_base_unittest.cc |
diff --git a/chrome/browser/permissions/permission_context_base_unittest.cc b/chrome/browser/permissions/permission_context_base_unittest.cc |
index 4dfe1e4118a4e5217d71ae5a570799305f596080..a7774f847b7664e08770cad2829e22a0ecc6f541 100644 |
--- a/chrome/browser/permissions/permission_context_base_unittest.cc |
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc |
@@ -5,6 +5,7 @@ |
#include "chrome/browser/permissions/permission_context_base.h" |
#include <map> |
+#include <set> |
#include <string> |
#include <utility> |
#include <vector> |
@@ -14,6 +15,7 @@ |
#include "base/macros.h" |
#include "base/memory/ptr_util.h" |
#include "base/metrics/field_trial.h" |
+#include "base/run_loop.h" |
#include "base/test/histogram_tester.h" |
#include "base/test/mock_entropy_provider.h" |
#include "base/test/scoped_feature_list.h" |
@@ -31,7 +33,10 @@ |
#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_types.h" |
+#include "components/safe_browsing_db/database_manager.h" |
+#include "components/safe_browsing_db/test_database_manager.h" |
#include "components/variations/variations_associated_data.h" |
+#include "content/public/browser/browser_thread.h" |
#include "content/public/browser/permission_type.h" |
#include "content/public/browser/render_frame_host.h" |
#include "content/public/browser/web_contents.h" |
@@ -50,6 +55,49 @@ const char kPermissionsKillSwitchTestGroup[] = "TestGroup"; |
const char* const kPromptGroupName = kPermissionsKillSwitchTestGroup; |
const char kPromptTrialName[] = "PermissionPromptsUX"; |
+class MockSafeBrowsingDatabaseManager |
+ : public safe_browsing::TestSafeBrowsingDatabaseManager { |
+ public: |
+ explicit MockSafeBrowsingDatabaseManager(bool perform_callback) |
+ : perform_callback_(perform_callback) {} |
+ |
+ bool CheckApiBlacklistUrl( |
+ const GURL& url, |
+ safe_browsing::SafeBrowsingDatabaseManager::Client* client) override { |
+ if (perform_callback_) { |
+ safe_browsing::ThreatMetadata metadata; |
+ std::map<GURL, std::set<std::string>>::iterator blacklisted_permissions = |
raymes
2017/01/09 06:28:18
nit: It would be fine to use "auto" here. We also
meredithl
2017/01/10 02:24:57
Done. Still getting used to C++11 :)
|
+ permissions_blacklist_.find(url); |
+ if (blacklisted_permissions != permissions_blacklist_.end()) |
+ metadata.api_permissions = blacklisted_permissions->second; |
+ client->OnCheckApiBlacklistUrlResult(url, metadata); |
+ } |
+ // Returns false if scheme is HTTP/HTTPS and able to be checked. |
+ return false; |
+ } |
+ |
+ bool CancelApiCheck(Client* client) override { |
+ if (perform_callback_) |
raymes
2017/01/09 06:28:17
nit: can have DCHECK(!perform_callback_) here to a
meredithl
2017/01/10 02:24:57
Done.
|
+ NOTREACHED(); |
+ // Returns true when client check could be stopped. |
+ return true; |
+ } |
+ |
+ void BlacklistUrlPermissions(const GURL& url, |
+ const std::set<std::string> permissions) { |
+ permissions_blacklist_[url] = permissions; |
+ } |
+ |
+ protected: |
+ ~MockSafeBrowsingDatabaseManager() override {} |
+ |
+ private: |
+ bool perform_callback_; |
+ std::map<GURL, std::set<std::string>> permissions_blacklist_; |
+ |
+ DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingDatabaseManager); |
+}; |
+ |
class TestPermissionContext : public PermissionContextBase { |
public: |
TestPermissionContext(Profile* profile, |
@@ -70,8 +118,12 @@ class TestPermissionContext : public PermissionContextBase { |
bool tab_context_updated() const { return tab_context_updated_; } |
+ // Once a decision for the requested permission has been made, run the |
+ // callback. |
void TrackPermissionDecision(ContentSetting content_setting) { |
decisions_.push_back(content_setting); |
+ content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE, |
+ quit_closure_); |
raymes
2017/01/09 06:28:17
Can we just call quit_closure.Run() here instead o
meredithl
2017/01/10 02:24:57
It appears we can! These test methods have been ha
|
} |
ContentSetting GetContentSettingFromMap(const GURL& url_a, |
@@ -81,6 +133,39 @@ class TestPermissionContext : public PermissionContextBase { |
content_settings_type(), std::string()); |
} |
+ void RequestPermission(content::WebContents* web_contents, |
+ const PermissionRequestID& id, |
+ const GURL& requesting_frame, |
+ bool user_gesture, |
+ const BrowserPermissionCallback& callback) override { |
+ base::RunLoop run_loop; |
+ quit_closure_ = run_loop.QuitClosure(); |
+ PermissionContextBase::RequestPermission(web_contents, id, requesting_frame, |
+ true /* user_gesture */, callback); |
+ run_loop.Run(); |
+ } |
+ |
+ void DecidePermission(content::WebContents* web_contents, |
+ const PermissionRequestID& id, |
+ const GURL& requesting_origin, |
+ const GURL& embedding_origin, |
+ bool user_gesture, |
+ const BrowserPermissionCallback& callback) override { |
+ PermissionContextBase::DecidePermission(web_contents, id, requesting_origin, |
+ embedding_origin, user_gesture, |
+ callback); |
+ if (respond_permission_) |
+ respond_permission_.Run(); |
raymes
2017/01/09 06:28:17
Same here, we probably want to reset this after ru
meredithl
2017/01/10 02:24:57
Done.
|
+ else |
+ quit_closure_.Run(); |
raymes
2017/01/09 06:28:18
I wonder if we can just remove this line?
meredithl
2017/01/10 02:24:57
respond_permission_ only gets set in the TestParal
|
+ } |
+ |
+ // Permission request will need to be responded to, so pass a callback to be |
+ // run once the request has completed and the decision has been made. |
+ void SetRespondPermissionCallback(base::Closure callback) { |
+ respond_permission_ = callback; |
+ } |
+ |
protected: |
void UpdateTabContext(const PermissionRequestID& id, |
const GURL& requesting_origin, |
@@ -95,7 +180,10 @@ class TestPermissionContext : public PermissionContextBase { |
private: |
std::vector<ContentSetting> decisions_; |
bool tab_context_updated_; |
- |
+ base::Closure quit_closure_; |
+ // Callback for responding to a permission once the request has been completed |
+ // (valid URL, kill switch disabled, not blacklisted) |
+ base::Closure respond_permission_; |
DISALLOW_COPY_AND_ASSIGN(TestPermissionContext); |
}; |
@@ -180,13 +268,15 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), |
-1); |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, |
+ persist, decision)); |
permission_context.RequestPermission( |
web_contents(), |
id, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
- |
- RespondToPermission(&permission_context, id, url, persist, decision); |
ASSERT_EQ(1u, permission_context.decisions().size()); |
EXPECT_EQ(decision, permission_context.decisions()[0]); |
EXPECT_TRUE(permission_context.tab_context_updated()); |
@@ -237,13 +327,16 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
const PermissionRequestID id( |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), i); |
+ |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, |
+ false, CONTENT_SETTING_ASK)); |
+ |
permission_context.RequestPermission( |
web_contents(), id, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
- |
- RespondToPermission(&permission_context, id, url, false, /* persist */ |
- CONTENT_SETTING_ASK); |
histograms.ExpectTotalCount( |
"Permissions.Prompt.Dismissed.PriorDismissCount." + |
PermissionUtil::GetPermissionString(permission_type), |
@@ -281,13 +374,15 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
const PermissionRequestID id( |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), i); |
+ |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, |
+ false, CONTENT_SETTING_ASK)); |
permission_context.RequestPermission( |
web_contents(), id, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
- |
- RespondToPermission(&permission_context, id, url, false, /* persist */ |
- CONTENT_SETTING_ASK); |
histograms.ExpectTotalCount( |
"Permissions.Prompt.Dismissed.PriorDismissCount.Geolocation", |
i + 1); |
@@ -365,13 +460,15 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
const PermissionRequestID id( |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), i); |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, |
+ false, CONTENT_SETTING_ASK)); |
permission_context.RequestPermission( |
web_contents(), id, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
- base::Unretained(&permission_context))); |
+ base::Unretained(&permission_context))); |
- RespondToPermission(&permission_context, id, url, false, /* persist */ |
- CONTENT_SETTING_ASK); |
EXPECT_EQ(1u, permission_context.decisions().size()); |
ASSERT_EQ(expected, permission_context.decisions()[0]); |
EXPECT_TRUE(permission_context.tab_context_updated()); |
@@ -431,14 +528,17 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), |
-1); |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, true, |
+ CONTENT_SETTING_ALLOW)); |
+ |
permission_context.RequestPermission( |
web_contents(), |
id, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
- RespondToPermission(&permission_context, id, url, true, /* persist */ |
- CONTENT_SETTING_ALLOW); |
ASSERT_EQ(1u, permission_context.decisions().size()); |
EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]); |
EXPECT_TRUE(permission_context.tab_context_updated()); |
@@ -490,21 +590,27 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
web_contents()->GetRenderProcessHost()->GetID(), |
web_contents()->GetMainFrame()->GetRoutingID(), 1); |
+ bool persist = (response == CONTENT_SETTING_ALLOW || |
+ response == CONTENT_SETTING_BLOCK); |
+ |
+ // Request a permission without setting the callback to DecidePermission. |
permission_context.RequestPermission( |
web_contents(), id0, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
+ |
+ EXPECT_EQ(0u, permission_context.decisions().size()); |
+ |
+ // Set the callback, and make a second permission request. |
+ permission_context.SetRespondPermissionCallback( |
+ base::Bind(&PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id0, url, |
+ persist, response)); |
permission_context.RequestPermission( |
web_contents(), id1, url, true /* user_gesture */, |
base::Bind(&TestPermissionContext::TrackPermissionDecision, |
base::Unretained(&permission_context))); |
- EXPECT_EQ(0u, permission_context.decisions().size()); |
- |
- bool persist = (response == CONTENT_SETTING_ALLOW || |
- response == CONTENT_SETTING_BLOCK); |
- RespondToPermission(&permission_context, id0, url, persist, response); |
- |
ASSERT_EQ(2u, permission_context.decisions().size()); |
EXPECT_EQ(response, permission_context.decisions()[0]); |
EXPECT_EQ(response, permission_context.decisions()[1]); |
@@ -513,6 +619,35 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness { |
EXPECT_EQ(response, permission_context.GetContentSettingFromMap(url, url)); |
} |
+ void TestPermissionsBlacklisting( |
+ content::PermissionType permission_type, |
+ ContentSettingsType content_settings_type, |
+ scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager, |
+ const GURL& url, |
+ int timeout, |
+ ContentSetting response) { |
+ NavigateAndCommit(url); |
+ base::test::ScopedFeatureList scoped_feature_list; |
+ scoped_feature_list.InitAndEnableFeature(features::kPermissionsBlacklist); |
+ TestPermissionContext permission_context(profile(), permission_type, |
+ content_settings_type); |
+ permission_context.SetSafeBrowsingDatabaseManagerAndTimeoutForTest( |
+ db_manager, timeout); |
+ const PermissionRequestID id( |
+ web_contents()->GetRenderProcessHost()->GetID(), |
+ web_contents()->GetMainFrame()->GetRoutingID(), -1); |
+ permission_context.SetRespondPermissionCallback(base::Bind( |
+ &PermissionContextBaseTests::RespondToPermission, |
+ base::Unretained(this), &permission_context, id, url, false, response)); |
raymes
2017/01/10 02:57:15
Is this needed?
meredithl
2017/01/10 03:40:46
Turns out for simplicity it is, because we also wa
|
+ permission_context.RequestPermission( |
+ web_contents(), id, url, true /* user_gesture */, |
+ base::Bind(&TestPermissionContext::TrackPermissionDecision, |
+ base::Unretained(&permission_context))); |
+ |
+ ASSERT_EQ(1u, permission_context.decisions().size()); |
+ EXPECT_EQ(response, permission_context.decisions()[0]); |
+ } |
+ |
private: |
// ChromeRenderViewHostTestHarness: |
void SetUp() override { |
@@ -667,3 +802,54 @@ TEST_F(PermissionContextBaseTests, TestParallelRequestsBlocked) { |
TEST_F(PermissionContextBaseTests, TestParallelRequestsDismissed) { |
TestParallelRequests(CONTENT_SETTING_ASK); |
} |
+ |
+// Tests a blacklisted (URL, permission) pair has had its permission request |
+// blocked. |
+TEST_F(PermissionContextBaseTests, TestPermissionsBlacklistingBlocked) { |
+ scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager = |
+ new MockSafeBrowsingDatabaseManager(true); |
raymes
2017/01/09 06:28:18
nit: here and below, document the bool parameter t
meredithl
2017/01/10 02:24:57
Done.
|
+ const GURL url("https://www.example.com"); |
+ std::set<std::string> blacklisted_permissions{ |
+ PermissionUtil::GetPermissionString( |
+ content::PermissionType::GEOLOCATION)}; |
+ db_manager->BlacklistUrlPermissions(url, blacklisted_permissions); |
+ TestPermissionsBlacklisting(content::PermissionType::GEOLOCATION, |
+ CONTENT_SETTINGS_TYPE_GEOLOCATION, db_manager, |
+ url, 2000 /* timeout */, CONTENT_SETTING_BLOCK); |
+} |
+ |
+// Tests that a URL with a blacklisted permission is permitted to request a |
+// non-blacklisted permission. |
+TEST_F(PermissionContextBaseTests, TestPermissionsBlacklistingAllowed) { |
+ scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager = |
+ new MockSafeBrowsingDatabaseManager( |
+ true /* run client with result method immediately */); |
raymes
2017/01/09 06:28:18
nit: just put perform_callback. If you want to add
meredithl
2017/01/10 02:24:57
Done.
|
+ const GURL url("https://www.example.com"); |
+ std::set<std::string> blacklisted_permissions{ |
+ PermissionUtil::GetPermissionString( |
+ content::PermissionType::GEOLOCATION)}; |
+ db_manager->BlacklistUrlPermissions(url, blacklisted_permissions); |
+ TestPermissionsBlacklisting( |
+ content::PermissionType::GEOLOCATION, CONTENT_SETTINGS_TYPE_GEOLOCATION, |
+ db_manager, url, 2000 /* timeout in ms */, CONTENT_SETTING_BLOCK); |
+ TestPermissionsBlacklisting(content::PermissionType::NOTIFICATIONS, |
+ CONTENT_SETTINGS_TYPE_NOTIFICATIONS, db_manager, |
+ url, 2000 /* timeout in ms */, |
+ CONTENT_SETTING_ASK); |
+} |
+ |
+// Tests that a URL with a blacklisted permisison is permitted to request that |
+// permission if Safe Browsing has timed out. |
+TEST_F(PermissionContextBaseTests, TestSafeBrowsingTimeout) { |
+ scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager = |
+ new MockSafeBrowsingDatabaseManager( |
+ false /* don't call client with result */); |
raymes
2017/01/09 06:28:18
nit: same thing here
meredithl
2017/01/10 02:24:57
Done.
|
+ const GURL url("https://www.example.com"); |
+ std::set<std::string> blacklisted_permissions{ |
+ PermissionUtil::GetPermissionString( |
+ content::PermissionType::GEOLOCATION)}; |
+ db_manager->BlacklistUrlPermissions(url, blacklisted_permissions); |
+ TestPermissionsBlacklisting(content::PermissionType::GEOLOCATION, |
+ CONTENT_SETTINGS_TYPE_GEOLOCATION, db_manager, |
+ url, 0 /* timeout in ms */, CONTENT_SETTING_ASK); |
+} |