Extensions: Make it possible for permission parsing code to return a detailed error message.

chrome/common/extensions/permissions/media_galleries_permission.cc

Index: chrome/common/extensions/permissions/media_galleries_permission.cc
===================================================================
--- chrome/common/extensions/permissions/media_galleries_permission.cc	(revision 247036)
+++ chrome/common/extensions/permissions/media_galleries_permission.cc	(working copy)
@@ -17,11 +17,30 @@
 
 // copyTo permission requires delete permission as a prerequisite.
 // delete permission requires read permission as a prerequisite.
-bool IsValidPermissionSet(bool has_read, bool has_copy_to, bool has_delete) {
-  if (has_copy_to)
-    return has_read && has_delete;
-  if (has_delete)
-    return has_read;
+bool IsValidPermissionSet(bool has_read, bool has_copy_to, bool has_delete,
+                          std::string* error) {
+  if (has_copy_to) {
+    if (has_read && has_delete)
+      return true;
+
+    std::string error_msg;
+    if (has_read) {
+      error_msg = "copyTo permission requires delete permission";

[Yoyo Zhou, 2014/01/29 00:39:59]

I foresee somebody with ["copyTo", "read"] encountering this message and
changing it to ["copyTo", "delete"]. Why not just return the "read and delete"
message?

[Lei Zhang, 2014/01/29 00:47:36]

Done.

+    } else if (has_delete) {
+      error_msg = "copyTo permission requires read permission";
+    } else {
+      error_msg = "copyTo permission requires read and delete permissions";
+    }
+    if (error)
+      *error = error_msg;
+    return false;
+  }
+  if (has_delete) {
+    if (has_read)
+      return true;
+    *error = "delete permission requires read permission";
+    return false;
+  }
   return true;
 }
 
@@ -44,9 +63,11 @@
 MediaGalleriesPermission::~MediaGalleriesPermission() {
 }
 
-bool MediaGalleriesPermission::FromValue(const base::Value* value) {
+bool MediaGalleriesPermission::FromValue(const base::Value* value,
+                                         std::string* error) {
   if (!SetDisjunctionPermission<MediaGalleriesPermissionData,
-                                MediaGalleriesPermission>::FromValue(value)) {
+                                MediaGalleriesPermission>::FromValue(value,
+                                                                     error)) {
     return false;
   }
 
@@ -78,7 +99,7 @@
     return false;
   }
 
-  return IsValidPermissionSet(has_read, has_copy_to, has_delete);
+  return IsValidPermissionSet(has_read, has_copy_to, has_delete, error);
 }
 
 PermissionMessages MediaGalleriesPermission::GetMessages() const {
@@ -102,7 +123,7 @@
       has_delete = true;
   }
 
-  if (!IsValidPermissionSet(has_read, has_copy_to, has_delete)) {
+  if (!IsValidPermissionSet(has_read, has_copy_to, has_delete, NULL)) {
     NOTREACHED();
     return result;
   }