Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2265)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/permissions/permission_context_base_unittest.cc

Issue 1610753002: Fixes Permission Bubbles never responding to duplicate requests (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | chrome/browser/ui/website_settings/permission_bubble_manager.h » ('j') | chrome/browser/ui/website_settings/permission_bubble_manager.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 65d54f09ebc7ec9a5e08cfca679676ccc996b7ed..e9db142e40eb80364b2c809ef989b2bec54ad560 100644
--- a/chrome/browser/permissions/permission_context_base_unittest.cc
+++ b/chrome/browser/permissions/permission_context_base_unittest.cc
@@ -4,6 +4,8 @@
#include "chrome/browser/permissions/permission_context_base.h"
+#include <vector>
+
#include "base/bind.h"
#include "base/command_line.h"
#include "base/macros.h"
@@ -27,7 +29,6 @@
#include "content/public/browser/render_frame_host.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"
#if defined(OS_ANDROID)
@@ -48,8 +49,6 @@ class TestPermissionContext : public PermissionContextBase {
const content::PermissionType permission_type,
const ContentSettingsType content_settings_type)
: PermissionContextBase(profile, permission_type, content_settings_type),
- permission_set_(false),
- permission_granted_(false),
tab_context_updated_(false),
field_trial_list_(
new base::FieldTrialList(new base::MockEntropyProvider)) {}
@@ -62,21 +61,14 @@ class TestPermissionContext : public PermissionContextBase {
}
#endif
- bool permission_granted() {
- return permission_granted_;
- }
-
- bool permission_set() {
- return permission_set_;
- }
+ const std::vector<ContentSetting>& decisions() { return decisions_; }
bool tab_context_updated() {
return tab_context_updated_;
}
void TrackPermissionDecision(ContentSetting content_setting) {
- permission_set_ = true;
- permission_granted_ = content_setting == CONTENT_SETTING_ALLOW;
+ decisions_.push_back(content_setting);
}
void ResetFieldTrialList() {
@@ -107,8 +99,7 @@ class TestPermissionContext : public PermissionContextBase {
}
private:
- bool permission_set_;
- bool permission_granted_;
+ std::vector<ContentSetting> decisions_;
bool tab_context_updated_;
scoped_ptr<base::FieldTrialList> field_trial_list_;
};
@@ -121,17 +112,31 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
void RespondToPermission(TestPermissionContext* context,
const PermissionRequestID& id,
const GURL& url,
- bool accept) {
+ ContentSetting response) {
+ DCHECK(response == CONTENT_SETTING_ALLOW ||
+ response == CONTENT_SETTING_BLOCK ||
+ response == CONTENT_SETTING_ASK);
#if defined(OS_ANDROID)
- context->GetInfoBarController()->OnPermissionSet(id, url, url, accept,
- accept);
+ bool update_content_setting = response != CONTENT_SETTING_ASK;
+ bool allowed = response == CONTENT_SETTING_ALLOW;
+ context->GetInfoBarController()->OnPermissionSet(
+ id, url, url, update_content_setting, allowed);
#else
PermissionBubbleManager* manager =
PermissionBubbleManager::FromWebContents(web_contents());
- if (accept)
- manager->Accept();
- else
- manager->Closing();
+ switch (response) {
+ case CONTENT_SETTING_ALLOW:
+ manager->Accept();
+ break;
+ case CONTENT_SETTING_BLOCK:
+ manager->Deny();
+ break;
+ case CONTENT_SETTING_ASK:
+ manager->Closing();
+ break;
+ default:
+ NOTREACHED();
+ }
#endif
}
@@ -140,7 +145,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
profile(), content::PermissionType::NOTIFICATIONS,
CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
GURL url("http://www.google.com");
- content::WebContentsTester::For(web_contents())->NavigateAndCommit(url);
+ NavigateAndCommit(url);
const PermissionRequestID id(
web_contents()->GetRenderProcessHost()->GetID(),
@@ -152,9 +157,9 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, true);
- EXPECT_TRUE(permission_context.permission_set());
- EXPECT_TRUE(permission_context.permission_granted());
+ RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ALLOW);
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
EXPECT_EQ(CONTENT_SETTING_ALLOW,
permission_context.GetContentSettingFromMap(url, url));
@@ -165,7 +170,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
profile(), content::PermissionType::MIDI_SYSEX,
CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
GURL url("http://www.google.es");
- content::WebContentsTester::For(web_contents())->NavigateAndCommit(url);
+ NavigateAndCommit(url);
const PermissionRequestID id(
web_contents()->GetRenderProcessHost()->GetID(),
@@ -177,9 +182,9 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, false);
- EXPECT_TRUE(permission_context.permission_set());
- EXPECT_FALSE(permission_context.permission_granted());
+ RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ASK);
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
EXPECT_EQ(CONTENT_SETTING_ASK,
permission_context.GetContentSettingFromMap(url, url));
@@ -192,7 +197,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
content_settings_type);
GURL url;
ASSERT_FALSE(url.is_valid());
- content::WebContentsTester::For(web_contents())->NavigateAndCommit(url);
+ NavigateAndCommit(url);
const PermissionRequestID id(
web_contents()->GetRenderProcessHost()->GetID(),
@@ -204,8 +209,8 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- EXPECT_TRUE(permission_context.permission_set());
- EXPECT_FALSE(permission_context.permission_granted());
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(CONTENT_SETTING_BLOCK, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
EXPECT_EQ(CONTENT_SETTING_ASK,
permission_context.GetContentSettingFromMap(url, url));
@@ -217,7 +222,7 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
TestPermissionContext permission_context(profile(), permission_type,
content_settings_type);
GURL url("https://www.google.com");
- content::WebContentsTester::For(web_contents())->NavigateAndCommit(url);
+ NavigateAndCommit(url);
const PermissionRequestID id(
web_contents()->GetRenderProcessHost()->GetID(),
@@ -229,9 +234,9 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
base::Bind(&TestPermissionContext::TrackPermissionDecision,
base::Unretained(&permission_context)));
- RespondToPermission(&permission_context, id, url, true);
- EXPECT_TRUE(permission_context.permission_set());
- EXPECT_TRUE(permission_context.permission_granted());
+ RespondToPermission(&permission_context, id, url, CONTENT_SETTING_ALLOW);
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]);
EXPECT_TRUE(permission_context.tab_context_updated());
EXPECT_EQ(CONTENT_SETTING_ALLOW,
permission_context.GetContentSettingFromMap(url, url));
@@ -265,6 +270,43 @@ class PermissionContextBaseTests : public ChromeRenderViewHostTestHarness {
EXPECT_TRUE(permission_context.IsPermissionKillSwitchOn());
}
+ // Don't call this more than once in the same test, as it persists data to
+ // HostContentSettingsMap.
+ void TestParallelRequests(ContentSetting response) {
+ TestPermissionContext permission_context(
+ profile(), content::PermissionType::NOTIFICATIONS,
+ CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
+ GURL url("http://www.google.com");
+ NavigateAndCommit(url);
+
+ const PermissionRequestID id0(
+ web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(), 0);
+ const PermissionRequestID id1(
+ web_contents()->GetRenderProcessHost()->GetID(),
+ web_contents()->GetMainFrame()->GetRoutingID(), 1);
+
+ permission_context.RequestPermission(
+ web_contents(), id0, url, true,
+ base::Bind(&TestPermissionContext::TrackPermissionDecision,
+ base::Unretained(&permission_context)));
+ permission_context.RequestPermission(
+ web_contents(), id1, url, true,
+ base::Bind(&TestPermissionContext::TrackPermissionDecision,
+ base::Unretained(&permission_context)));
+
+ EXPECT_EQ(0u, permission_context.decisions().size());
+
+ RespondToPermission(&permission_context, id0, url, response);
+
+ // TODO(johnme): This should be 2!
+ EXPECT_EQ(1u, permission_context.decisions().size());
+ EXPECT_EQ(response, permission_context.decisions()[0]);
+ EXPECT_TRUE(permission_context.tab_context_updated());
+
+ EXPECT_EQ(response, permission_context.GetContentSettingFromMap(url, url));
+ }
+
private:
// ChromeRenderViewHostTestHarness:
void SetUp() override {
@@ -373,3 +415,15 @@ TEST_F(PermissionContextBaseTests, TestGlobalKillSwitch) {
TestGlobalPermissionsKillSwitch(content::PermissionType::VIDEO_CAPTURE,
CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
}
+
+TEST_F(PermissionContextBaseTests, TestParallelRequestsAllowed) {
+ TestParallelRequests(CONTENT_SETTING_ALLOW);
+}
+
+TEST_F(PermissionContextBaseTests, TestParallelRequestsBlocked) {
+ TestParallelRequests(CONTENT_SETTING_BLOCK);
+}
+
+TEST_F(PermissionContextBaseTests, TestParallelRequestsDismissed) {
+ TestParallelRequests(CONTENT_SETTING_ASK);
+}
« no previous file with comments | « no previous file | chrome/browser/ui/website_settings/permission_bubble_manager.h » ('j') | chrome/browser/ui/website_settings/permission_bubble_manager.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698