From 72bbaccaa082a3ad6fff9bbc1cf70d01dabd79e8 Mon Sep 17 00:00:00 2001 From: "keybuk@google.com" Date: Wed, 21 Mar 2012 23:43:45 +0000 Subject: dbus: clear array values before reading from variant PopArrayFromVariant() appends values to the existing value, rather than clearing first like I expected, so using this without clear() first means the property value accumulates all values and never loses them. BUG=none TEST=unit test included Change-Id: Ie392a89190f4ad8570a905f24b2f446e1f2bed81 R=satorux@chromium.org Review URL: https://chromiumcodereview.appspot.com/9809001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128108 0039d316-1c4b-4281-b951-d872f2087c98 --- dbus/property.cc | 2 ++ dbus/property.h | 7 +++--- dbus/property_unittest.cc | 49 +++++++++++++++++++++++++++++++++++++ dbus/test_service.cc | 61 ++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 108 insertions(+), 11 deletions(-) (limited to 'dbus') diff --git a/dbus/property.cc b/dbus/property.cc index 0d6e7f0..0abe001 100644 --- a/dbus/property.cc +++ b/dbus/property.cc @@ -382,6 +382,7 @@ bool Property >::PopValueFromReader( if (!reader->PopVariant(&variant_reader)) return false; + value_.clear(); return variant_reader.PopArrayOfStrings(&value_); } @@ -406,6 +407,7 @@ bool Property >::PopValueFromReader( if (!reader->PopVariant(&variant_reader)) return false; + value_.clear(); return variant_reader.PopArrayOfObjectPaths(&value_); } diff --git a/dbus/property.h b/dbus/property.h index d46bd8d..2d2cc82 100644 --- a/dbus/property.h +++ b/dbus/property.h @@ -386,9 +386,10 @@ class Property : public PropertyBase { callback.Run(response); } - // Updates the cached property value by popping from |reader| which - // should be positioned at the property value, generally of variant - // type. Implementation provided by specialization. + // Updates the cached property value, replacing any previous value + // entirely, by popping from |reader| which should be positioned at the + // property value, generally of variant type. + // Implementation provided by specialization. virtual bool PopValueFromReader(MessageReader* reader); // Appends the passed |value| to |writer|, generally as a variant type. diff --git a/dbus/property_unittest.cc b/dbus/property_unittest.cc index effb751..71ce533 100644 --- a/dbus/property_unittest.cc +++ b/dbus/property_unittest.cc @@ -167,6 +167,55 @@ TEST_F(PropertyTest, InitialValues) { EXPECT_EQ(dbus::ObjectPath("/TestObjectPath"), objects[0]); } +TEST_F(PropertyTest, UpdatedValues) { + WaitForGetAll(); + + // Update the value of the "Name" property, this value should not change. + properties_->name.Get(base::Bind(&PropertyTest::PropertyCallback, + base::Unretained(this), + "Name")); + WaitForCallback("Name"); + WaitForUpdates(1); + + EXPECT_EQ(properties_->name.value(), "TestService"); + + // Update the value of the "Version" property, this value should be changed. + properties_->version.Get(base::Bind(&PropertyTest::PropertyCallback, + base::Unretained(this), + "Version")); + WaitForCallback("Version"); + WaitForUpdates(1); + + EXPECT_EQ(properties_->version.value(), 20); + + // Update the value of the "Methods" property, this value should not change + // and should not grow to contain duplicate entries. + properties_->methods.Get(base::Bind(&PropertyTest::PropertyCallback, + base::Unretained(this), + "Methods")); + WaitForCallback("Methods"); + WaitForUpdates(1); + + std::vector methods = properties_->methods.value(); + ASSERT_EQ(4U, methods.size()); + EXPECT_EQ("Echo", methods[0]); + EXPECT_EQ("SlowEcho", methods[1]); + EXPECT_EQ("AsyncEcho", methods[2]); + EXPECT_EQ("BrokenMethod", methods[3]); + + // Update the value of the "Objects" property, this value should not change + // and should not grow to contain duplicate entries. + properties_->objects.Get(base::Bind(&PropertyTest::PropertyCallback, + base::Unretained(this), + "Objects")); + WaitForCallback("Objects"); + WaitForUpdates(1); + + std::vector objects = properties_->objects.value(); + ASSERT_EQ(1U, objects.size()); + EXPECT_EQ(dbus::ObjectPath("/TestObjectPath"), objects[0]); +} + TEST_F(PropertyTest, Get) { WaitForGetAll(); diff --git a/dbus/test_service.cc b/dbus/test_service.cc index 443ad9b..986ebd6 100644 --- a/dbus/test_service.cc +++ b/dbus/test_service.cc @@ -336,17 +336,62 @@ void TestService::GetProperty( return; } - if (name != "Version") { + if (name == "Name") { + // Return the previous value for the "Name" property: + // Variant<"TestService"> + Response* response = Response::FromMethodCall(method_call); + MessageWriter writer(response); + + writer.AppendVariantOfString("TestService"); + + response_sender.Run(response); + } else if (name == "Version") { + // Return a new value for the "Version" property: + // Variant<20> + Response* response = Response::FromMethodCall(method_call); + MessageWriter writer(response); + + writer.AppendVariantOfInt16(20); + + response_sender.Run(response); + } else if (name == "Methods") { + // Return the previous value for the "Methods" property: + // Variant<["Echo", "SlowEcho", "AsyncEcho", "BrokenMethod"]> + Response* response = Response::FromMethodCall(method_call); + MessageWriter writer(response); + MessageWriter variant_writer(NULL); + MessageWriter variant_array_writer(NULL); + + writer.OpenVariant("as", &variant_writer); + variant_writer.OpenArray("s", &variant_array_writer); + variant_array_writer.AppendString("Echo"); + variant_array_writer.AppendString("SlowEcho"); + variant_array_writer.AppendString("AsyncEcho"); + variant_array_writer.AppendString("BrokenMethod"); + variant_writer.CloseContainer(&variant_array_writer); + writer.CloseContainer(&variant_writer); + + response_sender.Run(response); + } else if (name == "Objects") { + // Return the previous value for the "Objects" property: + // Variant<[objectpath:"/TestObjectPath"]> + Response* response = Response::FromMethodCall(method_call); + MessageWriter writer(response); + MessageWriter variant_writer(NULL); + MessageWriter variant_array_writer(NULL); + + writer.OpenVariant("ao", &variant_writer); + variant_writer.OpenArray("o", &variant_array_writer); + variant_array_writer.AppendObjectPath(dbus::ObjectPath("/TestObjectPath")); + variant_writer.CloseContainer(&variant_array_writer); + writer.CloseContainer(&variant_writer); + + response_sender.Run(response); + } else { + // Return error. response_sender.Run(NULL); return; } - - Response* response = Response::FromMethodCall(method_call); - MessageWriter writer(response); - - writer.AppendVariantOfInt16(20); - - response_sender.Run(response); } void TestService::SetProperty( -- cgit v1.1