Cleanup: remove evil hack to suppress permission warnings for searchProvider

BUG=533086

Review URL: https://codereview.chromium.org/1774103002

Cr-Commit-Position: refs/heads/master@{#379840}

3 files changed, 1 insertions, 57 deletions

diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc
index cc2aae7..49db418 100644
--- a/chrome/common/extensions/permissions/chrome_permission_message_provider.cc
+++ b/chrome/common/extensions/permissions/chrome_permission_message_provider.cc
@@ -44,15 +44,6 @@ class ComparablePermission {
 };
 using ComparablePermissions = std::vector<ComparablePermission>;
 
-void DropPermissionParameter(APIPermission::ID id,
-                             PermissionIDSet* permissions) {
-  if (permissions->ContainsID(id)) {
-    // Erase the permission, and insert it again without a parameter.
-    permissions->erase(id);
-    permissions->insert(id);
-  }
-}
-
 }  // namespace
 
 typedef std::set<PermissionMessage> PermissionMsgSet;
@@ -185,20 +176,6 @@ bool ChromePermissionMessageProvider::IsAPIOrManifestPrivilegeIncrease(
   AddAPIPermissions(new_permissions, &new_ids);
   AddManifestPermissions(new_permissions, &new_ids);
 
-  // Ugly hack: Before M46 beta, we didn't store the parameter for settings
-  // override permissions in prefs (which is where |old_permissions| is coming
-  // from). To avoid a spurious permission increase warning, drop the parameter.
-  // See crbug.com/533086.
-  // TODO(treib,devlin): Remove this for M48, when hopefully all users will have
-  // updated prefs.
-  const APIPermission::ID kSettingsOverrideIDs[] = {
-      APIPermission::kHomepage, APIPermission::kSearchProvider,
-      APIPermission::kStartupPages};
-  for (auto id : kSettingsOverrideIDs) {
-    DropPermissionParameter(id, &old_ids);
-    DropPermissionParameter(id, &new_ids);
-  }
-
   // If all the IDs were already there, it's not a privilege increase.
   if (old_ids.Includes(new_ids))
     return false;
diff --git a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc
index 2eab2a4..9ff56dd 100644
--- a/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc
+++ b/chrome/common/extensions/permissions/chrome_permission_message_provider_unittest.cc
@@ -131,30 +131,4 @@ TEST_F(ChromePermissionMessageProviderUnittest,
   EXPECT_FALSE(message1.submessages().empty());
 }
 
-// Anti-test: Check that adding a parameter to a SettingsOverridePermission
-// doesn't trigger a privilege increase. This is because prior to M46 beta, we
-// failed to store the parameter in the granted_permissions pref. Now we do, and
-// we don't want to bother every user with a spurious permissions warning.
-// See crbug.com/533086.
-// TODO(treib,devlin): Remove this for M48, when hopefully all users will have
-// updated prefs.
-TEST_F(ChromePermissionMessageProviderUnittest,
-       EvilHackToSuppressSettingsOverrideParameter) {
-  const APIPermissionInfo* info =
-      PermissionsInfo::GetInstance()->GetByID(APIPermission::kSearchProvider);
-
-  APIPermissionSet granted_permissions;
-  granted_permissions.insert(new SettingsOverrideAPIPermission(info));
-
-  APIPermissionSet actual_permissions;
-  actual_permissions.insert(new SettingsOverrideAPIPermission(info, "a.com"));
-
-  EXPECT_FALSE(IsPrivilegeIncrease(granted_permissions, actual_permissions));
-
-  // Just to be safe: Adding the permission (with or without parameter) should
-  // still be considered a privilege escalation.
-  EXPECT_TRUE(IsPrivilegeIncrease(APIPermissionSet(), granted_permissions));
-  EXPECT_TRUE(IsPrivilegeIncrease(APIPermissionSet(), actual_permissions));
-}
-
 }  // namespace extensions
diff --git a/extensions/common/permissions/settings_override_permission.cc b/extensions/common/permissions/settings_override_permission.cc
index 43765d2..69318d829 100644
--- a/extensions/common/permissions/settings_override_permission.cc
+++ b/extensions/common/permissions/settings_override_permission.cc
@@ -48,14 +48,7 @@ bool SettingsOverrideAPIPermission::FromValue(
     const base::Value* value,
     std::string* /*error*/,
     std::vector<std::string>* unhandled_permissions) {
-  // Ugly hack: |value| being null should be an error. But before M46 beta, we
-  // didn't store the parameter for settings override permissions in prefs.
-  // See crbug.com/533086.
-  // TODO(treib,devlin): Remove this for M48, when hopefully all users will have
-  // updated prefs.
-  // This should read:
-  // return value && value->GetAsString(&setting_value_);
-  return !value || value->GetAsString(&setting_value_);
+  return value && value->GetAsString(&setting_value_);
 }
 
 scoped_ptr<base::Value> SettingsOverrideAPIPermission::ToValue() const {