You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by jw...@apache.org on 2011/02/03 00:54:36 UTC

svn commit: r1066699 - in /myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal: renderkit/core/skin/ style/ style/cache/ style/xml/parse/

Author: jwaldman
Date: Wed Feb  2 23:54:35 2011
New Revision: 1066699

URL: http://svn.apache.org/viewvc?rev=1066699&view=rev
Log:
TRINIDAD-2025 move _styleNodeToStyleMap to FileSystemStyleCache to take advantage of same entires on the higher, browser or version or platform layer
1. moved _styleNodeToStyleMap to FSSC. renamed to private ConcurrentMap<Style, Style> _reusableStyleMap;
2. CSSStyle is a big object, so instead I use the new class UnmodifiableStyle which is much smaller
3. Use ArrayMap instead of ConcurrentHashMap for Properties when I could.
4. Remove CSSStyleParser.java since it was not being used.
5. mark CSSStyle and BaseStyle deprecated. They are only used now in the image generation code that isn't used in Trinidad.

Added:
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java   (with props)
Removed:
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/CSSStyleParser.java
Modified:
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java
    myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleSheetDocument.java

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/skin/XhtmlSkin.java Wed Feb  2 23:54:35 2011
@@ -18,17 +18,18 @@
  */
 package org.apache.myfaces.trinidadinternal.renderkit.core.skin;
 
-import java.util.HashMap;
 import java.util.Map;
 
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinProperties;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinSelectors;
 import org.apache.myfaces.trinidadinternal.skin.icon.ContextImageIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.NullIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.ReferenceIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.TextIcon;
-import org.apache.myfaces.trinidadinternal.style.CSSStyle;
-
+import org.apache.myfaces.trinidadinternal.style.UnmodifiableStyle;
 
 
 /**
@@ -111,18 +112,20 @@ public class XhtmlSkin extends BaseSkin
   
   static
   {
-    // does this matter if it's not an ArrayMap?
-    _spinboxTopStyleMap = new HashMap<String, String>();
+    // todo Use ArrayMap instead of ConcurrentHashMap
+    // We were using CSSStyle instead of UnmodifiableStyle and that class copied the properties 
+    // into a ConcurrentHashMap. Changing this to another map will change the spinbox golden files.
+    _spinboxTopStyleMap = new ConcurrentHashMap<String, String>();
     _spinboxTopStyleMap.put("display", "block");
     // this is needed for the image
-    _spinboxBottomStyleMap = new HashMap<String, String>();
+    _spinboxBottomStyleMap = new ConcurrentHashMap<String, String>();
     _spinboxBottomStyleMap.put("display", "block");
     _spinboxBottomStyleMap.put("padding-top", "2px");
 
   }
   
-  private static final CSSStyle spinboxTopStyle = new CSSStyle(_spinboxTopStyleMap);
-  private static final CSSStyle spinboxBottomStyle = new CSSStyle(_spinboxBottomStyleMap);
+  private static final Style spinboxTopStyle = new UnmodifiableStyle(_spinboxTopStyleMap);
+  private static final Style spinboxBottomStyle = new UnmodifiableStyle(_spinboxBottomStyleMap);
 
 
 

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/BaseStyle.java Wed Feb  2 23:54:35 2011
@@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHa
  * TODO Remove the ParsedProperty code from Trinidad. It is only used for
  * the un-used image generation code.
  * TODO Then remove CoreStyle and implement the public Style object instead.
+ * @deprecated
  * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/BaseStyle.java#0 $) $Date: 10-nov-2005.18:57:54 $
  */
 abstract public class BaseStyle extends CoreStyle implements Serializable 

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/CSSStyle.java Wed Feb  2 23:54:35 2011
@@ -19,17 +19,15 @@
 package org.apache.myfaces.trinidadinternal.style;
 
 import java.util.Arrays;
-import java.util.Map;
 import java.util.Collections;
-import java.util.Iterator;
-
-import org.apache.myfaces.trinidad.style.Style;
+import java.util.Map;
 
 import org.apache.myfaces.trinidadinternal.style.util.CSSUtils;
 
 
 /**
- * Style implementation for CSS.
+ * Style implementation for CSS. Mutable.
+ * @deprecated Use UnmodifiableStyle which cannot be modified once it is created.
  *
  * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/CSSStyle.java#0 $) $Date: 10-nov-2005.18:57:55 $
  */

