Forward stderr via Open3#popen3

Forwarding stderr via 2>&1 causes an additional child process to be
spawned.  Killing that process does not kill webkit_server.

JavaScipt console messages and alerts are now written to the logger
instead of directly to stdout.

lib/capybara/webkit/connection.rb

@@ -1,6 +1,7 @@
 require 'socket'
 require 'timeout'
 require 'thread'
+require 'open3'
 
 module Capybara::Webkit
   class Connection
@@ -11,8 +12,7 @@ module Capybara::Webkit
 
     def initialize(options = {})
       @socket_class = options[:socket_class] || TCPSocket
-      @stdout = options.has_key?(:stdout) ?  options[:stdout] : $stdout
-      @command = options[:command] || SERVER_PATH
+      @output_target = options.has_key?(:stdout) ? options[:stdout] : $stdout
       start_server
       connect
     end
@@ -38,12 +38,12 @@ module Capybara::Webkit
     def start_server
       open_pipe
       discover_port
-      forward_stdout_in_background_thread
+      forward_output_in_background_thread
     end
 
     def open_pipe
-      @pipe = IO.popen(@command)
-      @pid = @pipe.pid
+      _, @pipe_stdout, @pipe_stderr, wait_thr = Open3.popen3(SERVER_PATH)
+      @pid = wait_thr[:pid]
       register_shutdown_hook
     end
 
@@ -65,40 +65,19 @@ module Capybara::Webkit
     end
 
     def discover_port
-      if IO.select([@pipe], nil, nil, WEBKIT_SERVER_START_TIMEOUT)
-        @port = ((@pipe.first || '').match(/listening on port: (\d+)/) || [])[1].to_i
+      if IO.select([@pipe_stdout], nil, nil, WEBKIT_SERVER_START_TIMEOUT)
+        @port = ((@pipe_stdout.first || '').match(/listening on port: (\d+)/) || [])[1].to_i
       end
     end
 
-    def forward_stdout_in_background_thread
-      @stdout_thread = Thread.new do
+    def forward_output_in_background_thread
+      Thread.new do
         Thread.current.abort_on_exception = true
-        forward_stdout
+        IO.copy_stream(@pipe_stdout, @output_target)
       end
-    end
-
-    def forward_stdout
-      while pipe_readable?
-        line = @pipe.readline
-        if @stdout
-          @stdout.write(line)
-          @stdout.flush
-        end
-      end
-    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?
+      Thread.new do
+        Thread.current.abort_on_exception = true
+        IO.copy_stream(@pipe_stderr, @output_target)
       end
     end
 

spec/connection_spec.rb

@@ -22,6 +22,8 @@ describe Capybara::Webkit::Connection do
     io = StringIO.new
     redirected_connection = Capybara::Webkit::Connection.new(:stdout => io)
     script = 'console.log("hello world")'
+    redirected_connection.puts "EnableLogging"
+    redirected_connection.puts 0
     redirected_connection.puts "Execute"
     redirected_connection.puts 1
     redirected_connection.puts script.to_s.bytesize

spec/driver_spec.rb

@@ -2042,8 +2042,7 @@ describe Capybara::Webkit::Driver do
     end
 
     let(:driver) do
-      command = "#{Capybara::Webkit::Connection::SERVER_PATH} 2>&1"
-      connection = Capybara::Webkit::Connection.new(:command => command, :stdout => output)
+      connection = Capybara::Webkit::Connection.new(:stdout => output)
       browser = Capybara::Webkit::Browser.new(connection)
       Capybara::Webkit::Driver.new(AppRunner.app, :browser => browser)
     end

spec/spec_helper.rb

@@ -22,7 +22,7 @@ RSpec.configure do |c|
 end
 
 require 'capybara/webkit'
-connection = Capybara::Webkit::Connection.new(:socket_class => TCPSocket, :stdout => nil)
+connection = Capybara::Webkit::Connection.new(:socket_class => TCPSocket)
 $webkit_browser = Capybara::Webkit::Browser.new(connection)
 
 if ENV['DEBUG']

src/WebPage.cpp

@@ -139,13 +139,13 @@ void WebPage::javaScriptConsoleMessage(const QString &message, int lineNumber, c
   if (!sourceID.isEmpty())
     fullMessage = sourceID + "|" + QString::number(lineNumber) + "|" + fullMessage;
   m_consoleMessages.append(fullMessage.replace("\n", "\\n"));
-  std::cout << qPrintable(fullMessage) << std::endl;
+  m_manager->logger() << qPrintable(fullMessage);
 }
 
 void WebPage::javaScriptAlert(QWebFrame *frame, const QString &message) {
   Q_UNUSED(frame);
   m_alertMessages.append(message);
-  std::cout << "ALERT: " << qPrintable(message) << std::endl;
+  m_manager->logger() << "ALERT:" << qPrintable(message);
 }
 
 bool WebPage::javaScriptConfirm(QWebFrame *frame, const QString &message) {