Remove raw DictionaryValue::Set in //components

base/test/values_test_util.h

Index: base/test/values_test_util.h
diff --git a/base/test/values_test_util.h b/base/test/values_test_util.h
index e545660fbed318069b0a8dc834b2e54b33b6e37d..c1ef9a40fdb76d4e472ece577d7b6cd9125861f9 100644
--- a/base/test/values_test_util.h
+++ b/base/test/values_test_util.h
@@ -38,8 +38,8 @@ void ExpectDictStringValue(const std::string& expected_value,
                            const DictionaryValue& value,
                            const std::string& key);
 
-// Takes ownership of |actual|.
-void ExpectStringValue(const std::string& expected_str, Value* actual);
+void ExpectStringValue(const std::string& expected_str,
+                       std::unique_ptr<Value> actual);

[brettw, 2017/05/19 20:31:02]

It seems weird to me that such a function would take ownership of the value to
compare against.

Before we used unique_ptr and had to heap-allocate all Values, this may have
made sense so people wouldn't forget to free the values.

But now can we pass a const Ref here? This has the advantage of making the
callsites shorter too, since you can do *foo instead of std::move(foo).

[jdoerrie, 2017/05/22 08:15:59]

Good point, I changed it now. 

Shorter callsites is unfortunately not true currently, because all callsites are
of form |ExpectStringValue("my_string", MyFun())|, meaning there is no move we
can drop. Updating all MyFuns to return Values directly is currently blocked on
having DictionaryValue::Set(StringPiece, Value) or equivalent. However, once we
do have that, we can return Values in a lot of places directly and can fix the
MyFuns.

 
 namespace test {