[Extensions] Move permission withholding pref logic to extension prefs

Moved the key for this pref and the logic for getting / setting it from
ScriptingPermissionsModifier to ExtensionPrefs. This lays the groundwork
for making it cleaner to set this value on installation if permissions
were withheld on installation.

Bug: 984069
Change-Id: I5580dfbdd57f4e2a3e92dd7b62b02bf04b35fee9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762720Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Tim Judkins <tjudkins@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689170}

chrome/browser/extensions/scripting_permissions_modifier.cc

@@ -20,13 +20,6 @@ namespace extensions {
 
 namespace {
 
-// The entry into the ExtensionPrefs indicating that an extension should be
-// granted all the requested host permissions without requiring explicit runtime
-// permission from the user. The preference name is different for legacy
-// reasons.
-const char kGrantExtensionAllHostPermissionsPrefName[] =
-    "extension_can_script_all_urls";
-
 // Returns true if Chrome can potentially withhold permissions from the
 // extension.
 bool CanWithholdFromExtension(const Extension& extension) {
@@ -100,32 +93,6 @@ bool ShouldConsiderExtension(const Extension& extension) {
   return true;
 }
 
-base::Optional<bool> GetWithholdPermissionsPrefValue(
-    const ExtensionPrefs& prefs,
-    const ExtensionId& id) {
-  bool permissions_allowed = false;
-  if (!prefs.ReadPrefAsBoolean(id, kGrantExtensionAllHostPermissionsPrefName,
-                               &permissions_allowed)) {
-    return base::nullopt;
-  }
-  // NOTE: For legacy reasons, the preference stores whether the extension was
-  // allowed access to all its host permissions, rather than if Chrome should
-  // withhold permissions. Invert the boolean for backwards compatibility.
-  return !permissions_allowed;
-}
-
-void SetWithholdPermissionsPrefValue(ExtensionPrefs* prefs,
-                                     const ExtensionId& id,
-                                     bool should_withhold) {
-  // NOTE: For legacy reasons, the preference stores whether the extension was
-  // allowed access to all its host permissions, rather than if Chrome should
-  // withhold permissions. Invert the boolean for backwards compatibility.
-  bool permissions_allowed = !should_withhold;
-  prefs->UpdateExtensionPref(
-      id, kGrantExtensionAllHostPermissionsPrefName,
-      std::make_unique<base::Value>(permissions_allowed));
-}
-
 // Retrieves the effective list of runtime-granted permissions for a given
 // |extension| from the |prefs|. ExtensionPrefs doesn't store the valid schemes
 // for URLPatterns, which results in the chrome:-scheme being included for
@@ -219,7 +186,7 @@ void ScriptingPermissionsModifier::SetWithholdHostPermissions(
 
   // Set the pref first, so that listeners for permission changes get the proper
   // value if they query HasWithheldHostPermissions().
-  SetWithholdPermissionsPrefValue(extension_prefs_, extension_->id(),
+  extension_prefs_->SetShouldWithholdPermissions(extension_->id(),
                                                  should_withhold);
 
   if (should_withhold)
@@ -232,7 +199,7 @@ bool ScriptingPermissionsModifier::HasWithheldHostPermissions() const {
   DCHECK(CanAffectExtension());
 
   base::Optional<bool> pref_value =
-      GetWithholdPermissionsPrefValue(*extension_prefs_, extension_->id());
+      extension_prefs_->GetShouldWithholdPermissions(extension_->id());
   if (!pref_value.has_value()) {
     // If there is no value present, default to false.
     return false;
@@ -395,7 +362,7 @@ ScriptingPermissionsModifier::WithholdPermissionsIfNecessary(
   bool should_withhold = false;
   if (ShouldConsiderExtension(extension)) {
     base::Optional<bool> pref_value =
-        GetWithholdPermissionsPrefValue(extension_prefs, extension.id());
+        extension_prefs.GetShouldWithholdPermissions(extension.id());
     should_withhold = pref_value.has_value() && pref_value.value() == true;
   }
 

extensions/browser/extension_prefs.cc

@@ -137,6 +137,12 @@ constexpr const char kPrefUserDraggedApp[] = "user_dragged_app_ntp";
 constexpr const char kPrefActivePermissions[] = "active_permissions";
 constexpr const char kPrefGrantedPermissions[] = "granted_permissions";
 
+// A preference indicating if an extension should be granted all the requested
+// host permissions without requiring explicit runtime permission from the user.
+// The preference name is different for legacy reasons.
+const char kGrantExtensionAllHostPermissions[] =
+    "extension_can_script_all_urls";
+
 // The set of permissions that were granted at runtime, rather than at install
 // time. This includes permissions granted through the permissions API and
 // runtime host permissions.
@@ -1019,6 +1025,30 @@ void ExtensionPrefs::SetActivePermissions(const std::string& extension_id,
       extension_id, kPrefActivePermissions, permissions);
 }
 
+void ExtensionPrefs::SetShouldWithholdPermissions(
+    const ExtensionId& extension_id,
+    bool should_withhold) {
+  // NOTE: For legacy reasons, the preference stores whether the extension was
+  // allowed access to all its host permissions, rather than if Chrome should
+  // withhold permissions. Invert the boolean for backwards compatibility.
+  bool permissions_allowed = !should_withhold;
+  UpdateExtensionPref(extension_id, kGrantExtensionAllHostPermissions,
+                      std::make_unique<base::Value>(permissions_allowed));
+}
+
+base::Optional<bool> ExtensionPrefs::GetShouldWithholdPermissions(
+    const ExtensionId& extension_id) const {
+  bool permissions_allowed = false;
+  if (!ReadPrefAsBoolean(extension_id, kGrantExtensionAllHostPermissions,
+                         &permissions_allowed)) {
+    return base::nullopt;
+  }
+  // NOTE: For legacy reasons, the preference stores whether the extension was
+  // allowed access to all its host permissions, rather than if Chrome should
+  // withhold permissions. Invert the boolean for backwards compatibility.
+  return !permissions_allowed;
+}
+
 std::unique_ptr<const PermissionSet>
 ExtensionPrefs::GetRuntimeGrantedPermissions(
     const ExtensionId& extension_id) const {

extensions/browser/extension_prefs.h

@@ -398,6 +398,14 @@ class ExtensionPrefs : public KeyedService {
   void SetActivePermissions(const std::string& extension_id,
                             const PermissionSet& permissions);
 
+  // Sets/Gets the value indicating if an extension should be granted all the
+  // requested host permissions without requiring explicit runtime-granted
+  // permissions from the user.
+  void SetShouldWithholdPermissions(const ExtensionId& extension_id,
+                                    bool should_withhold);
+  base::Optional<bool> GetShouldWithholdPermissions(
+      const ExtensionId& extension_id) const;
+
   // Returns the set of runtime-granted permissions. These are permissions that
   // the user explicitly approved at runtime, rather than install time (such
   // as those granted through the permissions API or the runtime host