diff options
Diffstat (limited to 'chrome')
13 files changed, 131 insertions, 100 deletions
diff --git a/chrome/browser/extensions/api/permissions/permissions_api_helpers.cc b/chrome/browser/extensions/api/permissions/permissions_api_helpers.cc index 2991d79..fd70ba7 100644 --- a/chrome/browser/extensions/api/permissions/permissions_api_helpers.cc +++ b/chrome/browser/extensions/api/permissions/permissions_api_helpers.cc @@ -71,6 +71,7 @@ scoped_refptr<PermissionSet> UnpackPermissionSet( const Permissions& permissions, bool allow_file_access, std::string* error) { + DCHECK(error); APIPermissionSet apis; std::vector<std::string>* permissions_list = permissions.permissions.get(); if (permissions_list) { @@ -107,7 +108,7 @@ scoped_refptr<PermissionSet> UnpackPermissionSet( } CHECK(permission); - if (!permission->FromValue(permission_json.get())) { + if (!permission->FromValue(permission_json.get(), NULL)) { *error = ErrorUtils::FormatErrorMessage(kInvalidParameter, *it); return NULL; } diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index a7ea6cf..7d81c9f 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -200,8 +200,7 @@ class ExtensionPrefsGrantedPermissions : public ExtensionPrefsTest { value->Append(new base::StringValue("tcp-connect:*.example.com:80")); value->Append(new base::StringValue("udp-bind::8080")); value->Append(new base::StringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) - NOTREACHED(); + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } api_perm_set1_.insert(permission.release()); diff --git a/chrome/common/extensions/permissions/bluetooth_permission.cc b/chrome/common/extensions/permissions/bluetooth_permission.cc index 4d82868..15289ce 100644 --- a/chrome/common/extensions/permissions/bluetooth_permission.cc +++ b/chrome/common/extensions/permissions/bluetooth_permission.cc @@ -25,7 +25,8 @@ BluetoothPermission::BluetoothPermission(const APIPermissionInfo* info) BluetoothPermission::~BluetoothPermission() { } -bool BluetoothPermission::FromValue(const base::Value* value) { +bool BluetoothPermission::FromValue(const base::Value* value, + std::string* error) { // Value may be omitted to gain access to non-profile functions. if (!value) return true; @@ -36,7 +37,7 @@ bool BluetoothPermission::FromValue(const base::Value* value) { return true; if (!SetDisjunctionPermission<BluetoothPermissionData, - BluetoothPermission>::FromValue(value)) { + BluetoothPermission>::FromValue(value, error)) { return false; } diff --git a/chrome/common/extensions/permissions/bluetooth_permission.h b/chrome/common/extensions/permissions/bluetooth_permission.h index 5eea337..8d0c009 100644 --- a/chrome/common/extensions/permissions/bluetooth_permission.h +++ b/chrome/common/extensions/permissions/bluetooth_permission.h @@ -32,7 +32,8 @@ class BluetoothPermission // SetDisjunctionPermission overrides. // BluetoothPermission permits an empty list for gaining permission to the // Bluetooth APIs without implementing a profile. - virtual bool FromValue(const base::Value* value) OVERRIDE; + virtual bool FromValue(const base::Value* value, + std::string* error) OVERRIDE; // APIPermission overrides virtual PermissionMessages GetMessages() const OVERRIDE; diff --git a/chrome/common/extensions/permissions/media_galleries_permission.cc b/chrome/common/extensions/permissions/media_galleries_permission.cc index b95777b..f6d1689 100644 --- a/chrome/common/extensions/permissions/media_galleries_permission.cc +++ b/chrome/common/extensions/permissions/media_galleries_permission.cc @@ -17,11 +17,22 @@ 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; + if (error) + *error = "copyTo permission requires read and delete permissions"; + return false; + } + if (has_delete) { + if (has_read) + return true; + if (error) + *error = "delete permission requires read permission"; + return false; + } return true; } @@ -44,9 +55,11 @@ MediaGalleriesPermission::MediaGalleriesPermission( 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 +91,7 @@ bool MediaGalleriesPermission::FromValue(const base::Value* value) { 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 +115,7 @@ PermissionMessages MediaGalleriesPermission::GetMessages() const { 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; } diff --git a/chrome/common/extensions/permissions/media_galleries_permission.h b/chrome/common/extensions/permissions/media_galleries_permission.h index e385a24..8fd1a5d 100644 --- a/chrome/common/extensions/permissions/media_galleries_permission.h +++ b/chrome/common/extensions/permissions/media_galleries_permission.h @@ -38,7 +38,7 @@ class MediaGalleriesPermission // SetDisjunctionPermission overrides. // MediaGalleriesPermission does additional checks to make sure the // permissions do not contain unknown values. - virtual bool FromValue(const base::Value* value) OVERRIDE; + virtual bool FromValue(const base::Value* value, std::string* error) OVERRIDE; // APIPermission overrides. virtual PermissionMessages GetMessages() const OVERRIDE; diff --git a/chrome/common/extensions/permissions/media_galleries_permission_unittest.cc b/chrome/common/extensions/permissions/media_galleries_permission_unittest.cc index ddcfc92..1006d3c 100644 --- a/chrome/common/extensions/permissions/media_galleries_permission_unittest.cc +++ b/chrome/common/extensions/permissions/media_galleries_permission_unittest.cc @@ -19,6 +19,7 @@ namespace extensions { namespace { TEST(MediaGalleriesPermissionTest, GoodValues) { + std::string error; const APIPermissionInfo* permission_info = PermissionsInfo::GetInstance()->GetByID(APIPermission::kMediaGalleries); @@ -29,41 +30,55 @@ TEST(MediaGalleriesPermissionTest, GoodValues) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); // all_detected value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); // access_type value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); // Repeats do not make a difference. value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); @@ -72,10 +87,13 @@ TEST(MediaGalleriesPermissionTest, GoodValues) { value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - EXPECT_TRUE(permission->FromValue(value.get())); + EXPECT_TRUE(permission->FromValue(value.get(), &error)); + EXPECT_TRUE(error.empty()); + error.clear(); } TEST(MediaGalleriesPermissionTest, BadValues) { + std::string error; const APIPermissionInfo* permission_info = PermissionsInfo::GetInstance()->GetByID(APIPermission::kMediaGalleries); @@ -83,35 +101,47 @@ TEST(MediaGalleriesPermissionTest, BadValues) { // Empty scoped_ptr<base::ListValue> value(new base::ListValue()); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); // copyTo and delete without read value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kCopyToPermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kDeletePermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); // copyTo without delete value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); // Repeats do not make a difference. value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); @@ -119,7 +149,9 @@ TEST(MediaGalleriesPermissionTest, BadValues) { value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - EXPECT_FALSE(permission->FromValue(value.get())); + EXPECT_FALSE(permission->FromValue(value.get(), &error)); + EXPECT_FALSE(error.empty()); + error.clear(); } TEST(MediaGalleriesPermissionTest, Equal) { @@ -134,43 +166,43 @@ TEST(MediaGalleriesPermissionTest, Equal) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); - ASSERT_TRUE(permission2->FromValue(value.get())); + ASSERT_TRUE(permission2->FromValue(value.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); - ASSERT_TRUE(permission2->FromValue(value.get())); + ASSERT_TRUE(permission2->FromValue(value.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - ASSERT_TRUE(permission2->FromValue(value.get())); + ASSERT_TRUE(permission2->FromValue(value.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - ASSERT_TRUE(permission2->FromValue(value.get())); + ASSERT_TRUE(permission2->FromValue(value.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); } @@ -186,14 +218,14 @@ TEST(MediaGalleriesPermissionTest, NotEqual) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); - ASSERT_TRUE(permission2->FromValue(value.get())); + ASSERT_TRUE(permission2->FromValue(value.get(), NULL)); EXPECT_FALSE(permission1->Equal(permission2.get())); } @@ -209,32 +241,32 @@ TEST(MediaGalleriesPermissionTest, ToFromValue) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kAllAutoDetectedPermission); value->AppendString(MediaGalleriesPermission::kReadPermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); scoped_ptr<base::Value> vtmp(permission1->ToValue()); ASSERT_TRUE(vtmp); - ASSERT_TRUE(permission2->FromValue(vtmp.get())); + ASSERT_TRUE(permission2->FromValue(vtmp.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); value->AppendString(MediaGalleriesPermission::kCopyToPermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); vtmp = permission1->ToValue(); ASSERT_TRUE(vtmp); - ASSERT_TRUE(permission2->FromValue(vtmp.get())); + ASSERT_TRUE(permission2->FromValue(vtmp.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); value.reset(new base::ListValue()); value->AppendString(MediaGalleriesPermission::kReadPermission); value->AppendString(MediaGalleriesPermission::kDeletePermission); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); vtmp = permission1->ToValue(); ASSERT_TRUE(vtmp); - ASSERT_TRUE(permission2->FromValue(vtmp.get())); + ASSERT_TRUE(permission2->FromValue(vtmp.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); } diff --git a/chrome/common/extensions/permissions/permission_set_unittest.cc b/chrome/common/extensions/permissions/permission_set_unittest.cc index 73d02ca..1156ce4 100644 --- a/chrome/common/extensions/permissions/permission_set_unittest.cc +++ b/chrome/common/extensions/permissions/permission_set_unittest.cc @@ -255,9 +255,7 @@ TEST(PermissionsTest, CreateUnion) { base::Value::CreateStringValue("tcp-connect:*.example.com:80")); value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } // Union with an empty set. @@ -302,9 +300,7 @@ TEST(PermissionsTest, CreateUnion) { value->Append( base::Value::CreateStringValue("tcp-connect:*.example.com:80")); value->Append(base::Value::CreateStringValue("udp-send-to::8899")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } apis2.insert(permission); @@ -321,9 +317,7 @@ TEST(PermissionsTest, CreateUnion) { value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); value->Append(base::Value::CreateStringValue("udp-send-to::8899")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } // Insert a new permission socket permisssion which will replace the old one. expected_apis.insert(permission); @@ -390,9 +384,7 @@ TEST(PermissionsTest, CreateIntersection) { base::Value::CreateStringValue("tcp-connect:*.example.com:80")); value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } apis1.insert(permission); @@ -429,9 +421,7 @@ TEST(PermissionsTest, CreateIntersection) { value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); value->Append(base::Value::CreateStringValue("udp-send-to::8899")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } apis2.insert(permission); @@ -441,9 +431,7 @@ TEST(PermissionsTest, CreateIntersection) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } expected_apis.insert(permission); @@ -509,9 +497,7 @@ TEST(PermissionsTest, CreateDifference) { base::Value::CreateStringValue("tcp-connect:*.example.com:80")); value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } apis1.insert(permission); @@ -536,9 +522,7 @@ TEST(PermissionsTest, CreateDifference) { value->Append( base::Value::CreateStringValue("tcp-connect:*.example.com:80")); value->Append(base::Value::CreateStringValue("udp-send-to::8899")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } apis2.insert(permission); @@ -548,9 +532,7 @@ TEST(PermissionsTest, CreateDifference) { scoped_ptr<base::ListValue> value(new base::ListValue()); value->Append(base::Value::CreateStringValue("udp-bind::8080")); value->Append(base::Value::CreateStringValue("udp-send-to::8888")); - if (!permission->FromValue(value.get())) { - NOTREACHED(); - } + ASSERT_TRUE(permission->FromValue(value.get(), NULL)); } expected_apis.insert(permission); diff --git a/chrome/common/extensions/permissions/set_disjunction_permission.h b/chrome/common/extensions/permissions/set_disjunction_permission.h index 97388ba..f018e13 100644 --- a/chrome/common/extensions/permissions/set_disjunction_permission.h +++ b/chrome/common/extensions/permissions/set_disjunction_permission.h @@ -110,24 +110,29 @@ class SetDisjunctionPermission : public APIPermission { return result->data_set_.empty() ? NULL : result.release(); } - virtual bool FromValue(const base::Value* value) OVERRIDE { + virtual bool FromValue(const base::Value* value, + std::string* error) OVERRIDE { data_set_.clear(); const base::ListValue* list = NULL; - if (!value) - return false; - - if (!value->GetAsList(&list) || list->GetSize() == 0) + if (!value || !value->GetAsList(&list) || list->GetSize() == 0) { + if (error) + *error = "NULL or empty permission list"; return false; + } for (size_t i = 0; i < list->GetSize(); ++i) { - const base::Value* item_value; - if (!list->Get(i, &item_value)) - return false; + const base::Value* item_value = NULL; + bool got_item = list->Get(i, &item_value); + DCHECK(got_item); + DCHECK(item_value); PermissionDataType data; - if (!data.FromValue(item_value)) + if (!data.FromValue(item_value)) { + if (error) + *error = "Cannot parse an item from the permission list"; return false; + } data_set_.insert(data); } diff --git a/chrome/common/extensions/permissions/settings_override_permission.cc b/chrome/common/extensions/permissions/settings_override_permission.cc index f5bf946..934bf98 100644 --- a/chrome/common/extensions/permissions/settings_override_permission.cc +++ b/chrome/common/extensions/permissions/settings_override_permission.cc @@ -8,7 +8,6 @@ #include "grit/generated_resources.h" #include "ui/base/l10n/l10n_util.h" - namespace extensions { SettingsOverrideAPIPermission::SettingsOverrideAPIPermission( @@ -53,25 +52,23 @@ PermissionMessages SettingsOverrideAPIPermission::GetMessages() const { bool SettingsOverrideAPIPermission::Check( const APIPermission::CheckParam* param) const { - return !param; + return (param == NULL); } bool SettingsOverrideAPIPermission::Contains(const APIPermission* rhs) const { - CHECK(info() == rhs->info()); + CHECK_EQ(info(), rhs->info()); return true; } bool SettingsOverrideAPIPermission::Equal(const APIPermission* rhs) const { - if (this == rhs) - return true; - CHECK(info() == rhs->info()); + if (this != rhs) + CHECK_EQ(info(), rhs->info()); return true; } -bool SettingsOverrideAPIPermission::FromValue(const base::Value* value) { - if (value) - return false; - return true; +bool SettingsOverrideAPIPermission::FromValue(const base::Value* value, + std::string* /*error*/) { + return (value == NULL); } scoped_ptr<base::Value> SettingsOverrideAPIPermission::ToValue() const { @@ -84,19 +81,19 @@ APIPermission* SettingsOverrideAPIPermission::Clone() const { APIPermission* SettingsOverrideAPIPermission::Diff( const APIPermission* rhs) const { - CHECK(info() == rhs->info()); + CHECK_EQ(info(), rhs->info()); return NULL; } APIPermission* SettingsOverrideAPIPermission::Union( const APIPermission* rhs) const { - CHECK(info() == rhs->info()); + CHECK_EQ(info(), rhs->info()); return new SettingsOverrideAPIPermission(info(), setting_value_); } APIPermission* SettingsOverrideAPIPermission::Intersect( const APIPermission* rhs) const { - CHECK(info() == rhs->info()); + CHECK_EQ(info(), rhs->info()); return new SettingsOverrideAPIPermission(info(), setting_value_); } diff --git a/chrome/common/extensions/permissions/settings_override_permission.h b/chrome/common/extensions/permissions/settings_override_permission.h index 8f8a4e3..d123f83 100644 --- a/chrome/common/extensions/permissions/settings_override_permission.h +++ b/chrome/common/extensions/permissions/settings_override_permission.h @@ -25,7 +25,7 @@ class SettingsOverrideAPIPermission : public APIPermission { virtual bool Check(const APIPermission::CheckParam* param) const OVERRIDE; virtual bool Contains(const APIPermission* rhs) const OVERRIDE; virtual bool Equal(const APIPermission* rhs) const OVERRIDE; - virtual bool FromValue(const base::Value* value) OVERRIDE; + virtual bool FromValue(const base::Value* value, std::string* error) OVERRIDE; virtual scoped_ptr<base::Value> ToValue() const OVERRIDE; virtual APIPermission* Clone() const OVERRIDE; virtual APIPermission* Diff(const APIPermission* rhs) const OVERRIDE; diff --git a/chrome/common/extensions/permissions/socket_permission_unittest.cc b/chrome/common/extensions/permissions/socket_permission_unittest.cc index 78bd818..a968c46 100644 --- a/chrome/common/extensions/permissions/socket_permission_unittest.cc +++ b/chrome/common/extensions/permissions/socket_permission_unittest.cc @@ -293,7 +293,7 @@ TEST(SocketPermissionTest, IPC) { value->AppendString("tcp-connect:*.example.com:80"); value->AppendString("udp-bind::8080"); value->AppendString("udp-send-to::8888"); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); EXPECT_FALSE(permission1->Equal(permission2.get())); @@ -317,13 +317,13 @@ TEST(SocketPermissionTest, Value) { value->AppendString("tcp-connect:*.example.com:80"); value->AppendString("udp-bind::8080"); value->AppendString("udp-send-to::8888"); - ASSERT_TRUE(permission1->FromValue(value.get())); + ASSERT_TRUE(permission1->FromValue(value.get(), NULL)); EXPECT_FALSE(permission1->Equal(permission2.get())); scoped_ptr<base::Value> vtmp(permission1->ToValue()); ASSERT_TRUE(vtmp); - ASSERT_TRUE(permission2->FromValue(vtmp.get())); + ASSERT_TRUE(permission2->FromValue(vtmp.get(), NULL)); EXPECT_TRUE(permission1->Equal(permission2.get())); } diff --git a/chrome/common/extensions/permissions/usb_device_permission_unittest.cc b/chrome/common/extensions/permissions/usb_device_permission_unittest.cc index bb3ea70e..2aa47f1 100644 --- a/chrome/common/extensions/permissions/usb_device_permission_unittest.cc +++ b/chrome/common/extensions/permissions/usb_device_permission_unittest.cc @@ -51,7 +51,7 @@ TEST(USBDevicePermissionTest, MAYBE_PermissionMessage) { UsbDevicePermission permission( PermissionsInfo::GetInstance()->GetByID(APIPermission::kUsbDevice)); - ASSERT_TRUE(permission.FromValue(permission_list.get())); + ASSERT_TRUE(permission.FromValue(permission_list.get(), NULL)); PermissionMessages messages = permission.GetMessages(); ASSERT_EQ(3U, messages.size()); |