Browser Plugin: Update DOM Node attributes when properties are updated

content/renderer/browser_plugin/browser_plugin.cc

Index: content/renderer/browser_plugin/browser_plugin.cc
diff --git a/content/renderer/browser_plugin/browser_plugin.cc b/content/renderer/browser_plugin/browser_plugin.cc
index e47fe771e47b77ae448495f0dc5a472c352d10f6..edfb19b7f2b13a19be441db85c94d05e116dc311 100644
--- a/content/renderer/browser_plugin/browser_plugin.cc
+++ b/content/renderer/browser_plugin/browser_plugin.cc
@@ -6,6 +6,7 @@
 
 #include "base/json/json_string_value_serializer.h"
 #include "base/message_loop.h"
+#include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "content/common/browser_plugin_messages.h"
@@ -56,7 +57,12 @@ const char kEventLoadStop[] = "loadstop";
 const char kEventSizeChanged[] = "sizechanged";
 
 // Parameters/properties on events.
+const char kAutoSize[] = "autosize";

[lazyboy, 2012/11/30 19:15:06]

I'd group them separately if we have to add them, these are not
"parameters/properties on events", maybe group them under 'attributes' or sth
similar.


One way to avoid these dup attribute names would be to make all SetXXXAttribute
return bool, and in bindings if the func returns true you can UpdateAttribute
from there. e.g:

  BrowserPluginPropertyBindingMaxHeight {
    if (bindings->instance()->SetMaxHeightAttribute(max_height))
      bindings->instance()->SetAttribute(name_, ...);
   }

You wouldn't need these in that case. WDYT?

[Fady Samuel, 2012/11/30 21:56:50]

Added UpdateDOMAttribute as an abstract method of BrowserPluginPropertyBinding. 

 const char kIsTopLevel[] = "isTopLevel";
+const char kMaxHeight[] = "maxHeight";
+const char kMaxWidth[] = "maxWidth";
+const char kMinHeight[] = "minHeight";
+const char kMinWidth[] = "minWidth";
 const char kNewURL[] = "newUrl";
 const char kNewHeight[] = "newHeight";
 const char kNewWidth[] = "newWidth";
@@ -66,8 +72,8 @@ const char kOldWidth[] = "oldWidth";
 const char kPartition[] = "partition";
 const char kPersistPrefix[] = "persist:";
 const char kProcessId[] = "processId";
-const char kSrc[] = "src";
 const char kReason[] = "reason";
+const char kSrc[] = "src";
 const char kURL[] = "url";
 
 // Error messages.
@@ -188,6 +194,7 @@ bool BrowserPlugin::SetSrcAttribute(const std::string& src,
   // so this value is used for enforcing this.
   navigate_src_sent_ = true;
   src_ = src;
+  UpdateAttribute(kSrc, src_);

[lazyboy, 2012/11/30 19:15:06]

UpdateDOMAttribute maybe? b/c we already call the internal attribute update
functions 'SetXXXAttribute' maybe this will explicitly suggest that it's for DOM
element only.

[Fady Samuel, 2012/11/30 21:56:50]

Done.

   return true;
 }
 
@@ -195,6 +202,7 @@ void BrowserPlugin::SetAutoSizeAttribute(bool auto_size) {
   if (auto_size_ == auto_size)
     return;
   auto_size_ = auto_size;
+  UpdateAttribute(kAutoSize, auto_size_ ? "true" : "false");
   last_view_size_ = plugin_rect_.size();
   UpdateGuestAutoSizeState();
 }
@@ -259,12 +267,23 @@ bool BrowserPlugin::DamageBufferMatches(
 }
 #endif
 
+void BrowserPlugin::UpdateAttribute(
+    const std::string& attribute_name,
+    const std::string& attribute_value) {
+  if (container()) {
+    container()->element().setAttribute(
+        WebKit::WebString::fromUTF8(attribute_name),
+        WebKit::WebString::fromUTF8(attribute_value));
+  }
+}
+
 void BrowserPlugin::SetMaxHeightAttribute(int max_height) {
   if (max_height_ == max_height)
     return;
   max_height_ = max_height;
   if (!auto_size_)
     return;
+  UpdateAttribute(kMaxHeight, base::IntToString(max_height));
   UpdateGuestAutoSizeState();
 }
 
@@ -274,6 +293,7 @@ void BrowserPlugin::SetMaxWidthAttribute(int max_width) {
   max_width_ = max_width;
   if (!auto_size_)
     return;
+  UpdateAttribute(kMaxWidth, base::IntToString(max_width_));
   UpdateGuestAutoSizeState();
 }
 
@@ -283,6 +303,7 @@ void BrowserPlugin::SetMinHeightAttribute(int min_height) {
   min_height_ = min_height;
   if (!auto_size_)
      return;
+  UpdateAttribute(kMinHeight, base::IntToString(min_height_));
   UpdateGuestAutoSizeState();
 }
 
@@ -292,6 +313,7 @@ void BrowserPlugin::SetMinWidthAttribute(int min_width) {
   min_width_ = min_width;
   if (!auto_size_)
      return;
+  UpdateAttribute(kMinWidth, base::IntToString(min_width_));
   UpdateGuestAutoSizeState();
 }
 
@@ -358,6 +380,7 @@ bool BrowserPlugin::SetPartitionAttribute(const std::string& partition_id,
 
   valid_partition_id_ = true;
   storage_partition_id_ = input;
+  UpdateAttribute(kPartition, storage_partition_id_);
   return true;
 }
 
@@ -613,7 +636,10 @@ void BrowserPlugin::LoadCommit(
   // If the guest has just committed a new navigation then it is no longer
   // crashed.
   guest_crashed_ = false;
-  src_ = params.url.spec();
+  if (params.is_top_level) {

[lazyboy, 2012/11/30 19:15:06]

Yes, please make this fix in a separate CL.

[Fady Samuel, 2012/11/30 21:56:50]

Done.

+    src_ = params.url.spec();
+    UpdateAttribute(kSrc, src_.c_str());
+  }
   process_id_ = params.process_id;
   current_nav_entry_index_ = params.current_entry_index;
   nav_entry_count_ = params.entry_count;