Browser Plugin: Update DOM Node attributes when properties are updated

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 177ffaa008952281b452b94f6be375b17b60b717..9364fe6b4bdc46f0220fdd73cb5ee73fa1eb76a4 100644
--- a/content/renderer/browser_plugin/browser_plugin_bindings.cc
+++ b/content/renderer/browser_plugin/browser_plugin_bindings.cc
@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
+#include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "base/utf_string_conversions.h"
 #include "content/renderer/browser_plugin/browser_plugin.h"
@@ -392,6 +393,7 @@ class BrowserPluginPropertyBinding {
   explicit BrowserPluginPropertyBinding(const char name[]) : name_(name) {
   }
   virtual ~BrowserPluginPropertyBinding() {}
+  const std::string& name() const { return name_; }
   bool MatchesName(NPIdentifier name) const {
     return WebBindings::getStringIdentifier(name_.c_str()) == name;
   }
@@ -402,6 +404,7 @@ class BrowserPluginPropertyBinding {
                            NPObject* np_obj,
                            NPIdentifier name,
                            const NPVariant* variant) = 0;
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) = 0;

[lazyboy, 2012/11/30 22:11:36]

I'd make this non pure virtual, and remove empty impl in ContentWindowBinding
below.

Also add method comment 'Updates dom attribute value with the current plugin
attribute value', to make it clear that we need to call this *after* setting the
attribute value to plugin.

[Fady Samuel, 2012/11/30 22:57:12]

Done.

  private:
   std::string name_;
 
@@ -417,18 +420,23 @@ class BrowserPluginPropertyBindingAutoSize
   virtual bool GetProperty(BrowserPluginBindings* bindings,
                            NPIdentifier name,
                            NPVariant* result) OVERRIDE {
-    bool autosize = bindings->instance()->auto_size_attribute();
-    BOOLEAN_TO_NPVARIANT(autosize, *result);
+    bool auto_size = bindings->instance()->auto_size_attribute();
+    BOOLEAN_TO_NPVARIANT(auto_size, *result);
     return true;
   }
   virtual bool SetProperty(BrowserPluginBindings* bindings,
                            NPObject* np_obj,
                            NPIdentifier name,
                            const NPVariant* variant) OVERRIDE {
-    bool autosize = NPVARIANT_TO_BOOLEAN(*variant);
-    bindings->instance()->SetAutoSizeAttribute(autosize);
+    bool auto_size = NPVARIANT_TO_BOOLEAN(*variant);
+    bindings->instance()->SetAutoSizeAttribute(auto_size);
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    bool auto_size = bindings->instance()->auto_size_attribute();
+    bindings->instance()->UpdateDOMAttribute(
+        name(), auto_size ? "true" : "false");
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingAutoSize);
 };
@@ -455,6 +463,8 @@ class BrowserPluginPropertyBindingContentWindow
                            const NPVariant* variant) OVERRIDE {
     return false;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingContentWindow);
 };
@@ -480,6 +490,11 @@ class BrowserPluginPropertyBindingMaxHeight
     bindings->instance()->SetMaxHeightAttribute(max_height);
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    int max_height = bindings->instance()->max_height_attribute();
+    bindings->instance()->UpdateDOMAttribute(
+        name(), base::IntToString(max_height));
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingMaxHeight);
 };
@@ -505,6 +520,11 @@ class BrowserPluginPropertyBindingMaxWidth
     bindings->instance()->SetMaxWidthAttribute(max_width);
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    int max_width = bindings->instance()->max_width_attribute();
+    bindings->instance()->UpdateDOMAttribute(
+        name(), base::IntToString(max_width));
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingMaxWidth);
 };
@@ -530,6 +550,11 @@ class BrowserPluginPropertyBindingMinHeight
     bindings->instance()->SetMinHeightAttribute(min_height);
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    int min_height = bindings->instance()->min_height_attribute();
+    bindings->instance()->UpdateDOMAttribute(
+        name(), base::IntToString(min_height));
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingMinHeight);
 };
@@ -555,6 +580,11 @@ class BrowserPluginPropertyBindingMinWidth
     bindings->instance()->SetMinWidthAttribute(min_width);
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    int min_width = bindings->instance()->min_width_attribute();
+    bindings->instance()->UpdateDOMAttribute(
+        name(), base::IntToString(min_width));
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingMinWidth);
 };
@@ -585,6 +615,10 @@ class BrowserPluginPropertyBindingPartition
     }
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    std::string partition_id = bindings->instance()->GetPartitionAttribute();
+    bindings->instance()->UpdateDOMAttribute(name(), partition_id);
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingPartition);
 };
@@ -613,6 +647,10 @@ class BrowserPluginPropertyBindingSrc : public BrowserPluginPropertyBinding {
     }
     return true;
   }
+  virtual void UpdateDOMAttribute(BrowserPluginBindings* bindings) OVERRIDE {
+    std::string src = bindings->instance()->src_attribute();
+    bindings->instance()->UpdateDOMAttribute(name(), src);
+  }
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserPluginPropertyBindingSrc);
 };
@@ -698,8 +736,13 @@ bool BrowserPluginBindings::SetProperty(NPObject* np_obj,
   for (PropertyBindingList::iterator iter = property_bindings_.begin();
        iter != property_bindings_.end();
        ++iter) {
-    if ((*iter)->MatchesName(name))
-      return (*iter)->SetProperty(this, np_obj, name, variant);
+    if ((*iter)->MatchesName(name)) {
+      if ((*iter)->SetProperty(this, np_obj, name, variant)) {
+          (*iter)->UpdateDOMAttribute(this);
+          return true;
+      }
+      return false;

[lazyboy, 2012/11/30 22:11:36]

nit: is break; more readable? not sure.

[Fady Samuel, 2012/11/30 22:57:12]

I'm indifferent. I just changed it to break;

+    }
   }
   return false;
 }