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

chrome/common/extensions/permissions/media_galleries_permission.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/extensions/permissions/media_galleries_permission.h"
 
 #include <set>
 #include <string>
 
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "extensions/common/permissions/permissions_info.h"
 #include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
 // 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;
 }
 
 }  // namespace
 
 namespace extensions {
 
 const char MediaGalleriesPermission::kAllAutoDetectedPermission[] =
     "allAutoDetected";
 const char MediaGalleriesPermission::kReadPermission[] = "read";
 const char MediaGalleriesPermission::kCopyToPermission[] = "copyTo";
 const char MediaGalleriesPermission::kDeletePermission[] = "delete";
 
 MediaGalleriesPermission::MediaGalleriesPermission(
     const APIPermissionInfo* info)
   : SetDisjunctionPermission<MediaGalleriesPermissionData,
                              MediaGalleriesPermission>(info) {
 }
 
 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;
   }
 
   bool has_read = false;
   bool has_copy_to = false;
   bool has_delete = false;
   for (std::set<MediaGalleriesPermissionData>::const_iterator it =
       data_set_.begin(); it != data_set_.end(); ++it) {
     if (it->permission() == kAllAutoDetectedPermission) {
       continue;
@@ skipping 11 matching lines @@
       continue;
     }
 
     // No other permissions, so reaching this means
     // MediaGalleriesPermissionData is probably out of sync in some way.
     // Fail so developers notice this.
     NOTREACHED();
     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 {
   DCHECK(HasMessages());
   PermissionMessages result;
 
   bool has_all_auto_detected = false;
   bool has_read = false;
   bool has_copy_to = false;
   bool has_delete = false;
 
   for (std::set<MediaGalleriesPermissionData>::const_iterator it =
       data_set_.begin(); it != data_set_.end(); ++it) {
     if (it->permission() == kAllAutoDetectedPermission)
       has_all_auto_detected = true;
     else if (it->permission() == kReadPermission)
       has_read = true;
     else if (it->permission() == kCopyToPermission)
       has_copy_to = true;
     else if (it->permission() == kDeletePermission)
       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;
   }
 
   // If |has_all_auto_detected| is false, then Chrome will prompt the user at
   // runtime when the extension call the getMediaGalleries API.
   if (!has_all_auto_detected)
     return result;
   // No access permission case.
   if (!has_read)
@@ skipping 17 matching lines @@
   }
   if (has_delete) {
     result.push_back(PermissionMessage(
         PermissionMessage::kMediaGalleriesAllGalleriesDelete,
         base::string16()));
   }
   return result;
 }
 
 }  // namespace extensions