Commit 537399fd authored by Patti's avatar Patti Committed by Commit Bot

Page Info Android: Fix PageInfoTest.NonFactoryDefault...Shown test bug.

ShouldShowPermission() in page_info.cc always returns true for geolocation on
Android because Android does additional filtering to check if geolocation should
be shown due to DSE settings.
The test PageInfoTest.NonFactoryDefaultAndRecentlyChangedPermissionsShown
touches upon this in testing, but was coincidentally passing on Android because
it later expects geolocation to be shown regardless by changing it to a
non-default setting. Fix by changing the vector of the expected permissions to a

std: :set instead.
Change-Id: I9841563ddb6add41ea6f4d2edea4569d16b879a9
Reviewed-on: https://chromium-review.googlesource.com/813439Reviewed-by: default avatarRaymes Khoury <raymes@chromium.org>
Commit-Queue: Patti <patricialor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523691}
parent ff891640
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/ui/page_info/page_info.h" #include "chrome/browser/ui/page_info/page_info.h"
#include <set>
#include <string> #include <string>
#include <vector> #include <vector>
...@@ -213,17 +214,19 @@ bool PermissionInfoListContainsPermission(const PermissionInfoList& permissions, ...@@ -213,17 +214,19 @@ bool PermissionInfoListContainsPermission(const PermissionInfoList& permissions,
TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) { TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
page_info()->PresentSitePermissions(); page_info()->PresentSitePermissions();
std::vector<ContentSettingsType> expected_visible_permissions; std::set<ContentSettingsType> expected_visible_permissions;
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
// Geolocation is always allowed to pass through to Android-specific logic to // Geolocation is always allowed to pass through to Android-specific logic to
// check for DSE settings (so expect 1 item), but isn't actually shown later // check for DSE settings (so expect 1 item), but isn't actually shown later
// on because this test isn't testing with a default search engine origin. // on because this test isn't testing with a default search engine origin.
EXPECT_EQ(1uL, last_permission_info_list().size()); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_GEOLOCATION);
EXPECT_EQ(expected_visible_permissions.size(),
last_permission_info_list().size());
EXPECT_EQ(CONTENT_SETTINGS_TYPE_GEOLOCATION, EXPECT_EQ(CONTENT_SETTINGS_TYPE_GEOLOCATION,
last_permission_info_list().back().type); last_permission_info_list().back().type);
#else #else
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_PLUGINS); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_PLUGINS);
// Flash is always visible on desktop - see https://crbug.com/791142. // Flash is always visible on desktop - see https://crbug.com/791142.
EXPECT_EQ(expected_visible_permissions.size(), EXPECT_EQ(expected_visible_permissions.size(),
last_permission_info_list().size()); last_permission_info_list().size());
...@@ -234,24 +237,24 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) { ...@@ -234,24 +237,24 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
// Change some default-ask settings away from the default. // Change some default-ask settings away from the default.
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_GEOLOCATION, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_GEOLOCATION,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_GEOLOCATION); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_GEOLOCATION);
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_NOTIFICATIONS); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC,
CONTENT_SETTING_ALLOW); CONTENT_SETTING_ALLOW);
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC);
EXPECT_EQ(expected_visible_permissions.size(), EXPECT_EQ(expected_visible_permissions.size(),
last_permission_info_list().size()); last_permission_info_list().size());
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_POPUPS); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_POPUPS);
// Change a default-block setting to a user-preference block instead. // Change a default-block setting to a user-preference block instead.
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_POPUPS, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_POPUPS,
CONTENT_SETTING_BLOCK); CONTENT_SETTING_BLOCK);
EXPECT_EQ(expected_visible_permissions.size(), EXPECT_EQ(expected_visible_permissions.size(),
last_permission_info_list().size()); last_permission_info_list().size());
expected_visible_permissions.push_back(CONTENT_SETTINGS_TYPE_JAVASCRIPT); expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_JAVASCRIPT);
// Change a default-allow setting away from the default. // Change a default-allow setting away from the default.
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_JAVASCRIPT, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_JAVASCRIPT,
CONTENT_SETTING_BLOCK); CONTENT_SETTING_BLOCK);
...@@ -260,8 +263,7 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) { ...@@ -260,8 +263,7 @@ TEST_F(PageInfoTest, NonFactoryDefaultAndRecentlyChangedPermissionsShown) {
// Make sure changing a setting to its default causes it to show up, since it // Make sure changing a setting to its default causes it to show up, since it
// has been recently changed. // has been recently changed.
expected_visible_permissions.push_back( expected_visible_permissions.insert(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA);
page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, page_info()->OnSitePermissionChanged(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
CONTENT_SETTING_DEFAULT); CONTENT_SETTING_DEFAULT);
EXPECT_EQ(expected_visible_permissions.size(), EXPECT_EQ(expected_visible_permissions.size(),
......
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