Added: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java?rev=1066699&view=auto
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java (added)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java Wed Feb  2 23:54:35 2011
@@ -0,0 +1,131 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ * 
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.myfaces.trinidadinternal.style;
+
+import java.util.Map;
+import java.util.Collections;
+
+import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.myfaces.trinidad.style.Style;
+
+/**
+ * Style implementation for CSS properties. Once created, you cannot mutate since there is no setProperties method.
+ * The toInlineString prints the properties out in CSS format, e.g., color: blue; font-size: 12px;
+ * To save memory, as the constructor parameter
+ * use an ArrayMap instead of a HashMap or ConcurrentHashMap if you can.
+ *
+ * @version $Name:  $ ($Revision: adfrt/faces/adf-faces-impl/src/main/java/oracle/adfinternal/view/faces/style/CSSStyle.java#0 $) $Date: 10-nov-2005.18:57:55 $
+ */
+public class UnmodifiableStyle extends Style
+{
+  /**
+   * Creates a Style object with the specified properties. There is no no-argument constructor
+   * because you must pass in all your properties when you create your object.
+   *
+   * @param properties The properties of this style, like  background-color, blue.  The
+   *   values must be Strings. 
+   * @throws NullPointerException if properties is null
+   */
+  public UnmodifiableStyle(Map<String, String> properties)
+  {
+    if (properties == null)
+      throw new NullPointerException("properties must be non-null");
+
+     _propertiesMap = Collections.unmodifiableMap(properties);
+  }
+     
+  @Override
+  public Map<String, String> getProperties()
+  {
+    return _propertiesMap;
+  }
+  
+  /**
+   * Converts the style to a String suitable for use as an inline style
+   * attribute value.
+   */
+  @Override
+  public String toInlineString()
+  {
+    String inline = _inline;
+
+    if (inline != null)
+      return inline;
+    
+    Map<String, String> properties = getProperties();
+    StringBuffer buffer = new StringBuffer(_DEFAULT_BUFFER_SIZE);
+    boolean first = true;   
+    
+    for (Map.Entry<String, String> entrySet : properties.entrySet())
+    {
+
+      if (first)
+        first = false;
+      else
+        buffer.append(";");
+      String name = entrySet.getKey();
+      String value = entrySet.getValue();
+      buffer.append(name);
+      buffer.append(":");
+      buffer.append(value);
+    }
+
+    inline = buffer.toString();
+    
+    _inline = inline;
+
+    return inline;
+  }
+
+  @Override
+  public String toString()
+  {
+    return "UnmodifiableStyle[css=" + toInlineString() + "]"; 
+  }
+  
+
+  @Override
+  public boolean equals(Object obj)
+  {
+    if (this == obj)
+      return true;
+    if (!(obj instanceof UnmodifiableStyle))
+      return false;
+
+    UnmodifiableStyle test = (UnmodifiableStyle)obj;
+    return this._propertiesMap.equals(test._propertiesMap);
+  }
+
+  @Override
+  public int hashCode()
+  {
+    return _propertiesMap.hashCode();
+  }
+
+  // The cached inline String value
+  // Marking it volatile guarantees that a read of _inline 
+  // always returns the most recent write by any thread.
+  transient volatile private String _inline;
+  final private Map<String, String> _propertiesMap;
+
+  // Default length for inline string buffer
+  private static final int _DEFAULT_BUFFER_SIZE = 100;
+}

Propchange: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/UnmodifiableStyle.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/cache/FileSystemStyleCache.java Wed Feb  2 23:54:35 2011
@@ -50,15 +50,16 @@ import org.apache.myfaces.trinidad.skin.
 import org.apache.myfaces.trinidad.style.Selector;
 import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidad.style.Styles;
+import org.apache.myfaces.trinidad.util.ArrayMap;
 import org.apache.myfaces.trinidad.util.CollectionUtils;
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent;
 import org.apache.myfaces.trinidadinternal.renderkit.core.CoreRenderingContext;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinProperties;
 import org.apache.myfaces.trinidadinternal.renderkit.core.xhtml.SkinSelectors;
 import org.apache.myfaces.trinidadinternal.share.expl.Coercions;
