Serializing styles in JSON. Previous implementation (serializing as cssText) lost information about prperty shorthands.

Now StyleSidebarPane correctly groups properties.

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

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

4 files changed, 133 insertions, 56 deletions

diff --git a/webkit/glue/devtools/dom_agent_impl.cc b/webkit/glue/devtools/dom_agent_impl.cc
index 04b11da..c63764e 100644
--- a/webkit/glue/devtools/dom_agent_impl.cc
+++ b/webkit/glue/devtools/dom_agent_impl.cc
@@ -6,6 +6,8 @@
 
 #include "AtomicString.h"
 #include "CSSComputedStyleDeclaration.h"
+#include "CSSParser.h"
+#include "CSSPropertyNames.h"
 #include "CSSRule.h"
 #include "CSSRuleList.h"
 #include "CSSStyleRule.h"
@@ -447,9 +449,6 @@ void DomAgentImpl::PerformSearch(int call_id, const String& query) {
 void DomAgentImpl::GetNodeStyles(int call_id,
                                  int element_id,
                                  bool author_only) {
-  // TODO (serya): Currently styles are serialized as cssText.
-  // It could be not enough.
-
   DictionaryValue result;
 
   Node* node = GetNodeForId(element_id);
@@ -460,14 +459,12 @@ void DomAgentImpl::GetNodeStyles(int call_id,
   }
 
   Element* element = static_cast<Element*>(node);
-  String inline_style = element->style()->cssText();
-  result.SetString(L"inlineStyle",
-      webkit_glue::StringToStdString(inline_style));
+  result.Set(L"inlineStyle",
+      BuildValueForStyle(*element->style()));
 
   DOMWindow* window = element->document()->defaultView();
-  String computed_style = window->getComputedStyle(element, "")->cssText();
-  result.SetString(L"computedStyle",
-      webkit_glue::StringToStdString(computed_style));
+  result.Set(L"computedStyle",
+      BuildValueForStyle(*window->getComputedStyle(element, "")));
 
   RefPtr<CSSRuleList> rule_list = window->getMatchedCSSRules(element, "",
       author_only);
@@ -580,8 +577,9 @@ ListValue* DomAgentImpl::BuildValueForCSSRules(CSSRuleList& matched) {
         webkit_glue::StringToStdString(rule->selectorText()));
 
     CSSMutableStyleDeclaration* style = rule->style();
-    description->SetString(L"cssText",
-        webkit_glue::StringToStdString(style ? style->cssText() : ""));
+    if (style) {
+      description->Set(L"style", BuildValueForStyle(*style));
+    }
 
     CSSStyleSheet* parent_style_sheet = rule->parentStyleSheet();
     if (parent_style_sheet) {
@@ -608,15 +606,41 @@ DictionaryValue* DomAgentImpl::BuildValueForAttributeStyles(
     if (CSSStyleDeclaration* style = attr->style()) {
       std::wstring name =
           webkit_glue::StringToStdWString(attr->name().toString());
-      std::string css_text =
-          webkit_glue::StringToStdString(style->cssText());
 
-      description->SetString(name, css_text);
+      description->Set(name, BuildValueForStyle(*style));
     }
   }
   return description.release();
 }
 
+ListValue* DomAgentImpl::BuildValueForStyle(const CSSStyleDeclaration& style) {
+  OwnPtr<ListValue> prop_list(new ListValue);
+  for (int i = 0; i != style.length(); ++i) {
+    String name = style.item(i);
+    int id = cssPropertyID(name);
+
+    bool important = style.getPropertyPriority(id);
+    bool implicit = style.isPropertyImplicit(id);
+    int shorthand_id = style.getPropertyShorthand(id);
+    String shorthand =
+        getPropertyName(static_cast<CSSPropertyID>(shorthand_id));
+
+    OwnPtr<ListValue> prop(new ListValue);
+    prop->Append(Value::CreateStringValue(
+        webkit_glue::StringToStdWString(name)));
+    prop->Append(Value::CreateBooleanValue(important));
+    prop->Append(Value::CreateBooleanValue(implicit));
+    prop->Append(Value::CreateStringValue(
+        webkit_glue::StringToStdWString(shorthand)));
+    prop->Append(Value::CreateStringValue(
+        webkit_glue::StringToStdWString(style.getPropertyValue(id))));
+
+    prop_list->Append(prop.release());
+  }
+
+  return prop_list.release();
+}
+
 Node* DomAgentImpl::InnerFirstChild(Node* node) {
   if (node->isFrameOwnerElement()) {
     HTMLFrameOwnerElement* frame_owner =
diff --git a/webkit/glue/devtools/dom_agent_impl.h b/webkit/glue/devtools/dom_agent_impl.h
index 9c1bfc0..2751ecb 100644
--- a/webkit/glue/devtools/dom_agent_impl.h
+++ b/webkit/glue/devtools/dom_agent_impl.h
@@ -114,7 +114,7 @@ class DomAgentImpl : public DomAgent {
       int depth);
 
   // Serializes given element's attributes into the list value.
-  ListValue* BuildValueForElementAttributes(WebCore::Element* elemen);
+  static ListValue* BuildValueForElementAttributes(WebCore::Element* elemen);
 
   // Serializes given elements's children into the list value.
   ListValue* BuildValueForElementChildren(
@@ -122,12 +122,18 @@ class DomAgentImpl : public DomAgent {
       int depth);
 
   // Serializes CSS rule list into the list value.
-  ListValue* BuildValueForCSSRules(WebCore::CSSRuleList& matched);
+  static ListValue* BuildValueForCSSRules(WebCore::CSSRuleList& matched);
 
   // Serializes attribute styles into the dictionary value.
-  DictionaryValue* BuildValueForAttributeStyles(
+  static DictionaryValue* BuildValueForAttributeStyles(
       const WebCore::NamedNodeMap& attributes);
 
+  // Serializes CSSStyleDeclaration into a list of properties
+  // where aeach property represented as an array as an
+  // [name, important, implicit, shorthand, value]
+  static ListValue* BuildValueForStyle(
+      const WebCore::CSSStyleDeclaration& style);
+
   // We represent embedded doms as a part of the same hierarchy. Hence we
   // treat children of frame owners differently. Following two methods
   // encapsulate frame owner specifics.
diff --git a/webkit/glue/devtools/js/devtools_host_stub.js b/webkit/glue/devtools/js/devtools_host_stub.js
index fd398bc..347847e 100644
--- a/webkit/glue/devtools/js/devtools_host_stub.js
+++ b/webkit/glue/devtools/js/devtools_host_stub.js
@@ -128,13 +128,17 @@ RemoteDomAgentStub.prototype.DiscardBindings = function() {
 
 
 RemoteDomAgentStub.prototype.GetNodeStyles = function(callId, id, authorOnly) {
+  var prop = function(name, value) {
+    return [name, false, false, '', value];
+  };
+
   var styles = {
-      computedStyle: "display: none",
-      inlineStyle: "display: none",
-      styleAttributes: {attr: "display: none"},
-      matchedCSSRules: [{selector: "S", cssText: "display: none",
-                         parentStyleSheetHref: "http://localhost",
-                         parentStyleSheetOwnerNodeName: "DIV"}]
+      computedStyle: [prop('display', 'none')],
+      inlineStyle: [prop('display', 'none')],
+      styleAttributes: {attr: [prop('display', 'none')]},
+      matchedCSSRules: [{selector: 'S', style: [prop('display', 'none')],
+                         parentStyleSheetHref: 'http://localhost',
+                         parentStyleSheetOwnerNodeName: 'DIV'}]
   };
   setTimeout(function() {
     RemoteDomAgent.DidGetNodeStyles(callId, styles);
diff --git a/webkit/glue/devtools/js/dom_agent.js b/webkit/glue/devtools/js/dom_agent.js
index 2e4b547..0dbac35 100644
--- a/webkit/glue/devtools/js/dom_agent.js
+++ b/webkit/glue/devtools/js/dom_agent.js
@@ -214,6 +214,7 @@ devtools.DomNode.prototype.setAttribute = function(name, value) {
       });
 };
 
+
 /**
  * Creates an attribute-like object and adds it to the object.
  * @param {string} name Attribute name to set value for.
@@ -262,7 +263,6 @@ devtools.DomNode.prototype.__defineGetter__("style", function() {
 });
 
 
-
 /**
  * Makes available the following methods and properties:
  * - node.style property
@@ -282,12 +282,12 @@ devtools.DomNode.prototype.__defineGetter__("style", function() {
 devtools.DomNode.prototype.setStyles_ = function(computedStyle, inlineStyle,
      styleAttributes, matchedCSSRules) {
   var styles = {};
-  styles.computedStyle = this.parseCSSText_(computedStyle, "computed");
-  styles.inlineStyle = this.parseCSSText_(inlineStyle, "inline");
+  styles.computedStyle = this.makeStyle_(computedStyle);
+  styles.inlineStyle = this.makeStyle_(inlineStyle);
 
   styles.attributes = {};
   for (var name in styleAttributes) {
-    var style = this.parseCSSText_(styleAttributes[name], "@" + name);
+    var style = this.makeStyle_(styleAttributes[name]);
     styles.attributes[name] = style;
   }
 
@@ -295,7 +295,7 @@ devtools.DomNode.prototype.setStyles_ = function(computedStyle, inlineStyle,
   for (var i = 0; i < matchedCSSRules.length; i++) {
     var descr = matchedCSSRules[i];
     var selector = descr.selector;
-    var style = this.parseCSSText_(descr.cssText, "CSSRule#" + selector);
+    var style = this.makeStyle_(descr.style);
 
     var parentStyleSheet = undefined;
     if (descr.parentStyleSheetHref) {
@@ -312,37 +312,18 @@ devtools.DomNode.prototype.setStyles_ = function(computedStyle, inlineStyle,
   }
   
   this.styles_ = styles;
-}
+};
 
 
 /**
- * Creates a style object from the cssText.
- * Since the StyleSidebarPane implies the
- * style object lives as long as the node itself and stores data in
- * __disabledPropertyPriorities this methods adds a getter which stores the
- * data in the devtools.DomNode object.
- * @param {string} cssText
- * @param {string} styleId is used to distinguish associated part of
- *     __disabledPropertyPriorities with the style object.
- * @return {CSSStyleDescription}
+ * Creates a style declaration.
+ * @param {payload} payload
+ * @return {devtools.CSSStyleDeclaration:undefined}
+ * @see devtools.CSSStyleDeclaration
  */
-devtools.DomNode.prototype.parseCSSText_ = function(cssText, styleId) {
-  // There is no way to create CSSStyleDeclaration without creating a
-  // dummy element. In real DOM CSSStyleDeclaration has several
-  // implementations (for instance CSSComputedStyleDeclaration) and
-  // current method does not covers diffirences in behaviour.
-  // TODO (serya): correclty implement all types of CSSStyleDeclaration,
-  //               avoid creation a lot of dummy nodes.
-  
-  var style = document.createElement("SPAN").style;
-  style.cssText = cssText;
-  
-  var props = this.disabledStyleProperties_[styleId] || {};
-  this.disabledStyleProperties_[styleId] = props;
-  style.__disabledPropertyPriorities = props;
-  
-  return style;
-}
+devtools.DomNode.prototype.makeStyle_ = function(payload) {
+  return payload && new devtools.CSSStyleDeclaration(payload);
+};
 
 
 /**
@@ -352,7 +333,7 @@ devtools.DomNode.prototype.parseCSSText_ = function(cssText, styleId) {
  */
 devtools.DomNode.prototype.clearStyles_ = function() {
   this.styles_ = null;
-}
+};
 
 
 /**
@@ -881,6 +862,68 @@ devtools.DomAgent.prototype.getNodeStylesCallback_ = function(node,
 };
 
 
+/**
+ * Represents remote CSSStyleDeclaration for using in StyleSidebarPane.
+ * @param {Array} payload built by DomAgentImpl::BuildValueForStyle.
+ * @consctuctor
+ */
+devtools.CSSStyleDeclaration = function(payload) {
+  this.length = payload.length;
+  this.important_ = {};
+  this.implicit_ = {};
+  this.shorthand_ = {};
+  this.value_ = {};
+
+  for (var i = 0; i < payload.length; ++i) {
+    var p = payload[i];
+    var name = p[0];
+    
+    this.important_[name] = p[1];
+    this.implicit_[name] = p[2];
+    this.shorthand_[name] = p[3];
+    this.value_[name] = p[4];
+
+    this[i] = name;
+  }
+};
+
+
+/**
+ * @param {string} name of a CSS property.
+ * @return {string}
+ */
+devtools.CSSStyleDeclaration.prototype.getPropertyValue = function(name) {
+  return this.value_[name] || '';
+};
+
+
+/**
+ * @param {string} name of a CSS property.
+ * @return {string} 'important' | ''.
+ */
+devtools.CSSStyleDeclaration.prototype.getPropertyPriority = function(name) {
+  return this.important_[name] ? 'important' : '';
+};
+
+
+/**
+ * @param {string} name of a CSS property.
+ * @return {string} shorthand name  or ''
+ */
+devtools.CSSStyleDeclaration.prototype.getPropertyShorthand = function(name) {
+  return this.shorthand_[name] || '';
+};
+
+
+/**
+ * @param {string} name of a CSS property.
+ * @return {boolean}
+ */
+devtools.CSSStyleDeclaration.prototype.isPropertyImplicit = function(name) {
+  return Boolean(this.implicit_[name]);
+};
+
+
 function firstChildSkippingWhitespace() {
   return this.firstChild;
 }