Implement getter/setter-based bound properties for CppBoundClass.

This is necessary for AccessibilityController, which has non-trivial
property accessors.

R=darin
BUG=10322
TEST=CppBoundClassTest.SetAndGetPropertiesWithCallbacks

Review URL: http://codereview.chromium.org/243064

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@28872 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 184 insertions, 27 deletions

diff --git a/webkit/glue/cpp_binding_example.cc b/webkit/glue/cpp_binding_example.cc
index b71ffd3..79e96e4 100644
--- a/webkit/glue/cpp_binding_example.cc
+++ b/webkit/glue/cpp_binding_example.cc
@@ -2,11 +2,31 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// This file contains the definition for CppBindingExample, a usage example
-// that is not actually used anywhere.  See cpp_binding_example.h.
+// This file contains the definition for CppBindingExample, which is used in
+// cpp_bound_class_unittest.
 
 #include "cpp_binding_example.h"
 
+namespace {
+
+class PropertyCallbackExample : public CppBoundClass::PropertyCallback {
+ public:
+  virtual bool GetValue(CppVariant* value) {
+    value->Set(value_);
+    return true;
+  }
+
+  virtual bool SetValue(const CppVariant& value) {
+    value_.Set(value);
+    return true;
+  }
+
+ private:
+  CppVariant value_;
+};
+
+}
+
 CppBindingExample::CppBindingExample() {
   // Map properties.  It's recommended, but not required, that the JavaScript
   // names (used as the keys in this map) match the names of the member
@@ -14,6 +34,11 @@ CppBindingExample::CppBindingExample() {
   BindProperty("my_value", &my_value);
   BindProperty("my_other_value", &my_other_value);
 
+  // Bind property with a callback.
+  BindProperty("my_value_with_callback", new PropertyCallbackExample());
+  // Bind property with a getter callback.
+  BindProperty("same", &CppBindingExample::same);
+
   // Map methods.  See comment above about names.
   BindMethod("echoValue", &CppBindingExample::echoValue);
   BindMethod("echoType",  &CppBindingExample::echoType);
@@ -88,6 +113,10 @@ void CppBindingExample::plus(const CppArgumentList& args,
   result->Set(sum);
 }
 
+void CppBindingExample::same(CppVariant* result) {
+  result->Set(42);
+}
+
 void CppBindingExample::fallbackMethod(const CppArgumentList& args,
                                        CppVariant* result) {
   printf("Error: unknown JavaScript method invoked.\n");
diff --git a/webkit/glue/cpp_binding_example.h b/webkit/glue/cpp_binding_example.h
index c89e814..a663682 100644
--- a/webkit/glue/cpp_binding_example.h
+++ b/webkit/glue/cpp_binding_example.h
@@ -63,6 +63,9 @@ class CppBindingExample : public CppBoundClass {
   // numbers (integers or doubles).  Otherwise returns null.
   void plus(const CppArgumentList& args, CppVariant* result);
 
+  // Always returns the same value -- an example of a read-only property.
+  void same(CppVariant* result);
+
   // Invoked when a nonexistent method is called on this example object, this
   // prints an error message.
   void fallbackMethod(const CppArgumentList& args, CppVariant* result);
diff --git a/webkit/glue/cpp_bound_class.cc b/webkit/glue/cpp_bound_class.cc
index 3e5c1e7..f82b78e 100644
--- a/webkit/glue/cpp_bound_class.cc
+++ b/webkit/glue/cpp_bound_class.cc
@@ -23,6 +23,45 @@
 using WebKit::WebBindings;
 using WebKit::WebFrame;
 
+namespace {
+
+class CppVariantPropertyCallback : public CppBoundClass::PropertyCallback {
+ public:
+  CppVariantPropertyCallback(CppVariant* value) : value_(value) { }
+
+  virtual bool GetValue(CppVariant* value) {
+    value->Set(*value_);
+    return true;
+  }
+  virtual bool SetValue(const CppVariant& value) {
+    value_->Set(value);
+    return true;
+  }
+
+ private:
+  CppVariant* value_;
+};
+
+class GetterPropertyCallback : public CppBoundClass::PropertyCallback {
+public:
+  GetterPropertyCallback(CppBoundClass::GetterCallback* callback)
+      : callback_(callback) { }
+
+  virtual bool GetValue(CppVariant* value) {
+    callback_->Run(value);
+    return true;
+  }
+
+  virtual bool SetValue(const CppVariant& value) {
+    return false;
+  }
+
+private:
+  scoped_ptr<CppBoundClass::GetterCallback> callback_;
+};
+
+}
+
 // Our special NPObject type.  We extend an NPObject with a pointer to a
 // CppBoundClass, which is just a C++ interface that we forward all NPObject
 // callbacks to.
@@ -138,6 +177,11 @@ CppBoundClass::~CppBoundClass() {
   for (MethodList::iterator i = methods_.begin(); i != methods_.end(); ++i)
     delete i->second;
 
+  for (PropertyList::iterator i = properties_.begin(); i != properties_.end();
+      ++i) {
+    delete i->second;
+  }
+
   // Unregister ourselves if we were bound to a frame.
   if (bound_to_frame_)
     WebBindings::unregisterObject(NPVARIANT_TO_OBJECT(self_variant_));
@@ -181,45 +225,76 @@ bool CppBoundClass::Invoke(NPIdentifier ident,
 }
 
 bool CppBoundClass::GetProperty(NPIdentifier ident, NPVariant* result) const {
-  PropertyList::const_iterator prop = properties_.find(ident);
-  if (prop == properties_.end()) {
+  PropertyList::const_iterator callback = properties_.find(ident);
+  if (callback == properties_.end()) {
     VOID_TO_NPVARIANT(*result);
     return false;
   }
 
-  const CppVariant* cpp_value = (*prop).second;
-  cpp_value->CopyToNPVariant(result);
+  CppVariant cpp_value;
+  if (!callback->second->GetValue(&cpp_value))
+    return false;
+  cpp_value.CopyToNPVariant(result);
   return true;
 }
 
 bool CppBoundClass::SetProperty(NPIdentifier ident,
-                                   const NPVariant* value) {
-  PropertyList::iterator prop = properties_.find(ident);
-  if (prop == properties_.end())
+                                const NPVariant* value) {
+  PropertyList::iterator callback = properties_.find(ident);
+  if (callback == properties_.end())
     return false;
 
-  (*prop).second->Set(*value);
-  return true;
+  CppVariant cpp_value;
+  cpp_value.Set(*value);
+  return (*callback).second->SetValue(cpp_value);
 }
 
-void CppBoundClass::BindCallback(std::string name, Callback* callback) {
-  // NPUTF8 is a typedef for char, so this cast is safe.
-  NPIdentifier ident = WebBindings::getStringIdentifier((const NPUTF8*)name.c_str());
+void CppBoundClass::BindCallback(const std::string& name, Callback* callback) {
+  NPIdentifier ident = WebBindings::getStringIdentifier(name.c_str());
   MethodList::iterator old_callback = methods_.find(ident);
-  if (old_callback != methods_.end())
+  if (old_callback != methods_.end()) {
     delete old_callback->second;
+    if (callback == NULL) {
+      methods_.erase(old_callback);
+      return;
+    }
+  }
+
   methods_[ident] = callback;
 }
 
-void CppBoundClass::BindProperty(std::string name, CppVariant* prop) {
-  // NPUTF8 is a typedef for char, so this cast is safe.
-  NPIdentifier ident = WebBindings::getStringIdentifier((const NPUTF8*)name.c_str());
-  properties_[ident] = prop;
+void CppBoundClass::BindGetterCallback(const std::string& name,
+                                       GetterCallback* callback) {
+  PropertyCallback* property_callback = callback == NULL ?
+      NULL : new GetterPropertyCallback(callback);
+
+  BindProperty(name, property_callback);
+}
+
+void CppBoundClass::BindProperty(const std::string& name, CppVariant* prop) {
+  PropertyCallback* property_callback = prop == NULL ?
+      NULL : new CppVariantPropertyCallback(prop);
+
+  BindProperty(name, property_callback);
+}
+
+void CppBoundClass::BindProperty(const std::string& name,
+                                 PropertyCallback* callback) {
+  NPIdentifier ident = WebBindings::getStringIdentifier(name.c_str());
+  PropertyList::iterator old_callback = properties_.find(ident);
+  if (old_callback != properties_.end()) {
+    delete old_callback->second;
+    if (callback == NULL) {
+      properties_.erase(old_callback);
+      return;
+    }
+  }
+
+  properties_[ident] = callback;
 }
 
-bool CppBoundClass::IsMethodRegistered(std::string name) const {
-  // NPUTF8 is a typedef for char, so this cast is safe.
-  NPIdentifier ident = WebBindings::getStringIdentifier((const NPUTF8*)name.c_str());
+bool CppBoundClass::IsMethodRegistered(const std::string& name) const {
+  NPIdentifier ident = WebBindings::getStringIdentifier(name.c_str());
   MethodList::const_iterator callback = methods_.find(ident);
   return (callback != methods_.end());
 }
diff --git a/webkit/glue/cpp_bound_class.h b/webkit/glue/cpp_bound_class.h
index 7db5f3b..afa4b8a 100644
--- a/webkit/glue/cpp_bound_class.h
+++ b/webkit/glue/cpp_bound_class.h
@@ -36,6 +36,18 @@ typedef std::vector<CppVariant> CppArgumentList;
 // directly to C++ method calls and CppVariant* variable access.
 class CppBoundClass {
  public:
+  class PropertyCallback {
+   public:
+    virtual ~PropertyCallback() { }
+
+    // Sets |value| to the value of the property. Returns false in case of
+    // failure. |value| is always non-NULL.
+    virtual bool GetValue(CppVariant* value) = 0;
+
+    // sets the property value to |value|. Returns false in case of failure.
+    virtual bool SetValue(const CppVariant& value) = 0;
+  };
+
   // The constructor should call BindMethod, BindProperty, and
   // SetFallbackMethod as needed to set up the methods, properties, and
   // fallback method.
@@ -58,20 +70,21 @@ class CppBoundClass {
 
   // The type of callbacks.
   typedef Callback2<const CppArgumentList&, CppVariant*>::Type Callback;
+  typedef Callback1<CppVariant*>::Type GetterCallback;
 
   // Used by a test.  Returns true if a method with name |name| exists,
   // regardless of whether a fallback is registered.
-  bool IsMethodRegistered(std::string name) const;
+  bool IsMethodRegistered(const std::string& name) const;
 
  protected:
   // Bind the Javascript method called |name| to the C++ callback |callback|.
-  void BindCallback(std::string name, Callback* callback);
+  void BindCallback(const std::string& name, Callback* callback);
 
   // A wrapper for BindCallback, to simplify the common case of binding a
   // method on the current object.  Though not verified here, |method|
   // must be a method of this CppBoundClass subclass.
   template<typename T>
-  void BindMethod(std::string name,
+  void BindMethod(const std::string& name,
       void (T::*method)(const CppArgumentList&, CppVariant*)) {
     Callback* callback =
         NewCallback<T, const CppArgumentList&, CppVariant*>(
@@ -79,8 +92,26 @@ class CppBoundClass {
     BindCallback(name, callback);
   }
 
+  // Bind Javascript property |name| to the C++ getter callback |callback|.
+  // This can be used to create read-only properties.
+  void BindGetterCallback(const std::string& name, GetterCallback* callback);
+
+  // A wrapper for BindGetterCallback, to simplify the common case of binding a
+  // property on the current object.  Though not verified here, |method|
+  // must be a method of this CppBoundClass subclass.
+  template<typename T>
+  void BindProperty(const std::string& name, void (T::*method)(CppVariant*)) {
+    GetterCallback* callback =
+        NewCallback<T, CppVariant*>(static_cast<T*>(this), method);
+    BindGetterCallback(name, callback);
+  }
+
   // Bind the Javascript property called |name| to a CppVariant |prop|.
-  void BindProperty(std::string name, CppVariant* prop);
+  void BindProperty(const std::string& name, CppVariant* prop);
+
+  // Bind Javascript property called |name| to a PropertyCallback |callback|.
+  // CppBoundClass assumes control over the life time of the |callback|.
+  void BindProperty(const std::string& name, PropertyCallback* callback);
 
   // Set the fallback callback, which is called when when a callback is
   // invoked that isn't bound.
@@ -116,7 +147,7 @@ class CppBoundClass {
   // Some fields are protected because some tests depend on accessing them,
   // but otherwise they should be considered private.
 
-  typedef std::map<NPIdentifier, CppVariant*> PropertyList;
+  typedef std::map<NPIdentifier, PropertyCallback*> PropertyList;
   typedef std::map<NPIdentifier, Callback*> MethodList;
   // These maps associate names with property and method pointers to be
   // exposed to JavaScript.
diff --git a/webkit/glue/cpp_bound_class_unittest.cc b/webkit/glue/cpp_bound_class_unittest.cc
index a368e62..919ffc5 100644
--- a/webkit/glue/cpp_bound_class_unittest.cc
+++ b/webkit/glue/cpp_bound_class_unittest.cc
@@ -209,6 +209,25 @@ TEST_F(CppBoundClassTest, SetAndGetProperties) {
   }
 }
 
+TEST_F(CppBoundClassTest, SetAndGetPropertiesWithCallbacks) {
+  // TODO(dglazkov): fix NPObject issues around failing property setters and
+  // getters and add tests for situations when GetProperty or SetProperty fail.
+  std::string js = "var result = 'SUCCESS';\n"
+    "example.my_value_with_callback = 10;\n"
+    "if (example.my_value_with_callback != 10)\n"
+    "  result = 'FAIL: unable to set property.';\n"
+    "example.my_value_with_callback = 11;\n"
+    "if (example.my_value_with_callback != 11)\n"
+    "  result = 'FAIL: unable to set property again';\n"
+    "if (example.same != 42)\n"
+    "  result = 'FAIL: same property should always be 42';\n"
+    "example.same = 24;\n"
+    "if (example.same != 42)\n"
+    "  result = 'FAIL: same property should always be 42';\n"
+    "document.writeln(result);\n";
+  CheckJavaScriptSuccess(js);
+}
+
 TEST_F(CppBoundClassTest, InvokeMethods) {
   // The expression on the left is expected to return the value on the right.
   static const std::string tests[] = {