-import org.apache.myfaces.trinidadinternal.style.CSSStyle;
 import org.apache.myfaces.trinidadinternal.style.StyleContext;
 import org.apache.myfaces.trinidadinternal.style.StyleProvider;
+import org.apache.myfaces.trinidadinternal.style.UnmodifiableStyle;
 import org.apache.myfaces.trinidadinternal.style.util.CSSGenerationUtils;
 import org.apache.myfaces.trinidadinternal.style.util.NameUtils;
 import org.apache.myfaces.trinidadinternal.style.util.StyleWriterFactory;
@@ -72,7 +73,11 @@ import org.apache.myfaces.trinidadintern
 
 /**
  * The FileSystemStyleCache is a StyleProvider implementation which
- * caches generated CSS style sheets on the file system.
+ * caches generated CSS style sheets on the file system. A FileSystemStyleCache object is for one Skin.
+ * The FileSystemStyleCache instance contains Entry objects for a Skin: one Entry
+ * object per unique generated CSS style sheet (e.g., gecko and ie will most
+ * likely have different generated CSS style sheets for the same Skin because these browsers tend to need
+ * different css rules).
  *
  * Note that StyleProviders are responsible for providing access
  * both to style information (eg. getStyleSheetURI(), getStyles()) as
@@ -121,7 +126,7 @@ public class FileSystemStyleCache implem
    * Implementation of StyleCache.getStyleSheetURI().
    */
   public List<String> getStyleSheetURIs(StyleContext context)
-  {
+  {    
     Entry entry = _getEntry(context);
 
     if (entry == null)
@@ -304,6 +309,7 @@ public class FileSystemStyleCache implem
         _cache = null;
         _entryCache = null;
         _document = null;
+        _reusableStyleMap = null;
         _shortStyleClassMap = null;
         _namespacePrefixes  = null;
       }
@@ -319,6 +325,8 @@ public class FileSystemStyleCache implem
         _cache = new ConcurrentHashMap<Key, Entry>();
       if (_entryCache == null)
         _entryCache = new ConcurrentHashMap<Object, Entry>(19);
+      if (_reusableStyleMap == null)
+        _reusableStyleMap = new ConcurrentHashMap<Style, Style>();
 
       cache = _cache;
       entryCache = _entryCache;
@@ -440,7 +448,22 @@ public class FileSystemStyleCache implem
     if (styleNodes == null)
       return null;
 
-
+    /* This code fills in the <Selector, Style> resolvedSelectorStyleMap map. 
+     * We use _reusableStyleMap to reuse the Style objects when possible
+     * since we have a large number of Style objects. */
+    ConcurrentMap<Selector, Style> resolvedSelectorStyleMap = null;
+    for (int i=0; i < styleNodes.length; i++)
+    {
+      String selector = styleNodes[i].getSelector();
+      if (selector != null)
+      {
+        Style style = _convertStyleNodeToStyle(styleNodes[i], _reusableStyleMap);
+        if (resolvedSelectorStyleMap == null)
+          resolvedSelectorStyleMap = new ConcurrentHashMap<Selector, Style>();
+        resolvedSelectorStyleMap.put(Selector.createSelector(selector), style);
+      }
+    }
+    
     // Generate the style sheet file, if it isn't already generated,
     // and return the uri.
     // Only the StyleNodes with non-null selectors get written to the
@@ -469,8 +492,8 @@ public class FileSystemStyleCache implem
     // Create a new entry and cache it in the "normal" cache. The "normal" cache is one
     // where the key is the Key object which is built based on information from the StyleContext,
     // like browser, agent, locale, direction.
-    Styles styles = new StylesImpl(styleNodes, namespacePrefixes, _STYLE_KEY_MAP,
-                                   shortStyleClassMap,  _isCompressStyles(context));
+    Styles styles = new StylesImpl(namespacePrefixes, _STYLE_KEY_MAP,
+                                   shortStyleClassMap,  _isCompressStyles(context), resolvedSelectorStyleMap);
     Entry entry = new Entry(uris, styles, icons, skinProperties);
     cache.put(key, entry);
 
@@ -1084,9 +1107,55 @@ public class FileSystemStyleCache implem
     return _SHORT_CLASS_PREFIX + Integer.toString(count, Character.MAX_RADIX);
   }
 
