Move layout rendering logic to ActionView::Base

4 files changed, 57 insertions, 70 deletions

diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index d12cc41cfa..87361788d1 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -859,7 +859,7 @@ module ActionController #:nodoc:
         raise DoubleRenderError, "Can only render or redirect once per action" if performed?
 
         if options.nil?
-          return render_for_file(default_template_name)
+          return render(:file => default_template_name, :layout => true)
         elsif !extra_options.is_a?(Hash)
           raise RenderError, "You called render with invalid options : #{options.inspect}, #{extra_options.inspect}"
         else
@@ -870,6 +870,8 @@ module ActionController #:nodoc:
           end
         end
 
+        response.layout = layout = pick_layout(options)
+
         if content_type = options[:content_type]
           response.content_type = content_type.to_s
         end
@@ -879,20 +881,21 @@ module ActionController #:nodoc:
         end
 
         if options.has_key?(:text)
-          render_for_text(options[:text], options[:status])
+          text = layout ? @template.render(options.merge(:text => options[:text], :layout => layout)) : options[:text]
+          render_for_text(text, options[:status])
 
         else
           if file = options[:file]
-            render_for_file(file, options[:status], options[:locals] || {})
+            render_for_file(file, options[:status], layout, options[:locals] || {})
 
           elsif template = options[:template]
-            render_for_file(template, options[:status], options[:locals] || {})
+            render_for_file(template, options[:status], layout, options[:locals] || {})
 
           elsif inline = options[:inline]
-            render_for_text(@template.render(options), options[:status])
+            render_for_text(@template.render(options.merge(:layout => layout)), options[:status])
 
           elsif action_name = options[:action]
-            render_for_file(default_template_name(action_name.to_s), options[:status])
+            render_for_file(default_template_name(action_name.to_s), options[:status], layout)
 
           elsif xml = options[:xml]
             response.content_type ||= Mime::XML
@@ -906,7 +909,11 @@ module ActionController #:nodoc:
 
           elsif options[:partial]
             options[:partial] = default_template_name if options[:partial] == true
-            render_for_text(@template.render(options), options[:status])
+            if layout
+              render_for_text(@template.render(:text => @template.render(options), :layout => layout), options[:status])
+            else
+              render_for_text(@template.render(options), options[:status])
+            end
 
           elsif options[:update]
             @template.send! :evaluate_assigns_and_ivars
@@ -919,7 +926,7 @@ module ActionController #:nodoc:
             render_for_text(nil, options[:status])
 
           else
-            render_for_file(default_template_name, options[:status])
+            render_for_file(default_template_name, options[:status], layout)
           end
         end
       end
@@ -1114,9 +1121,9 @@ module ActionController #:nodoc:
 
 
     private
-      def render_for_file(template_path, status = nil, locals = {}) #:nodoc:
+      def render_for_file(template_path, status = nil, layout = nil, locals = {}) #:nodoc:
         logger.info("Rendering #{template_path}" + (status ? " (#{status})" : '')) if logger
-        render_for_text(@template.render(:file => template_path, :locals => locals), status)
+        render_for_text @template.render(:file => template_path, :locals => locals, :layout => layout), status
       end
 
       def render_for_text(text = nil, status = nil, append_response = false) #:nodoc:
diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb
index f3535f8330..3d32519abd 100644
--- a/actionpack/lib/action_controller/caching/actions.rb
+++ b/actionpack/lib/action_controller/caching/actions.rb
@@ -121,7 +121,7 @@ module ActionController #:nodoc:
           end
 
           def content_for_layout(controller)
-            controller.response.layout && controller.response.template.instance_variable_get('@content_for_layout')
+            controller.response.layout && controller.response.template.instance_variable_get('@cached_content_for_layout')
           end
       end
 
diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb
index 585bd03797..ef59c7d63e 100644
--- a/actionpack/lib/action_controller/layout.rb
+++ b/actionpack/lib/action_controller/layout.rb
@@ -3,11 +3,6 @@ module ActionController #:nodoc:
     def self.included(base)
       base.extend(ClassMethods)
       base.class_eval do
-        # NOTE: Can't use alias_method_chain here because +render_without_layout+ is already
-        # defined as a publicly exposed method
-        alias_method :render_with_no_layout, :render
-        alias_method :render, :render_with_a_layout
-
         class << self
           alias_method_chain :inherited, :layout
         end
