From eaef841d45c02e40c751e20082004cd38afb595e Mon Sep 17 00:00:00 2001
From: Joe Ferris <jferris@thoughtbot.com>
Date: Sun, 8 Jul 2012 08:39:27 -0700
Subject: [PATCH] General clean up of Connection

* Use instance variables over private methods with arguments
* Use a constant for the server path
* Reorganize methods
---
 lib/capybara/driver/webkit/connection.rb | 89 ++++++++++++------------
 1 file changed, 43 insertions(+), 46 deletions(-)

diff --git a/lib/capybara/driver/webkit/connection.rb b/lib/capybara/driver/webkit/connection.rb
index 97f96fb..366cc84 100644
--- a/lib/capybara/driver/webkit/connection.rb
+++ b/lib/capybara/driver/webkit/connection.rb
@@ -4,15 +4,14 @@ require 'thread'
 
 class Capybara::Driver::Webkit
   class Connection
+    SERVER_PATH = File.expand_path("../../../../../bin/webkit_server", __FILE__)
     WEBKIT_SERVER_START_TIMEOUT = 15
 
     attr_reader :port
 
     def initialize(options = {})
       @socket_class = options[:socket_class] || TCPSocket
-      @stdout       = options.has_key?(:stdout) ?
-                        options[:stdout] :
-                        $stdout
+      @stdout = options.has_key?(:stdout) ?  options[:stdout] : $stdout
       start_server
       connect
     end
@@ -36,52 +35,50 @@ class Capybara::Driver::Webkit
     private
 
     def start_server
-      pipe = fork_server
-      @port = discover_port(pipe)
-      @stdout_thread = Thread.new do
-        Thread.current.abort_on_exception = true
-        forward_stdout(pipe)
-      end
+      open_pipe
+      discover_port
+      forward_stdout_in_background_thread
     end
 
-    def fork_server
-      server_path = File.expand_path("../../../../../bin/webkit_server", __FILE__)
-      pipe, @pid = server_pipe_and_pid(server_path)
+    def open_pipe
+      @pipe = IO.popen(SERVER_PATH)
+      @pid = @pipe.pid
       register_shutdown_hook
-      pipe
-    end
-
-    def kill_process(pid)
-      if RUBY_PLATFORM =~ /mingw32/
-        Process.kill(9, pid)
-      else
-        Process.kill("INT", pid)
-      end
     end
 
     def register_shutdown_hook
       @owner_pid = Process.pid
       at_exit do
         if Process.pid == @owner_pid
-          kill_process(@pid)
+          kill_process
         end
       end
     end
 
-    def server_pipe_and_pid(server_path)
-      cmdline = [server_path]
-      pipe = IO.popen(cmdline.join(" "))
-      [pipe, pipe.pid]
+    def kill_process
+      if RUBY_PLATFORM =~ /mingw32/
+        Process.kill(9, @pid)
+      else
+        Process.kill("INT", @pid)
+      end
     end
 
-    def discover_port(read_pipe)
-      return unless IO.select([read_pipe], nil, nil, WEBKIT_SERVER_START_TIMEOUT)
-      ((read_pipe.first || '').match(/listening on port: (\d+)/) || [])[1].to_i
+    def discover_port
+      if IO.select([@pipe], nil, nil, WEBKIT_SERVER_START_TIMEOUT)
+        @port = ((@pipe.first || '').match(/listening on port: (\d+)/) || [])[1].to_i
+      end
     end
 
-    def forward_stdout(pipe)
-      while pipe_readable?(pipe)
-        line = pipe.readline
+    def forward_stdout_in_background_thread
+      @stdout_thread = Thread.new do
+        Thread.current.abort_on_exception = true
+        forward_stdout
+      end
+    end
+
+    def forward_stdout
+      while pipe_readable?
+        line = @pipe.readline
         if @stdout
           @stdout.write(line)
           @stdout.flush
@@ -90,6 +87,20 @@ class Capybara::Driver::Webkit
     rescue EOFError
     end
 
+    if !defined?(RUBY_ENGINE) || (RUBY_ENGINE == "ruby" && RUBY_VERSION <= "1.8")
+      # please note the use of IO::select() here, as it is used specifically to
+      # preserve correct signal handling behavior in ruby 1.8.
+      # https://github.com/thibaudgg/rb-fsevent/commit/d1a868bf8dc72dbca102bedbadff76c7e6c2dc21
+      # https://github.com/thibaudgg/rb-fsevent/blob/1ca42b987596f350ee7b19d8f8210b7b6ae8766b/ext/fsevent/fsevent_watch.c#L171
+      def pipe_readable?
+        IO.select([@pipe])
+      end
+    else
+      def pipe_readable?
+        !@pipe.eof?
+      end
+    end
+
     def connect
       Timeout.timeout(5) do
         while @socket.nil?
@@ -102,19 +113,5 @@ class Capybara::Driver::Webkit
       @socket = @socket_class.open("127.0.0.1", @port)
     rescue Errno::ECONNREFUSED
     end
-
-    if !defined?(RUBY_ENGINE) || (RUBY_ENGINE == "ruby" && RUBY_VERSION <= "1.8")
-      # please note the use of IO::select() here, as it is used specifically to
-      # preserve correct signal handling behavior in ruby 1.8.
-      # https://github.com/thibaudgg/rb-fsevent/commit/d1a868bf8dc72dbca102bedbadff76c7e6c2dc21
-      # https://github.com/thibaudgg/rb-fsevent/blob/1ca42b987596f350ee7b19d8f8210b7b6ae8766b/ext/fsevent/fsevent_watch.c#L171
-      def pipe_readable?(pipe)
-        IO.select([pipe])
-      end
-    else
-      def pipe_readable?(pipe)
-        !pipe.eof?
-      end
-    end
   end
 end