+  
+  /**
+   * Given a StyleNode object, which is an internal API that denotes a Style object
+   * with additional information like includedSelectors, create a simple public
+   * Style object which will be used in the SelectorStyleMap. When this method is called,
+   * the StyleNode object is already resolved (included selectors have been merged in)
+   * so that all the css properties are there.
+   * @param styleNode
+   * @param reusableStyleMap A Map<Style, Style>. This is 
+   *  used so that we can reuse Style objects in StylesImpl if they have the same list of style property
+   *  names and values.
+   * @return A Style object created from the information in the styleNode. We reuse
+   *  Style objects if the properties are the same.
+   */
+  public Style _convertStyleNodeToStyle(
+    StyleNode          styleNode, 
+    Map<Style, Style>  reusableStyleMap)
+  {
+    // Add in the properties for the style; PropertyNode interns the 'name' and the most common 'value's.
+    Collection<PropertyNode> propertyNodeList = styleNode.getProperties();
+    Map<String, String> styleProperties = new ArrayMap<String, String>(propertyNodeList.size());
 
+    for (PropertyNode property : propertyNodeList)
+    {
+      String name = property.getName();
+      String value = property.getValue();
+      if (name != null && value != null)
+      {
+        styleProperties.put(name, value);
+      }
+    }
+
+    // To save memory, we reuse Style objects for each FileSystemStyleCache instance.
+    Style style = new UnmodifiableStyle(styleProperties);
+    Style cachedStyle = reusableStyleMap.get(style);
+    if (cachedStyle == null)
+    {
+      reusableStyleMap.put(style, style);
+      return style;         
+    }
+    else
+    {
+      return cachedStyle;
+    }
+  }
+  
   /**
-   * Key class used for hashing style sheet URIs
+   * Key class used for hashing style sheet URIs. This key for the Entry
+   * cache is dependent on the agent, locale, direction, etc.
    */
   private static class Key
   {
@@ -1199,7 +1268,10 @@ public class FileSystemStyleCache implem
   }
 
   /**
-   * Cache entry class
+   * Cache entry class.
+   * The FileSystemStyleCache instance contains Entry objects: one Entry
+   * object per unique generated CSS style sheet (e.g., gecko and ie will most
+   * likely have different generated CSS style sheets).
    */
   private static class Entry
   {
@@ -1222,9 +1294,9 @@ public class FileSystemStyleCache implem
   }
 
   /**
-   * A key object which is used to hash Entrys in the entry cache.  The key for the entry
+   * A key object which is used to hash Entrys in the entry cache.  This key for the Entry
    * cache is the style sheet derivation list - that is a list of StyleSheetNodes, sorted
-   * by specficity.
+   * by specficity. It's not dependent on the agent, locale, direction like the Key object is.
    */
   private static class DerivationKey
   {
@@ -1290,9 +1362,6 @@ public class FileSystemStyleCache implem
     private boolean _short;   // Do we use short style classes?
   }
 
