More perf work:

  * Move #set_cookie and #delete_cookie inline to optimize. These optimizations should
    almost certainly be sent back upstream to Rack. The optimization involves using
    an ivar for cookies instead of indexing into the headers each time.
  * Was able to use a bare Hash for headers now that cookies have their own joining
    semantics (some code assumed that the raw cookies were an Array).
  * Cache blankness of body on body=
  * Improve expand_cache_key for Arrays of a single element (common in our case)
  * Use a simple layout condition check unless conditions are used
  * Cache visible actions
  * Lazily load the UrlRewriter
  * Make etag an ivar that is set on prepare!

10 files changed, 113 insertions, 93 deletions

diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb
index d7317b415c..ac2154dffc 100644
--- a/actionpack/lib/abstract_controller/layouts.rb
+++ b/actionpack/lib/abstract_controller/layouts.rb
@@ -6,17 +6,21 @@ module AbstractController
 
     included do
       extlib_inheritable_accessor(:_layout_conditions) { Hash.new }
+      extlib_inheritable_accessor(:_action_has_layout) { Hash.new }
       _write_layout_method
     end
 
     module ClassMethods
       def inherited(klass)
         super
-        klass._write_layout_method
+        klass.class_eval do
+          _write_layout_method
+          @found_layouts = {}
+        end
       end
 
       def cache_layout(details)
-        layout = @found_layouts ||= {}
+        layout = @found_layouts
         values = details.values_at(:formats, :locale)
 
         # Cache nil
@@ -27,6 +31,28 @@ module AbstractController
         end
       end
 
+      # This module is mixed in if layout conditions are provided. This means
+      # that if no layout conditions are used, this method is not used
+      module LayoutConditions
+        # Determines whether the current action has a layout by checking the
+        # action name against the :only and :except conditions set on the
+        # layout.
+        #
+        # ==== Returns
+        # Boolean:: True if the action has a layout, false otherwise.
+        def _action_has_layout?
+          conditions = _layout_conditions
+
+          if only = conditions[:only]
+            only.include?(action_name)
+          elsif except = conditions[:except]
+            !except.include?(action_name)
+          else
+            true
+          end
+        end
+      end
+
       # Specify the layout to use for this class.
       #
       # If the specified layout is a:
@@ -43,6 +69,8 @@ module AbstractController
       # :only<#to_s, Array[#to_s]>:: A list of actions to apply this layout to.
       # :except<#to_s, Array[#to_s]>:: Apply this layout to all actions but this one
       def layout(layout, conditions = {})
+        include LayoutConditions unless conditions.empty?
+
         conditions.each {|k, v| conditions[k] = Array(v).map {|a| a.to_s} }
         self._layout_conditions = conditions
 
@@ -150,7 +178,7 @@ module AbstractController
       view_paths.find(name, details, prefix)
     end
 
-    # Returns the default layout for this controller and a given set of details. 
+    # Returns the default layout for this controller and a given set of details.
     # Optionally raises an exception if the layout could not be found.
     #
     # ==== Parameters
@@ -176,21 +204,8 @@ module AbstractController
       end
     end
 
-    # Determines whether the current action has a layout by checking the
-    # action name against the :only and :except conditions set on the
-    # layout.
-    #
-    # ==== Returns
-    # Boolean:: True if the action has a layout, false otherwise.
     def _action_has_layout?
-      conditions = _layout_conditions
-      if only = conditions[:only]
-        only.include?(action_name)
-      elsif except = conditions[:except]
-        !except.include?(action_name)
-      else
-        true
-      end
+      true
     end
   end
 end
diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index a008b53d45..5b0165f0e7 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -102,11 +102,10 @@ module ActionController
         options[:template].sub!(/^\//, '')
       end
 
-      options[:text] = nil if options[:nothing] == true
+      options[:text] = nil if options.delete(:nothing) == true
+      options[:text] = " " if options.key?(:text) && options[:text].nil?
 
-      body = super
-      body = [' '] if body.blank?
-      body
+      super || " "
     end
 
     def _handle_method_missing
diff --git a/actionpack/lib/action_controller/metal/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb
index af68c772b1..cdacdc40a6 100644
--- a/actionpack/lib/action_controller/metal/hide_actions.rb
+++ b/actionpack/lib/action_controller/metal/hide_actions.rb
@@ -13,7 +13,9 @@ module ActionController
     # Overrides AbstractController::Base#action_method? to return false if the
     # action name is in the list of hidden actions.
     def action_method?(action_name)
-      !hidden_actions.include?(action_name) && super
+      self.class.visible_action?(action_name) do
+        !hidden_actions.include?(action_name) && super
+      end
     end
 
     module ClassMethods
@@ -25,6 +27,16 @@ module ActionController
         hidden_actions.merge(args.map! {|a| a.to_s })
       end
 
+      def inherited(klass)
+        klass.instance_variable_set("@visible_actions", {})
+        super
+      end
+
+      def visible_action?(action_name)
+        return @visible_actions[action_name] if @visible_actions.key?(action_name)
+        @visible_actions[action_name] = yield
+      end
+
       # Overrides AbstractController::Base#action_methods to remove any methods
       # that are listed as hidden methods.
       def action_methods
diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
index 7119c14cd3..14c6523045 100644
--- a/actionpack/lib/action_controller/metal/url_for.rb
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -4,15 +4,6 @@ module ActionController
 
     include RackConvenience
 
-    def process_action(*)
-      initialize_current_url
-      super
-    end
-
-    def initialize_current_url
-      @url = UrlRewriter.new(request, params.clone)
-    end
-
     # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in
     # the form of a hash, just like the one you would use for url_for directly. Example:
     #
@@ -40,6 +31,7 @@ module ActionController
         when String
           options
         when Hash
+          @url ||= UrlRewriter.new(request, params)
           @url.rewrite(rewrite_options(options))
         else
           polymorphic_url(options)
diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb
index 147a7e7631..09b1a59254 100644
--- a/actionpack/lib/action_controller/testing/process.rb
+++ b/actionpack/lib/action_controller/testing/process.rb
@@ -52,7 +52,7 @@ module ActionController #:nodoc:
   class TestResponse < ActionDispatch::TestResponse
     def recycle!
       @status = 200
-      @header = ActionDispatch::Response::SimpleHeaderHash.new
+      @header = {}
       @writer = lambda { |x| @body << x }
       @block = nil
       @length = 0
diff --git a/actionpack/lib/action_controller/testing/test_case.rb b/actionpack/lib/action_controller/testing/test_case.rb
index a11755b517..b66a4c15ff 100644
--- a/actionpack/lib/action_controller/testing/test_case.rb
+++ b/actionpack/lib/action_controller/testing/test_case.rb
@@ -179,7 +179,6 @@ module ActionController
       if @controller
         @controller.request = @request
         @controller.params = {}
-        @controller.send(:initialize_current_url)
       end
     end
 
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 958a541436..b23306af62 100755
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -173,21 +173,16 @@ module ActionDispatch
         end
     end
 
-    # Expand raw_formats by converting Mime::ALL to the Mime::SET.
-    #
     def formats
-      return raw_formats
-      # if ActionController::Base.use_accept_header
-      #   raw_formats.tap do |ret|
-      #     if ret == ONLY_ALL
-      #       ret.replace Mime::SET
-      #     elsif all = ret.index(Mime::ALL)
-      #       ret.delete_at(all) && ret.insert(all, *Mime::SET)
-      #     end
-      #   end
-      # else
-      #   raw_formats + Mime::SET
-      # end
+      if ActionController::Base.use_accept_header
+        if param = parameters[:format]
+          Array.wrap(Mime[param])
+        else
+          accepts.dup
+        end
+      else
+        [format]
+      end
     end
 
     # Sets the \format by string extension, which can be used to force custom formats
@@ -488,7 +483,7 @@ EOM
     # matches the order array.
     #
     def negotiate_mime(order)
-      raw_formats.each do |priority|
+      formats.each do |priority|
         if priority == Mime::ALL
           return order.first
         elsif order.include?(priority)
@@ -501,18 +496,6 @@ EOM
 
     private
 
-      def raw_formats
-        if ActionController::Base.use_accept_header
-          if param = parameters[:format]
-            Array.wrap(Mime[param])
-          else
-            accepts.dup
-          end
-        else
-          [format]
-        end
-      end
-
       def named_host?(host)
         !(host.nil? || /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.match(host))
       end
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 1d3cf63984..055f29a972 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -32,18 +32,7 @@ module ActionDispatch # :nodoc:
   #    end
   #  end
   class Response < Rack::Response
-    class SimpleHeaderHash < Hash
-      def to_hash
-        result = {}
-        each do |k,v|
-          v = v.join("\n") if v.is_a?(Array)
-          result[k] = v
-        end
-        result
-      end
-    end
-
-    attr_accessor :request
+    attr_accessor :request, :blank
     attr_reader :cache_control
 
     attr_writer :header, :sending_file
@@ -51,19 +40,23 @@ module ActionDispatch # :nodoc:
 
     def initialize
       @status = 200
-      @header = SimpleHeaderHash.new
+      @header = {}
       @cache_control = {}
 
       @writer = lambda { |x| @body << x }
       @block = nil
       @length = 0
 
-      @body = []
+      @body, @cookie = [], []
       @sending_file = false
 
       yield self if block_given?
     end
 
+    def cache_control
+      @cache_control ||= {}
+    end
+
     def write(str)
       s = str.to_s
       @writer.call s
@@ -95,7 +88,10 @@ module ActionDispatch # :nodoc:
       str
     end
 
+    EMPTY = " "
+
     def body=(body)
+      @blank = true if body == EMPTY
       @body = body.respond_to?(:to_str) ? [body] : body
     end
 
@@ -137,19 +133,16 @@ module ActionDispatch # :nodoc:
     end
 
     def etag
-      headers['ETag']
+      @etag
     end
 
     def etag?
-      headers.include?('ETag')
+      @etag
     end
 
     def etag=(etag)
-      if etag.blank?
-        headers.delete('ETag')
-      else
-        headers['ETag'] = %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(etag))}")
-      end
+      key = ActiveSupport::Cache.expand_cache_key(etag)
+      @etag = %("#{Digest::MD5.hexdigest(key)}")
     end
 
     CONTENT_TYPE    = "Content-Type"