@@ -240,50 +235,24 @@ module ActionController #:nodoc:
       end
     end
 
-    protected
-      def render_with_a_layout(options = nil, extra_options = {}, &block) #:nodoc:
-        template_with_options = options.is_a?(Hash)
-
-        if (layout = pick_layout(template_with_options, options)) && apply_layout?(template_with_options, options)
-          options.delete(:layout) if template_with_options
-          logger.info("Rendering template within #{layout}") if logger
-
-          content_for_layout = render_with_no_layout(options, extra_options, &block)
-          erase_render_results
-          @template.instance_variable_set("@content_for_layout", content_for_layout)
-          response.layout = layout
-          status = template_with_options ? options[:status] : nil
-          render_for_text(@template.render(layout), status)
-        else
-          render_with_no_layout(options, extra_options, &block)
-        end
-      end
-
-
     private
-      def apply_layout?(template_with_options, options)
-        return false if options == :update
-        template_with_options ?  candidate_for_layout?(options) : !template_exempt_from_layout?
-      end
-
       def candidate_for_layout?(options)
-        (options.has_key?(:layout) && options[:layout] != false) ||
-          options.values_at(:text, :xml, :json, :file, :inline, :partial, :nothing).compact.empty? &&
+        options.values_at(:text, :xml, :json, :file, :inline, :partial, :nothing, :update).compact.empty? &&
           !template_exempt_from_layout?(options[:template] || default_template_name(options[:action]))
       end
 
-      def pick_layout(template_with_options, options)
-        if template_with_options
-          case layout = options[:layout]
-            when FalseClass
-              nil
-            when NilClass, TrueClass
-              active_layout if action_has_layout?
-            else
-              active_layout(layout)
+      def pick_layout(options)
+        if options.has_key?(:layout)
+          case layout = options.delete(:layout)
+          when FalseClass
+            nil
+          when NilClass, TrueClass
+            active_layout if action_has_layout? && !template_exempt_from_layout?(default_template_name)
+          else
+            active_layout(layout)
           end
         else
-          active_layout if action_has_layout?
+          active_layout if action_has_layout? && candidate_for_layout?(options)
         end
       end
 
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index ce629d332a..f1c6f4a7a0 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -246,23 +246,8 @@ module ActionView #:nodoc:
         update_page(&block)
       elsif options.is_a?(Hash)
         options = options.reverse_merge(:locals => {})
-
-        if partial_layout = options.delete(:layout)
-          if block_given?
-            begin
-              @_proc_for_layout = block
-              concat(render(options.merge(:partial => partial_layout)))
-            ensure
-              @_proc_for_layout = nil
-            end
-          else
-            begin
-              original_content_for_layout, @content_for_layout = @content_for_layout, render(options)
-              render(options.merge(:partial => partial_layout))
-            ensure
-              @content_for_layout = original_content_for_layout
-            end
-          end
+        if options[:layout]
+          render_with_layout(options, local_assigns, &block)
         elsif options[:file]
           if options[:use_full_path]
             ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller)
@@ -273,6 +258,8 @@ module ActionView #:nodoc:
           render_partial(options)
         elsif options[:inline]
           InlineTemplate.new(options[:inline], options[:type]).render(self, options[:locals])
+        elsif options[:text]
+          options[:text]
         end
       end
     end
@@ -362,5 +349,29 @@ module ActionView #:nodoc:
           controller.response.content_type ||= content_type
         end
       end
+
+      def render_with_layout(options, local_assigns, &block)
+        partial_layout = options.delete(:layout)
+        if block_given?
+          begin
+            @_proc_for_layout = block
+            concat(render(options.merge(:partial => partial_layout)))
+          ensure
+            @_proc_for_layout = nil
+          end
+        else
+          begin
+            original_content_for_layout, @content_for_layout = @content_for_layout, render(options)
+            if (options[:inline] || options[:file] || options[:text])
+              @cached_content_for_layout = @content_for_layout
+              render(:file => partial_layout, :locals => local_assigns)
+            else
+              render(options.merge(:partial => partial_layout))
+            end
+          ensure
+            @content_for_layout = original_content_for_layout
+          end
+        end
+      end
   end
 end