Inline StringValue into base::Value

chromeos/network/network_state_unittest.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 "chromeos/network/network_state.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
 
 #include "base/i18n/streaming_utf8_validator.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 namespace {
 
 // StringValue that skips the DCHECK in the constructor for valid UTF8.
 class TestStringValue : public base::Value {

[jdoerrie, 2016/12/16 09:54:37]

This class and these tests will break once we remove the virtual keyword from
base::Value. Should we consider removing the DCHECK in Value(StringPiece) to
make this derived class unnecessary?

[vabr (Chromium), 2016/12/19 13:00:56]

Not sure yet. The written contract on the current StringValue is that the input
should be UTF8. The networking code is breaking it if it makes it possible to be
otherwise. I'm not sure if it is possible, but I guess when they put effort into
it to validate the string, then it is.
Ideally, they would validate the string before putting it into the Value. May be
worth filing a bug for them.

Or perhaps Brett knows that Value getting unsanitised input from the
environment/users is a wide-spread practice in Chromium, and hence the contract
about UTF8 should be dropped?

  public:
   explicit TestStringValue(const std::string& in_value)
       : base::Value(Type::STRING), value_(in_value) {}
 
-  ~TestStringValue() override {}
+  ~TestStringValue() override {
+    // Ugly hack that prevents ~Value() from trying to destroy string_value_.
+    // TODO(crbug.com/646113): Clean this up when StringValue will be removed.
+    type_ = Type::NONE;
+  }
 
   // Overridden from Value:
   bool GetAsString(std::string* out_value) const override {
     if (out_value)
       *out_value = value_;
     return true;
   }
 
   TestStringValue* DeepCopy() const override {
     return new TestStringValue(value_);
@@ skipping 189 matching lines @@
   provider->SetStringWithoutPathExpansion(
       shill::kHostProperty, "third-party-vpn-provider-extension-id");
   EXPECT_TRUE(SetProperty(shill::kProviderProperty, std::move(provider)));
   SignalInitialPropertiesReceived();
   EXPECT_EQ(network_state_.vpn_provider_type(), shill::kProviderThirdPartyVpn);
   EXPECT_EQ(network_state_.third_party_vpn_provider_extension_id(),
             "third-party-vpn-provider-extension-id");
 }
 
 }  // namespace chromeos