@@ -157,7 +150,7 @@ module ActionDispatch # :nodoc:
     cattr_accessor(:default_charset) { "utf-8" }
 
     def assign_default_content_type_and_charset!
-      return if !headers[CONTENT_TYPE].blank?
+      return if headers[CONTENT_TYPE].present?
 
       @content_type ||= Mime::HTML
       @charset      ||= self.class.default_charset
@@ -171,7 +164,8 @@ module ActionDispatch # :nodoc:
     def prepare!
       assign_default_content_type_and_charset!
       handle_conditional_get!
-      self["Set-Cookie"] ||= ""
+      self["Set-Cookie"] = @cookie.join("\n")
+      self["ETag"]       = @etag if @etag
     end
 
     def each(&callback)
@@ -197,7 +191,7 @@ module ActionDispatch # :nodoc:
     #   assert_equal 'AuthorOfNewPage', r.cookies['author']
     def cookies
       cookies = {}
-      if header = headers['Set-Cookie']
+      if header = @cookie
         header = header.split("\n") if header.respond_to?(:to_str)
         header.each do |cookie|
           if pair = cookie.split(';').first
@@ -209,9 +203,40 @@ module ActionDispatch # :nodoc:
       cookies
     end
 
+    def set_cookie(key, value)
+      case value
+      when Hash
+        domain  = "; domain="  + value[:domain]    if value[:domain]
+        path    = "; path="    + value[:path]      if value[:path]
+        # According to RFC 2109, we need dashes here.
+        # N.B.: cgi.rb uses spaces...
+        expires = "; expires=" + value[:expires].clone.gmtime.
+          strftime("%a, %d-%b-%Y %H:%M:%S GMT")    if value[:expires]
+        secure = "; secure"  if value[:secure]
+        httponly = "; HttpOnly" if value[:httponly]
+        value = value[:value]
+      end
+      value = [value]  unless Array === value
+      cookie = Rack::Utils.escape(key) + "=" +
+        value.map { |v| Rack::Utils.escape v }.join("&") +
+        "#{domain}#{path}#{expires}#{secure}#{httponly}"
+
+      @cookie << cookie
+    end
+
+    def delete_cookie(key, value={})
+      @cookie.reject! { |cookie|
+        cookie =~ /\A#{Rack::Utils.escape(key)}=/
+      }
+
+      set_cookie(key,
+                 {:value => '', :path => nil, :domain => nil,
+                   :expires => Time.at(0) }.merge(value))
+    end
+
     private
       def handle_conditional_get!
-        if etag? || last_modified? || !cache_control.empty?
+        if etag? || last_modified? || !@cache_control.empty?
           set_conditional_cache_control!
         elsif nonempty_ok_response?
           self.etag = @body
@@ -227,21 +252,18 @@ module ActionDispatch # :nodoc:
         end
       end
 
-      EMPTY_RESPONSE = [" "]
-
       def nonempty_ok_response?
-        ok = !@status || @status == 200
-        ok && string_body? && @body != EMPTY_RESPONSE
+        @status == 200 && string_body?
       end
 
       def string_body?
-        !body_parts.respond_to?(:call) && body_parts.any? && body_parts.all? { |part| part.is_a?(String) }
+        !@blank && @body.respond_to?(:all?) && @body.all? { |part| part.is_a?(String) }
       end
 
       DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate"
 
       def set_conditional_cache_control!
-        control = cache_control
+        control = @cache_control
 
         if control.empty?
           headers["Cache-Control"] = DEFAULT_CACHE_CONTROL
diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index b317b6dc1a..c2ccd1d3a5 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -72,7 +72,6 @@ module ActionView
         @response = ActionController::TestResponse.new
 
         @params = {}
-        send(:initialize_current_url)
       end
     end
 
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 346fa09414..f1c93e6b1e 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -536,7 +536,6 @@ class FragmentCachingTest < ActionController::TestCase
     @controller.params = @params
     @controller.request = @request
     @controller.response = @response
-    @controller.send(:initialize_current_url)
     @controller.send(:initialize_template_class, @response)
     @controller.send(:assign_shortcuts, @request, @response)
   end