-
-
-
   /**
    * A Styles implementation that adds the resolved (merged together based on the StyleContext)
    * StyleNodes to a Map. Only the style selectors and not the aliased (aka named) styles
@@ -1307,20 +1376,20 @@ public class FileSystemStyleCache implem
      * StyleNode are all null. This way we do not have to resolve the
      * styles based on the StyleContext when someone calls getStyles,
      * etc.
-     * @param resolvedStyles
      * @param namespacePrefixArray an array of namespace prefixes that are used in the custom css
      * skinning selectors, like "af" in af|inputText.
      * @param afSelectorMap a map from one selector to another (like af|panelHeader::link maps to
      * af|panelHeader A
      * @param shortStyleClassMap a map from the  non-compressed styleclass
      * to a compressed styleclass.
+     * @param resolvedSelectorStyleMap
      */
     public StylesImpl(
-        StyleNode[]         resolvedStyles,
         String[]            namespacePrefixArray,
         Map<String, String> afSelectorMap,
         Map<String, String> shortStyleClassMap,
-        boolean             compress
+        boolean             compress,
+        Map<Selector, Style> resolvedSelectorStyleMap
       )
     {
       // store these local variables to be used in getNativeSelectorString
@@ -1328,48 +1397,7 @@ public class FileSystemStyleCache implem
       _afSelectorMap = afSelectorMap;
       _shortStyleClassMap = shortStyleClassMap;
       _compress = compress;
-      // create a Selector->Style map right here (aggressively versus lazily)
-      ConcurrentMap<Selector, Style> resolvedSelectorStyleMap = null;
-      /* This is used so that we can re-use Style objects if the property name and value 
-       * is the same. Instead of creating a Style object for every property, we only
-       * create it for unique property name/values, and we store these in the map. 
-       * Then we pull them out of the map to create a Style object. */
-      Map<StyleKey, Style> styleNodeToStyleMap = new ConcurrentHashMap<StyleKey, Style>();
 
-      // Loop through each StyleNode and use it to add to the StyleMap.
-      for (int i=0; i < resolvedStyles.length; i++)
-      {
-        String selector = resolvedStyles[i].getSelector();
-        if (selector != null)
-        {
-          Style style = _convertStyleNodeToStyle(resolvedStyles[i], styleNodeToStyleMap);
-          if (resolvedSelectorStyleMap == null)
-            resolvedSelectorStyleMap = new ConcurrentHashMap<Selector, Style>();
-          resolvedSelectorStyleMap.put(Selector.createSelector(selector), style);
-        }
-        /*
-        else
-        {
-          // For now, do not add the named styles to the map. If we do add the named styles
-          // to the map then we should in SkinStyleSheetParserUtils put the full name in
-          // the StyleNode, not strip out the '.' or the ':alias'. However, in the XSS
-          // the named styles do not have the '.' or the ':alias' which is why we string them out.
-          // if we put them back, then things won't merge correctly. How do I workaround this?
-          // Do I change all the named styles in the XSS file?
-          // I think the best thing to do is to change the XSS file, since that is proprietary,
-          // and no one should be relying on it. If we instead kept stripping out the . and the alias
-          // and required the person to ask for the alias without this,
-          // that is much more confusing to the user.
-          String name = _resolvedStyles[i].getName();
-          if (name != null)
-          {
-            // create a Style Object from the StyleNode object
-            Style style = _convertStyleNode(resolvedStyles[i]);
-            resolvedSelectorStyleMap.put(name, style);
-          }
-        }
-        */
-      }
       if (resolvedSelectorStyleMap != null)
         _unmodifiableResolvedSelectorStyleMap =
           Collections.unmodifiableMap(resolvedSelectorStyleMap);
@@ -1425,111 +1453,10 @@ public class FileSystemStyleCache implem
     }
 
 
