browser-plugin: Refactor the code for binding methods on the plugin.

content/renderer/browser_plugin/browser_plugin_bindings.cc

Index: content/renderer/browser_plugin/browser_plugin_bindings.cc
diff --git a/content/renderer/browser_plugin/browser_plugin_bindings.cc b/content/renderer/browser_plugin/browser_plugin_bindings.cc
index afb0556a27f35540af3215421dd71dc304f61857..f121bb5a304df3e1f4cbdf89b2050979c33b27e8 100644
--- a/content/renderer/browser_plugin/browser_plugin_bindings.cc
+++ b/content/renderer/browser_plugin/browser_plugin_bindings.cc
@@ -37,77 +37,6 @@ namespace content {
 
 namespace {
 
-const char kAddEventListener[] = "addEventListener";
-const char kBackMethod[] = "back";
-const char kCanGoBack[] = "canGoBack";
-const char kCanGoForward[] = "canGoForward";
-const char kForwardMethod[] = "forward";
-const char kGetProcessId[] = "getProcessId";
-const char kGoMethod[] = "go";
-const char kPartitionAttribute[] = "partition";
-const char kReloadMethod[] = "reload";
-const char kRemoveEventListener[] = "removeEventListener";
-const char kSrcAttribute[] = "src";
-const char kStopMethod[] = "stop";
-const char kTerminateMethod[] = "terminate";
-
-BrowserPluginBindings* GetBindings(NPObject* object) {
-  return static_cast<BrowserPluginBindings::BrowserPluginNPObject*>(object)->
-      message_channel;
-}
-
-bool IdentifierIsAddEventListener(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kAddEventListener) == identifier;
-}
-
-bool IdentifierIsBackMethod(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kBackMethod) == identifier;
-}
-
-bool IdentifierIsCanGoBack(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kCanGoBack) == identifier;
-}
-
-bool IdentifierIsCanGoForward(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kCanGoForward) == identifier;
-}
-
-bool IdentifierIsForwardMethod(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kForwardMethod) == identifier;
-}
-
-bool IdentifierIsGetProcessID(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kGetProcessId) == identifier;
-}
-
-bool IdentifierIsGoMethod(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kGoMethod) == identifier;
-}
-
-bool IdentifierIsPartitionAttribute(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kPartitionAttribute) == identifier;
-}
-
-bool IdentifierIsReload(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kReloadMethod) == identifier;
-}
-
-bool IdentifierIsRemoveEventListener(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kRemoveEventListener) == identifier;
-}
-
-bool IdentifierIsSrcAttribute(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kSrcAttribute) == identifier;
-}
-
-bool IdentifierIsStop(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kStopMethod) == identifier;
-}
-
-bool IdentifierIsTerminate(NPIdentifier identifier) {
-  return WebBindings::getStringIdentifier(kTerminateMethod) == identifier;
-}
-
 int Int32FromNPVariant(const NPVariant& variant) {
   if (NPVARIANT_IS_INT32(variant))
     return NPVARIANT_TO_INT32(variant);
@@ -137,6 +66,33 @@ bool StringToNPVariant(const std::string &in, NPVariant *variant) {
   return true;
 }
 
+const char kAddEventListener[] = "addEventListener";
+const char kBackMethod[] = "back";
+const char kCanGoBack[] = "canGoBack";
+const char kCanGoForward[] = "canGoForward";
+const char kForwardMethod[] = "forward";
+const char kGetProcessId[] = "getProcessId";
+const char kGoMethod[] = "go";
+const char kPartitionAttribute[] = "partition";
+const char kReloadMethod[] = "reload";
+const char kRemoveEventListener[] = "removeEventListener";
+const char kSrcAttribute[] = "src";
+const char kStopMethod[] = "stop";
+const char kTerminateMethod[] = "terminate";
+
+BrowserPluginBindings* GetBindings(NPObject* object) {
+  return static_cast<BrowserPluginBindings::BrowserPluginNPObject*>(object)->
+      message_channel;
+}
+
+bool IdentifierIsPartitionAttribute(NPIdentifier identifier) {
+  return WebBindings::getStringIdentifier(kPartitionAttribute) == identifier;
+}
+
+bool IdentifierIsSrcAttribute(NPIdentifier identifier) {
+  return WebBindings::getStringIdentifier(kSrcAttribute) == identifier;
+}

[Charlie Reis, 2012/10/15 22:19:33]

nit: Why move these?  Nice that a lot of this was removed, but the move seems
like unnecessary code churn.

[sadrul, 2012/10/15 22:37:12]

Done.

+
 //------------------------------------------------------------------------------
 // Implementations of NPClass functions.  These are here to:
 // - Implement src attribute.
@@ -155,40 +111,11 @@ bool BrowserPluginBindingsHasMethod(NPObject* np_obj, NPIdentifier name) {
   if (!np_obj)
     return false;
 
-  if (IdentifierIsAddEventListener(name))
-    return true;
-
-  if (IdentifierIsBackMethod(name))
-    return true;
-
-  if (IdentifierIsCanGoBack(name))
-    return true;
-
-  if (IdentifierIsCanGoForward(name))
-    return true;
-
-  if (IdentifierIsForwardMethod(name))
-    return true;
-
-  if (IdentifierIsGetProcessID(name))
-    return true;
-
-  if (IdentifierIsGoMethod(name))
-    return true;
-
-  if (IdentifierIsReload(name))
-    return true;
-
-  if (IdentifierIsRemoveEventListener(name))
-    return true;
-
-  if (IdentifierIsStop(name))
-    return true;
-
-  if (IdentifierIsTerminate(name))
-    return true;
+  BrowserPluginBindings* bindings = GetBindings(np_obj);
+  if (!bindings)
+    return false;
 
-  return false;
+  return bindings->HasMethod(name);
 }
 
 bool BrowserPluginBindingsInvoke(NPObject* np_obj, NPIdentifier name,
@@ -201,90 +128,7 @@ bool BrowserPluginBindingsInvoke(NPObject* np_obj, NPIdentifier name,
   if (!bindings)
     return false;
 
-  if (IdentifierIsAddEventListener(name) && (arg_count == 2)) {
-    std::string event_name = StringFromNPVariant(args[0]);
-    if (event_name.empty())
-      return false;
-
-    v8::Local<v8::Value> value =
-        v8::Local<v8::Value>::New(WebBindings::toV8Value(&args[1]));
-    if (value.IsEmpty() || !value->IsFunction())
-      return false;
-
-    v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(value);
-    result->type = NPVariantType_Bool;
-    result->value.boolValue =
-        bindings->instance()->AddEventListener(event_name, function);
-    return true;
-  }
-
-  if (IdentifierIsBackMethod(name) && !arg_count) {
-    bindings->instance()->Back();
-    return true;
-  }
-
-  if (IdentifierIsCanGoBack(name) && !arg_count) {
-    result->type = NPVariantType_Bool;
-    result->value.boolValue = bindings->instance()->CanGoBack();
-    return true;
-  }
-
-  if (IdentifierIsCanGoForward(name) && !arg_count) {
-    result->type = NPVariantType_Bool;
-    result->value.boolValue = bindings->instance()->CanGoForward();
-    return true;
-  }
-
-  if (IdentifierIsForwardMethod(name) && !arg_count) {
-    bindings->instance()->Forward();
-    return true;
-  }
-
-  if (IdentifierIsGetProcessID(name) && !arg_count) {
-    int process_id = bindings->instance()->process_id();
-    result->type = NPVariantType_Int32;
-    result->value.intValue = process_id;
-    return true;
-  }
-
-  if (IdentifierIsGoMethod(name) && arg_count == 1) {
-    bindings->instance()->Go(Int32FromNPVariant(args[0]));
-    return true;
-  }
-
-  if (IdentifierIsReload(name) && !arg_count) {
-    bindings->instance()->Reload();
-    return true;
-  }
-
-  if (IdentifierIsRemoveEventListener(name) && arg_count == 2) {
-    std::string event_name = StringFromNPVariant(args[0]);
-    if (event_name.empty())
-      return false;
-
-    v8::Local<v8::Value> value =
-        v8::Local<v8::Value>::New(WebBindings::toV8Value(&args[1]));
-
-    if (value.IsEmpty() || !value->IsFunction())
-      return false;
-
-    v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(value);
-    result->type = NPVariantType_Bool;
-    result->value.boolValue =
-        bindings->instance()->RemoveEventListener(event_name, function);
-    return true;
-  }
-
-  if (IdentifierIsStop(name) && !arg_count) {
-    bindings->instance()->Stop();
-    return true;
-  }
-
-  if (IdentifierIsTerminate(name)) {
-    bindings->instance()->TerminateGuest();
-  }
-
-  return false;
+  return bindings->InvokeMethod(name, args, arg_count, result);
 }
 
 bool BrowserPluginBindingsInvokeDefault(NPObject* np_obj,
@@ -384,6 +228,250 @@ NPClass browser_plugin_message_class = {
 
 }  // namespace
 
+namespace internal {
+
+class BrowserPluginBindingInterface {

[Charlie Reis, 2012/10/15 22:19:33]

nit: Interface suggests a class with only pure virtual methods.  Maybe
BrowserPluginMethodBinding, since they all appear to be methods?

[sadrul, 2012/10/15 22:37:12]

Good point. Done.

+ public:
+  BrowserPluginBindingInterface(const char name[],
+                                uint32 arg_count)
+      : name_(name),
+        arg_count_(arg_count) {
+  }
+
+  virtual ~BrowserPluginBindingInterface() {}
+
+  bool MatchesName(NPIdentifier name) const {
+    return WebBindings::getStringIdentifier(name_.c_str()) == name;
+  }
+
+  uint32 arg_count() const { return arg_count_; }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,

[Charlie Reis, 2012/10/15 22:19:33]

Why past tense?  Invoke seems clearer to me.

[sadrul, 2012/10/15 22:37:12]

Done.

+                       const NPVariant* args,
+                       NPVariant* result) = 0;
+
+ private:
+  std::string name_;
+  uint32 arg_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingInterface);
+};
+
+class BrowserPluginBindingAddListener : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingAddListener()
+      : BrowserPluginBindingInterface(kAddEventListener, 2) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    std::string event_name = StringFromNPVariant(args[0]);
+    if (event_name.empty())
+      return false;
+
+    v8::Local<v8::Value> value =
+        v8::Local<v8::Value>::New(WebBindings::toV8Value(&args[1]));
+    if (value.IsEmpty() || !value->IsFunction())
+      return false;
+
+    v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(value);
+    BOOLEAN_TO_NPVARIANT(
+        bindings->instance()->AddEventListener(event_name, function), *result);
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingAddListener);
+};
+
+class BrowserPluginBindingBack : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingBack()
+      : BrowserPluginBindingInterface(kBackMethod, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->Back();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingBack);
+};
+
+class BrowserPluginBindingCanGoBack : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingCanGoBack()
+      : BrowserPluginBindingInterface(kCanGoBack, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    BOOLEAN_TO_NPVARIANT(bindings->instance()->CanGoBack(), *result);
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingCanGoBack);
+};
+
+class BrowserPluginBindingCanGoForward : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingCanGoForward()
+      : BrowserPluginBindingInterface(kCanGoForward, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    BOOLEAN_TO_NPVARIANT(bindings->instance()->CanGoForward(), *result);
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingCanGoForward);
+};
+
+class BrowserPluginBindingForward : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingForward()
+      : BrowserPluginBindingInterface(kForwardMethod, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->Forward();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingForward);
+};
+
+class BrowserPluginBindingGetProcessID : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingGetProcessID()
+      : BrowserPluginBindingInterface(kGetProcessId, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    int process_id = bindings->instance()->process_id();
+    INT32_TO_NPVARIANT(process_id, *result);
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingGetProcessID);
+};
+
+class BrowserPluginBindingGo : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingGo()
+      : BrowserPluginBindingInterface(kGoMethod, 1) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->Go(Int32FromNPVariant(args[0]));
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingGo);
+};
+
+class BrowserPluginBindingReload : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingReload()
+      : BrowserPluginBindingInterface(kReloadMethod, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->Reload();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingReload);
+};
+
+class BrowserPluginBindingRemoveListener
+    : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingRemoveListener()
+      : BrowserPluginBindingInterface(kRemoveEventListener, 2) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    std::string event_name = StringFromNPVariant(args[0]);
+    if (event_name.empty())
+      return false;
+
+    v8::Local<v8::Value> value =
+        v8::Local<v8::Value>::New(WebBindings::toV8Value(&args[1]));
+
+    if (value.IsEmpty() || !value->IsFunction())
+      return false;
+
+    v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(value);
+    BOOLEAN_TO_NPVARIANT(
+        bindings->instance()->RemoveEventListener(event_name, function),
+        *result);
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingRemoveListener);
+};
+
+class BrowserPluginBindingStop : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingStop()
+      : BrowserPluginBindingInterface(kStopMethod, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->Stop();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingStop);
+};
+
+class BrowserPluginBindingTerminate : public BrowserPluginBindingInterface {
+ public:
+  BrowserPluginBindingTerminate()
+      : BrowserPluginBindingInterface(kTerminateMethod, 0) {
+  }
+
+  virtual bool Invoked(BrowserPluginBindings* bindings,
+                       const NPVariant* args,
+                       NPVariant* result) OVERRIDE {
+    bindings->instance()->TerminateGuest();
+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(BrowserPluginBindingTerminate);
+};
+
+}  // namespace internal
+
 // BrowserPluginBindings ------------------------------------------------------
 
 BrowserPluginBindings::BrowserPluginNPObject::BrowserPluginNPObject() {
@@ -400,10 +488,45 @@ BrowserPluginBindings::BrowserPluginBindings(BrowserPlugin* instance)
       WebBindings::createObject(NULL, &browser_plugin_message_class);
   np_object_ = static_cast<BrowserPluginBindings::BrowserPluginNPObject*>(obj);
   np_object_->message_channel = weak_ptr_factory_.GetWeakPtr();
+
+  method_bindings_.push_back(new internal::BrowserPluginBindingAddListener);
+  method_bindings_.push_back(new internal::BrowserPluginBindingBack);
+  method_bindings_.push_back(new internal::BrowserPluginBindingCanGoBack);
+  method_bindings_.push_back(new internal::BrowserPluginBindingCanGoForward);
+  method_bindings_.push_back(new internal::BrowserPluginBindingForward);
+  method_bindings_.push_back(new internal::BrowserPluginBindingGetProcessID);
+  method_bindings_.push_back(new internal::BrowserPluginBindingGo);
+  method_bindings_.push_back(new internal::BrowserPluginBindingReload);
+  method_bindings_.push_back(new internal::BrowserPluginBindingRemoveListener);
+  method_bindings_.push_back(new internal::BrowserPluginBindingStop);
+  method_bindings_.push_back(new internal::BrowserPluginBindingTerminate);
 }
 
 BrowserPluginBindings::~BrowserPluginBindings() {
   WebBindings::releaseObject(np_object_);
 }
 
+bool BrowserPluginBindings::HasMethod(NPIdentifier name) const {
+  for (BindingList::const_iterator iter = method_bindings_.begin();
+      iter != method_bindings_.end();

[Charlie Reis, 2012/10/15 22:19:33]

nit: wrong indent.  (Should have one more space, to line up with BindingsList.)
Same below in InvokeMethod.

[sadrul, 2012/10/15 22:37:12]

Done.

+      ++iter) {
+    if ((*iter)->MatchesName(name))
+      return true;
+  }
+  return false;
+}
+
+bool BrowserPluginBindings::InvokeMethod(NPIdentifier name,
+                                         const NPVariant* args,
+                                         uint32 arg_count,
+                                         NPVariant* result) {
+  for (BindingList::iterator iter = method_bindings_.begin();
+      iter != method_bindings_.end();
+      ++iter) {
+    if ((*iter)->MatchesName(name) && (*iter)->arg_count() == arg_count)
+      return (*iter)->Invoked(this, args, result);
+  }
+  return false;
+}
+
 }  // namespace content