Misc rework - possibly improved stability

README.md

@@ -77,16 +77,15 @@ port: '12345'         # default '35729'
 apply_css_live: false # default true
 override_url: false   # default false
 grace_period: 0.5     # default 0 (seconds)
-js_template: './my_livereload.js.erb' # default is livereload.js.erb from gem
-```
 
-Additional custom JS template options (see livereload.js.erb for details):
-``` ruby
-js_apple_webkit_extra_wait_time: 50 # default is 5 (see issue #123)
-js_default_extra_wait_time: 100 # default is 200
+# Additional custom JS template options (see livereload.js.erb for details):
+js_template: './my_livereload.js.erb' # default is livereload.js.erb from gem
+js_options: {
+  apple_webkit_extra_wait_time: 50 # default is 5 (see issue #123)
+  default_extra_wait_time: 100 # default is 200
+}
 ```
 
-
 `notify` uses Guard's [system notifications](https://github.com/guard/guard/wiki/System-notifications).
 See [LiveReload configuration doc](https://github.com/mockko/livereload/blob/master/README-old.md) from version 1.x for more info about other options.
 

js/livereload.js.erb

@@ -733,9 +733,9 @@
          return this.waitUntilCssLoads(clone, function() {
            var additionalWaitingTime;
            if (/AppleWebKit/.test(navigator.userAgent)) {
-             additionalWaitingTime = <%= options[:js_apple_webkit_extra_wait_time] || 5 %>;
+             additionalWaitingTime = <%= options[:js_options][:apple_webkit_extra_wait_time] || 5 %>;
            } else {
-             additionalWaitingTime = <%= options[:js_default_extra_wait_time] || 200 %>;
+             additionalWaitingTime = <%= options[:js_options][:default_extra_wait_time] || 200 %>;
            }
            return _this.Timer.start(additionalWaitingTime, function() {
              var _ref;

lib/guard/livereload.rb

@@ -18,9 +18,19 @@ def initialize(options = {})
         apply_css_live: true,
         override_url:   false,
         grace_period:   0,
-        js_template: js_path
+        js_template: js_path,
+        js_options: {}
       }.merge(options)
 
+      %i(js_apple_webkit_extra_wait_time js_default_extra_wait_time).each do |key|
+        if options.keys.include?(key)
+          Guard::Compat::UI.deprecation "#{key.inspect} is deprecated. Check out :js_options parameter instead."
+          new_key = key.to_s.sub(/^js_/, '').to_sym
+          @options[:js_options][new_key] ||= options[key]
+          @options.delete(key)
+        end
+      end
+
       js_path = @options[:js_template]
 
       # NOTE: save snippet as instvar, so it's not GC'ed

lib/guard/livereload/reactor.rb

@@ -1,15 +1,55 @@
 require 'multi_json'
 
+require 'guard/livereload/reactor/sockets'
+
 module Guard
   class LiveReload < Plugin
     class Reactor
-      attr_reader :web_sockets, :thread, :options, :connections_count
+      KNOWN_OPTIONS = %i(
+        notify
+        host
+        port
+        apply_css_live
+        override_url
+        grace_period
+
+        js_template
+        livereload_js_path
+
+        js_options
+      )
+
+      # TODO: implement usage of js_options
+      # TODO: deprecate actual js options (js_apple(...) and js_default(...))
+
+      attr_reader :thread, :options
 
       def initialize(options)
-        @web_sockets       = []
-        @options           = options
-        @thread            = Thread.new { _start_reactor }
-        @connections_count = 0
+        options.keys.each do |key|
+          fail "Unknown option: #{key.inspect}" unless KNOWN_OPTIONS.include?(key)
+        end
+
+        @sockets = Sockets.new
+        @options = options
+        @thread = Thread.new { _start_reactor }
+      end
+
+      def sockets
+        @sockets
+      end
+
+      # TODO: deprecate
+      # just to prevent semver change
+      # (it's used only for tests anyway)
+      def web_sockets
+        sockets.internal_list
+      end
+
+      # TODO: deprecate
+      # just to prevent semver change
+      # (it's used only for tests anyway)
+      def connections_count
+        sockets.size
       end
 
       def stop
@@ -18,6 +58,11 @@ def stop
 
       def reload_browser(paths = [])
         msg = "Reloading browser: #{paths.join(' ')}"
+
+        if sockets.empty?
+          Compat::UI.warning "No browsers connected at this time! Browser won't refresh!"
+        end
+
         Compat::UI.info msg
         if options[:notify]
           Compat::UI.notify(msg, title: 'Reloading browser', image: :success)
@@ -26,7 +71,8 @@ def reload_browser(paths = [])
         paths.each do |path|
           data = _data(path)
           Compat::UI.debug(data)
-          web_sockets.each { |ws| ws.send(MultiJson.encode(data)) }
+          json = MultiJson.encode(data)
+          sockets.broadcast(json)
         end
       end
 
@@ -63,22 +109,25 @@ def _start_reactor
       end
 
       def _connect(ws)
-        @connections_count += 1
-        Compat::UI.info 'Browser connected.' if connections_count == 1
+        Compat::UI.info 'New browser connecting...'
 
-        ws.send MultiJson.encode(
+        json = MultiJson.encode(
           command:    'hello',
           protocols:  ['http://livereload.com/protocols/official-7'],
           serverName: 'guard-livereload'
         )
-        @web_sockets << ws
+
+        sockets.add(ws) { |socket| socket.send(json) }
+
+        Compat::UI.info "New browser connected (total: #{connections_count})"
       rescue
         Compat::UI.error $!
         Compat::UI.error $!.backtrace
       end
 
       def _disconnect(ws)
-        @web_sockets.delete(ws)
+        sockets.delete(ws)
+        Compat::UI.info "Browser disconnected (total: #{connections_count})"
       end
 
       def _print_message(message)

lib/guard/livereload/reactor/sockets.rb

@@ -0,0 +1,43 @@
+require 'thread'
+
+module Guard
+  class LiveReload < Plugin
+    class Reactor
+      class Sockets
+        def initialize
+          @sockets = []
+          @mutex = Mutex.new
+        end
+
+        def internal_list
+          @mutex.synchronize { @sockets }
+        end
+
+        def size
+          @mutex.synchronize { @sockets.size }
+        end
+
+        def broadcast(json)
+          @mutex.synchronize do
+            @sockets.each { |ws| ws.send(json) }
+          end
+        end
+
+        def add(socket)
+          @mutex.synchronize do
+            yield socket if block_given? # within mutex, so 'hello' has a chance to be sent
+            @sockets << socket
+          end
+        end
+
+        def delete(socket)
+          @mutex.synchronize { @sockets.delete(socket) }
+        end
+
+        def empty?
+          @mutex.synchronize { @sockets.empty? }
+        end
+      end
+    end
+  end
+end

lib/guard/livereload/websocket.rb

@@ -14,6 +14,8 @@ def initialize(options)
       def dispatch(data)
         parser = Http::Parser.new
         parser << data
+        UI.debug "Request: #{parser.request_url}"
+        UI.debug "Headers: #{parser.headers.inspect}"
         # prepend with '.' to make request url usable as a file path
         request_path = '.' + URI.parse(parser.request_url).path
         request_path += '/index.html' if File.directory? request_path

spec/lib/guard/livereload/reactor_spec.rb

@@ -1,14 +1,23 @@
 RSpec.describe Guard::LiveReload::Reactor do
   let(:paths) { %w[stylesheets/layout.css stylesheets/style.css] }
 
+  let(:sockets) { instance_double(described_class::Sockets) }
+
   before do
     allow(Guard::Compat::UI).to receive(:info)
     allow(Guard::Compat::UI).to receive(:debug)
     allow(Guard::Compat::UI).to receive(:error)
     allow(Guard::Compat::UI).to receive(:warning)
+
+    allow(described_class::Sockets).to receive(:new).and_return(sockets)
   end
 
   describe '#reload_browser(paths = [])' do
+    before do
+      allow(sockets).to receive(:broadcast)
+      allow(sockets).to receive(:empty?).and_return(false)
+    end
+
     it 'displays a message' do
       expect(Guard::Compat::UI).to receive(:info).
         with('Reloading browser: stylesheets/layout.css stylesheets/style.css')
@@ -26,46 +35,89 @@
       new_live_reactor(notify: true).reload_browser(paths)
     end
 
-    it 'each web socket receives send with data containing default options for each path modified' do
-      reactor = new_live_reactor
-      paths.each do |path|
-        reactor.web_sockets.each do |ws|
-          expect(ws).to receive(:send).with(MultiJson.encode(['refresh', path: "#{Dir.pwd}/#{path}", apply_js_live: true, apply_css_live: true]))
+    context 'with multiple paths' do
+      subject { new_live_reactor(*args) }
+
+      let(:paths) { %w(foo.css bar.css) }
+
+      context 'with default options' do
+        let(:options) { { apply_css_live: false } }
+        let(:args) { [] }
+
+        it 'sends json data with options' do
+          paths.each do |path|
+            json = MultiJson.encode('command': 'reload', path: "#{Dir.pwd}/#{path}", liveCSS: true)
+            expect(sockets).to receive(:broadcast).with(json)
+          end
+          subject.reload_browser(paths)
         end
       end
-      reactor.reload_browser(paths)
-    end
 
-    it 'each web socket receives send with data containing custom options for each path modified' do
-      reactor = new_live_reactor(apply_css_live: false, apply_js_live: false)
-      paths.each do |path|
-        reactor.web_sockets.each do |ws|
-          expect(ws).to receive(:send).with(MultiJson.encode(['refresh', path: "#{Dir.pwd}/#{path}", apply_js_live: false, apply_css_live: false]))
+      context 'with custom options' do
+        let(:options) { { apply_css_live: false } }
+        let(:args) { [options] }
+
+        it 'sends json data with custom options' do
+          paths.each do |path|
+            json = MultiJson.encode('command': 'reload', path: "#{Dir.pwd}/#{path}", liveCSS: false)
+            expect(sockets).to receive(:broadcast).with(json)
+          end
+          subject.reload_browser(paths)
         end
       end
-      reactor.reload_browser(paths)
+
+    end
+
+    context 'with no connected browsers' do
+      subject { new_live_reactor }
+
+      before do
+        allow(sockets).to receive(:empty?).and_return(true)
+      end
+
+      it 'shows a warning' do
+        expect(Guard::Compat::UI).to receive(:warning).with(/No browsers connected at this time!/)
+        subject.reload_browser(paths)
+      end
     end
   end
 
   describe '#_connect(ws)' do
     let(:ws)      { double.as_null_object }
     let(:reactor) { new_live_reactor }
+    let!(:socket_list) { [] }
+
+    before do
+      allow(sockets).to receive(:add).with(ws) do |socket|
+        socket_list << socket
+      end
 
-    it 'displays a message once' do
-      expect(Guard::Compat::UI).to receive(:info).with('Browser connected.').once
-      reactor.send(:_connect, ws)
-      reactor.send(:_connect, ws)
+      allow(sockets).to receive(:size) { socket_list.size }
     end
 
-    it 'increments the connection count' do
-      expect do
+    context 'when connected once' do
+      it 'displays a message once' do
+        expect(Guard::Compat::UI).to receive(:info).with('New browser connecting...')
+        expect(Guard::Compat::UI).to receive(:info).with('New browser connected (total: 1)')
         reactor.send(:_connect, ws)
-      end.to change { reactor.connections_count }.from(0).to 1
+      end
     end
-  end
 
+    context 'when already connected' do
+      before do
+        allow(Guard::Compat::UI).to receive(:info)
+        reactor.send(:_connect, ws)
+      end
+
+      it 'displays a message with total' do
+        expect(Guard::Compat::UI).to receive(:info).with('New browser connecting...')
+        expect(Guard::Compat::UI).to receive(:info).with('New browser connected (total: 2)')
+        reactor.send(:_connect, ws)
+      end
+    end
+  end
 end
 
 def new_live_reactor(options = {})
-  Guard::LiveReload::Reactor.new({ api_version: '1.6', host: '0.0.0.0', port: '35729', apply_js_live: true, apply_css_live: true, grace_period: 0 }.merge(options))
+  Guard::LiveReload::Reactor.new({ host: '0.0.0.0', port: '35729', apply_css_live: true, grace_period: 0 }.merge(options))
 end