[Chromecast] Capabilities do not get removed when Validators unregister.

Also removing capability value initialization from Register().
This is done separately now.

The unit tests were checking the internal dictionary to test the
result of capability changes. This is incorrect. They have
been changed to use the public GetCapability() accessor.

Also adding a GetValidator() method that is currently used
in unit tests but also can be useful in future for checking if
a Validator is currently registered for a capability.

TEST=cast_base_unittests
BUG=

Review URL: https://codereview.chromium.org/1401993002

Cr-Commit-Position: refs/heads/master@{#353945}

chromecast/base/device_capabilities.h

@@ -104,26 +104,30 @@ class DeviceCapabilities {
 
   // Registers a Validator for a capability. A given key must only be
   // registered once, and must be unregistered before calling Register() again.
-  // The capability also gets added to the class with an initial value passed
-  // in. If the capability has a value of Dictionary type, |key| must be just
+  // If the capability has a value of Dictionary type, |key| must be just
   // the capability's top-level key and not include path expansions to levels
   // farther down. For example, "foo" is a valid value for |key|, but "foo.bar"
   // is not. Note that if "foo.bar" is updated in SetCapability(), the
   // Validate() method for "foo"'s Validator will be called, with a |path| of
   // "foo.bar". Both Register() and Unregister() must be called on cast
   // receiver main thread; the Validator provided will also be called on cast
-  // receiver main thread.
+  // receiver main thread. Note that this method does not add or modify
+  // the capability. To do this, SetCapability() should be called, or
+  // Validators can call SetValidatedValue().
   virtual void Register(const std::string& key,
-                        scoped_ptr<base::Value> init_value,
                         Validator* validator) = 0;
-  // Unregisters Validator for |key| and removes capability. |validator|
-  // argument must match |validator| argument that was passed in to Register()
-  // for |key|.
+  // Unregisters Validator for |key|. |validator| argument must match
+  // |validator| argument that was passed in to Register() for |key|. Note that
+  // the capability and its value remain untouched.
   virtual void Unregister(const std::string& key,
                           const Validator* validator) = 0;
+  // Gets the Validator currently registered for |key|. Returns nullptr if
+  // no Validator is registered.
+  virtual Validator* GetValidator(const std::string& key) const = 0;
 
   // Accessors for default capabilities. Note that the capability must be added
-  // through Register() or SetCapability() before accessors are called.
+  // through SetCapability() or SetValidatedValue() (for Validators) before
+  // accessors are called.
   virtual bool BluetoothSupported() const = 0;
   virtual bool DisplaySupported() const = 0;
 

chromecast/base/device_capabilities_impl.cc

@@ -83,33 +83,31 @@ DeviceCapabilitiesImpl::~DeviceCapabilitiesImpl() {
 }
 
 void DeviceCapabilitiesImpl::Register(const std::string& key,
-                                      scoped_ptr<base::Value> init_value,
                                       Validator* validator) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(IsValidRegisterKey(key));
-  DCHECK(init_value.get());
   DCHECK(validator);
 
-  AddValidator(key, validator);
-
-  capabilities_->Set(key, init_value.Pass());
-  UpdateStrAndNotifyChanged(key);
+  bool added = validator_map_.insert(std::make_pair(key, validator)).second;
+  // Check that a validator has not already been registered for this key
+  DCHECK(added);
 }
 
 void DeviceCapabilitiesImpl::Unregister(const std::string& key,
                                         const Validator* validator) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  auto validator_it = validator_map_.find(key);
-  DCHECK(validator_it != validator_map_.end());
   // Check that validator being unregistered matches the original for |key|.
   // This prevents managers from accidentally unregistering incorrect
   // validators.
-  DCHECK_EQ(validator, validator_it->second);
-  validator_map_.erase(validator_it);
+  DCHECK_EQ(validator, GetValidator(key));
+  bool erased = validator_map_.erase(key);
+  DCHECK(erased);
+}
 
-  bool removed = capabilities_->Remove(key, nullptr);
-  DCHECK(removed);
-  UpdateStrAndNotifyChanged(key);
+DeviceCapabilities::Validator* DeviceCapabilitiesImpl::GetValidator(
+    const std::string& key) const {
+  auto validator_it = validator_map_.find(key);
+  return validator_it == validator_map_.end() ? nullptr : validator_it->second;
 }
 
 bool DeviceCapabilitiesImpl::BluetoothSupported() const {
@@ -208,14 +206,6 @@ void DeviceCapabilitiesImpl::SetValidatedValueInternal(
   UpdateStrAndNotifyChanged(path);
 }
 
-void DeviceCapabilitiesImpl::AddValidator(const std::string& key,
-                                          Validator* validator) {
-  DCHECK(validator);
-  bool added = validator_map_.insert(std::make_pair(key, validator)).second;
-  // Check that a validator has not already been registered for this key
-  DCHECK(added);
-}
-
 void DeviceCapabilitiesImpl::UpdateStrAndNotifyChanged(
     const std::string& path) {
   // Update capabilities string here since all updates to capabilities must

chromecast/base/device_capabilities_impl.h

@@ -17,10 +17,9 @@ class DeviceCapabilitiesImpl : public DeviceCapabilities {
   ~DeviceCapabilitiesImpl() override;
 
   // DeviceCapabilities implementation:
-  void Register(const std::string& key,
-                scoped_ptr<base::Value> init_value,
-                Validator* validator) override;
+  void Register(const std::string& key, Validator* validator) override;
   void Unregister(const std::string& key, const Validator* validator) override;
+  Validator* GetValidator(const std::string& key) const override;
   bool BluetoothSupported() const override;
   bool DisplaySupported() const override;
   bool GetCapability(const std::string& path,
@@ -48,8 +47,6 @@ class DeviceCapabilitiesImpl : public DeviceCapabilities {
 
   void SetValidatedValueInternal(const std::string& path,
                                  scoped_ptr<base::Value> new_value) override;
-
-  void AddValidator(const std::string& key, Validator* validator);
   void UpdateStrAndNotifyChanged(const std::string& path);
 
   scoped_ptr<base::DictionaryValue> capabilities_;