-    /**
-     * Given a StyleNode object, which is an internal API that denotes a Style object
-     * with additional information like includedSelectors, create a simple public
-     * Style object which will be used in the SelectorStyleMap. The StyleNode object
-     * should already be resolved (included selectors have been merged in)
-     * so that all the css properties are there.
-     * @param styleNode
-     * @param styleNodeToStyleMap A Map holding StyleKey objects to Style objects. This is 
-     *  used so that we can reuse CSSStyle objects if they have the same list of style property
-     *  names and values.
-     * @return A Style object created from the information in the styleNode. We reuse
-     *  Style objects if the properties are the same.
-     */
-    public Style _convertStyleNodeToStyle(
-      StyleNode            styleNode, 
-      Map<StyleKey, Style> styleNodeToStyleMap)
-    {
-      Map<String, String> styleProperties = new ConcurrentHashMap<String, String>();
-      // Add in the properties for the style
-      Iterable<PropertyNode> propertyNodeList = styleNode.getProperties();
-      for (PropertyNode property : propertyNodeList)
-      {
-        String name = property.getName();
-        String value = property.getValue();
-        // We get a NPE in CSSStyle(styleProperties) -> putAll if we add a value that is null.
-        // See the BaseStyle(Map<String, String> propertiesMap) constructor.
-        if (name != null && value != null)
-          styleProperties.put(name, value);
-      }
-      
-      // To save memory, we reuse CSSStyle objects if 
-      // they have the same list of style property names and values.
-      // StyleKey is the key into the StyleKey, CSSStyle map.
-      StyleKey key = new StyleKey(styleProperties);
-      Style cachedStyle = styleNodeToStyleMap.get(key);
-      if (cachedStyle == null)
-      {
-        // no match is cached yet, so create a new CSSStyle and cache in the map.
-        Style style = new CSSStyle(styleProperties);
-        styleNodeToStyleMap.put(key, style);
-        return style;         
-      }
-      else
-      {
-        return cachedStyle;
-      }
-    }
-    
-    /**
-     * A StyleKey object is used as a key into a map so that we can share CSSStyle objects
-     * if they are equal and they have the same hashCode.
-     */
-    private static class StyleKey
-    {
-      public StyleKey(Map<String, String> styleProperties)
-      {
-        _styleProperties = styleProperties;
-        _hashCode = _hashCode();
-      }
-      
-      @Override
-      public int hashCode()
-      {
-        return _hashCode;
-      }
-      
-      @Override  
-      public boolean equals(Object obj)
-      {
-        if (this == obj)
-          return true;
-        if (!(obj instanceof StyleKey))
-          return false;
-          
-        // obj at this point must be a StyleKey
-        StyleKey test = (StyleKey)obj;
-        return test._styleProperties.equals(this._styleProperties);
-      }
-      
-      /**
-       * Private implementation of hashCode. This way we can cache the hashcode.
-       * @return
-       */
-      private int _hashCode() 
-      {
-        int hash = 17;
-        // take each style property name and value and create a hashCode from it.
-        for (Map.Entry<String, String> e : _styleProperties.entrySet())
-        {
-          String name = e.getKey();
-          hash = 37*hash + ((null == name) ? 0 : name.hashCode());
-
-          String value = e.getValue();
-          hash = 37*hash + ((null == value) ? 0 : value.hashCode());
 
-        }
-        return hash;        
-      }
-      
-      private final Map<String, String> _styleProperties;
-      private final int _hashCode;
+    
 
 
-    }
-
     private final Map<Selector, Style> _unmodifiableResolvedSelectorStyleMap;
     private final Map<String, String>  _afSelectorMap;
     private final String[]             _namespacePrefixArray;
@@ -1582,12 +1509,18 @@ public class FileSystemStyleCache implem
       }
     }
   }
-
+  
   private final String _targetPath; // The location of the cache
 
   /** The parsed StyleSheetDocument */
   private StyleSheetDocument _document;
-
+  
+  /** Since each FileSystemStyleCache$Entry object holds a FileSystemStyleCache$StylesImpl object
+   *  which holds on to many Style objects, it reduces the memory consumption by about half
+   *  if we reuse Style objects per FileSystemStyleCache instance rather than per
+   *  FileSystemStyleCache$Entry instance. The is the map we use to store unique Style objects. */
+  private ConcurrentMap<Style, Style> _reusableStyleMap;
+  
   /** The cache of style sheet URIs */
   private ConcurrentMap<Key, Entry> _cache;
 

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/PropertyNode.java Wed Feb  2 23:54:35 2011
@@ -16,6 +16,12 @@
 
 package org.apache.myfaces.trinidadinternal.style.xml.parse;
 
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
 import org.apache.myfaces.trinidad.logging.TrinidadLogger;
 /**
  * Private implementation of PropertyNode.
@@ -24,26 +30,40 @@ import org.apache.myfaces.trinidad.loggi
  */
 public class PropertyNode
 {
-
   /**
-   * Creates a PropertyNode with the specified name and value.
-   * @param name name of the propertyNode. Examples are 'font-size', 'background-color'.
-   * @param value value of the propertyNode. Examples are '12px', 'red', '0xeaeaea'
-   * If name is null or the empty string, an IllegalArgumentException is thrown.
-   */
-  public PropertyNode(String name, String value)
-  {
-
-    if (name == null || "".equals(name))
-      throw new IllegalArgumentException(_LOG.getMessage(
-        "PROPERTYNODE_NAME_CANNOT_BE_NULL_OR_EMPTY", new Object[]{name, value}));
-
-    // intern() name because many of the property names are the same,
-    // like color, background-color, background-image, font-size, etc.
-    // This will improve the memory used.
-    _name = name.intern();
-    _value = value;
-  }
+     * Creates a PropertyNode with the specified name and value.
+     * @param name name of the propertyNode. Examples are 'font-size', 'background-color'.
+     * @param value value of the propertyNode. Examples are '12px', 'red', '0xeaeaea'
+     * If name is null or the empty string, an IllegalArgumentException is thrown.
+     */
+    public PropertyNode(String name, String value)
+    {
+
+      if (name == null || "".equals(name))
+        throw new IllegalArgumentException(_LOG.getMessage(
+          "PROPERTYNODE_NAME_CANNOT_BE_NULL_OR_EMPTY", new Object[]{name, value}));
+
+      // intern() name because many of the property names are the same,
+      // like color, background-color, background-image, font-size, etc.
+      // This will improve the memory used.
+      _name = name.intern();
+     
+      if (_INTERN_VALUES_FOR.contains(name))
+      {
+        value = value.intern();
+      }
+      else
+      {
+        String internedValue =  _INTERNED_VALUES.get(value);
+       
+        if (internedValue != null)
+        {
+          value = internedValue;
+        }
+      }
+     
+      _value = value;
+    }
 
   /**
    * Implementation of PropertyNode.getName().
@@ -93,6 +113,107 @@ public class PropertyNode
       "[name="   + _name   + ", " +
       "value=" + _value + "]";
   }
+  
+  // map to the interned value for values that are common enough that we should intern them,
+  // but we can't always intern based purely on the property name.  A good example are
+  // proprties like vertical-align or cursor where there are many fixed values but the
+  // option to use a percentage or URL is also there
+  private static final Map<String, String> _INTERNED_VALUES = new HashMap<String, String>();
+
+  static
+  {
+    // initialize values that we should share
+    String[] internedValues = new String[]
+    {
+      "#000000",
+      "#FFFFFF",
+      "#ffffff",
+      "0",
+      "0%",
+      "0px",
+      "1",
+      "10%",
+      "100%",
+      "1px",
+      "1em",
+      "2",
+      "2px",
+      "3px",
+      "auto",
+      "4px",
+      "5px",
+      "50%",
+      "6px",
+      "7px",
+      "8px",
+      "9px",
+      "auto",
+      "baseline",
+      "black",
+      "bottom",
+      "center",
+      "center center",
+      "crosshair",
+      "default",
+      "e-resize",
+      "gray",
+      "help",
+      "inherit",
+      "left",
+      "middle",
+      "move",
+      "n-resize",
+      "ne-resize",
+      "nw-resize",
+      "none",
+      "pointer",
+      "progress",
+      "right",
+      "s-resize",
+      "se-resize",
+      "sub",
+      "super",
+      "sw-resize",
+      "text",
+      "text-bottom",
+      "text-top",
+      "top",
+      "transparent",
+      "w-resize",
+      "wait",
+      "white"
+    };
+   
+    for (int i = 0; i < internedValues.length; i++)
+    {
+      String internedValue = internedValues[i];
+     
+      _INTERNED_VALUES.put(internedValue, internedValue);
+    }
+  }
+ 
+  // property names that we should always intern the values for because they are either
+  // enumerations or are repeated often (font-family)
+  private static final Set<String> _INTERN_VALUES_FOR = new HashSet<String>(
+    Arrays.asList("-moz-box-sizing",
+                  "background-repeat",
+                  "border-style",
+                  "display",
+                  "float",
+                  "font-family",
+                  "font-style",
+                  "font-weight",
+                  "list-style-position",
+                  "list-style-type",
+                  "overflow",
+                  "overflow-x",
+                  "overflow-y",
+                  "position",
+                  "text-align",
+                  "text-decoration",
+                  "transparent",
+                  "visibility",
+                  "white-space"));
 
   private final String _name;
   private final String _value;

Modified: myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleSheetDocument.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleSheetDocument.java?rev=1066699&r1=1066698&r2=1066699&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleSheetDocument.java (original)
+++ myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/xml/parse/StyleSheetDocument.java Wed Feb  2 23:54:35 2011
@@ -34,12 +34,14 @@ import java.util.Map;
 import java.util.Set;
 import java.util.Stack;
 
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.regex.Pattern;
 
 import org.apache.myfaces.trinidad.context.AccessibilityProfile;
 import org.apache.myfaces.trinidad.context.LocaleContext;
 import org.apache.myfaces.trinidad.logging.TrinidadLogger;
 import org.apache.myfaces.trinidad.skin.Icon;
+import org.apache.myfaces.trinidad.style.Style;
 import org.apache.myfaces.trinidad.util.IntegerUtils;
 
 import org.apache.myfaces.trinidadinternal.agent.TrinidadAgent;
@@ -47,9 +49,9 @@ import org.apache.myfaces.trinidadintern
 import org.apache.myfaces.trinidadinternal.skin.icon.NullIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.TextIcon;
 import org.apache.myfaces.trinidadinternal.skin.icon.URIImageIcon;
-import org.apache.myfaces.trinidadinternal.style.CSSStyle;
 import org.apache.myfaces.trinidadinternal.style.PropertyParseException;
 import org.apache.myfaces.trinidadinternal.style.StyleContext;
+import org.apache.myfaces.trinidadinternal.style.UnmodifiableStyle;
 import org.apache.myfaces.trinidadinternal.style.util.CSSUtils;
 import org.apache.myfaces.trinidadinternal.style.util.ModeUtils;
 import org.apache.myfaces.trinidadinternal.style.util.NameUtils;
@@ -295,14 +297,18 @@ public class StyleSheetDocument
     String  uri = null;
     String  text = null;
     boolean isNullIcon = false;
-    CSSStyle  inlineStyle = null;
-
+    
+    // TODO Use an ArrayMap to use less memory, and we do not need concurrency since we will never modify this map.
+    // switching from ConcurrentHashMap to ArrayMap may break golden files, so be sure to check those
+    // if you switch.
+    Map<String, String> propertyMap = new ConcurrentHashMap<String, String>();
+    
     // loop through each property in the StyleNode.
     // If 'content', then get the url and the type of icon: 
     // Context, Image, Null, or Text
     // If 'width', then get the width
     // If 'height', then get the height
-    // Build up all the rest of the properties as a CSSStyle object.
+    // Build up all the rest of the properties as an UnmodifiableStyle object.
     // Then create an Icon object. 
     Collection<PropertyNode> properties = resolvedNode.getProperties();
     for (PropertyNode propertyNode : properties)
@@ -325,9 +331,7 @@ public class StyleSheetDocument
           {
             widthValue = null;
             // use inlineStyle for non-integer width values;
-            if (inlineStyle == null)
-              inlineStyle = new CSSStyle();
-            inlineStyle.setProperty(propertyName, propertyValue);
+            propertyMap.put(propertyName, propertyValue);
           }
         }
         else if (propertyName.equals("height"))
@@ -343,9 +347,7 @@ public class StyleSheetDocument
           {
             // use inlineStyle for non-integer height values;
             heightValue = null;
-            if (inlineStyle == null)
-              inlineStyle = new CSSStyle();
-            inlineStyle.setProperty(propertyName, propertyValue);
+            propertyMap.put(propertyName, propertyValue);
           }
         }
         else if (propertyName.equals("content"))
@@ -367,9 +369,7 @@ public class StyleSheetDocument
         else
         {
           // create an inlineStyle with all the extraneous style properties
-          if (inlineStyle == null)
-            inlineStyle = new CSSStyle();
-          inlineStyle.setProperty(propertyName, propertyValue);
+          propertyMap.put(propertyName, propertyValue);
         }
       }
     }
@@ -386,19 +386,20 @@ public class StyleSheetDocument
        // don't allow styleClass from the css parsing file. We can handle
        // this when we have style includes
        // put back the width/height properties if there were some
-       if ((heightValue != null || widthValue != null) && inlineStyle == null)
-         inlineStyle = new CSSStyle();
        if (heightValue != null)
-         inlineStyle.setProperty("height", heightValue);
+         propertyMap.put("height", heightValue);
        if (widthValue != null)
-         inlineStyle.setProperty("width", widthValue);
-       icon = new TextIcon(text, text, null, inlineStyle);
+         propertyMap.put("width", widthValue);
+       icon = new TextIcon(text, text, null, 
+                           propertyMap.isEmpty() ? null : new UnmodifiableStyle(propertyMap));
      }
      else if (uri != null)
      {
        // A URIImageIcon url starts with '/' or 'http:',
        // whereas a ContextImageIcons uri does not. 
        boolean startsWithASlash = uri.startsWith("/");
+      Style inlineStyle = propertyMap.isEmpty() ? null : new UnmodifiableStyle(propertyMap);
+
        if (!startsWithASlash)
        {
          icon =