From dc43d51ebb6283409040d6ddc4eb995f1b126218 Mon Sep 17 00:00:00 2001
From: Thomas Walpole <twalpole@gmail.com>
Date: Sun, 10 Feb 2019 23:40:09 -0800
Subject: [PATCH] Simplify page management and attach to pages as early as
 possible

---
 lib/capybara/apparition/browser.rb            | 154 +++++++++++-------
 lib/capybara/apparition/browser/header.rb     |   4 +-
 lib/capybara/apparition/browser/window.rb     |  39 ++---
 .../apparition/dev_tools_protocol/session.rb  |   5 +-
 .../apparition/dev_tools_protocol/target.rb   |  68 --------
 .../dev_tools_protocol/target_manager.rb      |  48 ------
 lib/capybara/apparition/driver.rb             |  16 +-
 lib/capybara/apparition/page.rb               |  58 ++++---
 lib/capybara/apparition/page/frame.rb         |   3 +
 lib/capybara/apparition/page/frame_manager.rb |   2 +-
 spec/capybara-webkit/driver_spec.rb           |   2 -
 11 files changed, 169 insertions(+), 230 deletions(-)
 delete mode 100644 lib/capybara/apparition/dev_tools_protocol/target.rb
 delete mode 100644 lib/capybara/apparition/dev_tools_protocol/target_manager.rb

diff --git a/lib/capybara/apparition/browser.rb b/lib/capybara/apparition/browser.rb
index 279c3f2..10b8f92 100644
--- a/lib/capybara/apparition/browser.rb
+++ b/lib/capybara/apparition/browser.rb
@@ -1,9 +1,9 @@
 # frozen_string_literal: true
 
 require 'capybara/apparition/errors'
-require 'capybara/apparition/dev_tools_protocol/target_manager'
 require 'capybara/apparition/page'
 require 'capybara/apparition/console'
+require 'capybara/apparition/dev_tools_protocol/session'
 require 'capybara/apparition/browser/header'
 require 'capybara/apparition/browser/window'
 require 'capybara/apparition/browser/render'
@@ -30,7 +30,7 @@ class Browser
     def initialize(client, logger = nil)
       @client = client
       @current_page_handle = nil
-      @targets = Capybara::Apparition::DevToolsProtocol::TargetManager.new(self)
+      @pages = {}
       @context_id = nil
       @js_errors = true
       @ignore_https_errors = false
@@ -41,11 +41,8 @@ def initialize(client, logger = nil)
       initialize_handlers
 
       command('Target.setDiscoverTargets', discover: true)
-      while @current_page_handle.nil?
-        puts 'waiting for target...'
-        sleep 0.1
-      end
-      @context_id = current_target.context_id
+      yield self if block_given?
+      reset
     end
 
     def restart
@@ -81,24 +78,31 @@ def click_coordinates(x, y)
     include Auth
 
     def reset
-      current_page_targets = @targets.of_type('page').values
-
       new_context_id = command('Target.createBrowserContext')['browserContextId']
-      new_target_response = client.send_cmd('Target.createTarget', url: 'about:blank', browserContextId: new_context_id)
+      current_pages = @pages.keys
 
-      current_page_targets.each do |target|
+      new_target_response = client.send_cmd('Target.createTarget', url: 'about:blank', browserContextId: new_context_id)
+      @pages.each do |id, page|
         begin
-          client.send_cmd('Target.disposeBrowserContext', browserContextId: target.context_id).discard_result
+          client.send_cmd('Target.disposeBrowserContext', browserContextId: page.browser_context_id).discard_result
         rescue WrongWorld
           puts 'Unknown browserContextId'
         end
-        @targets.delete(target.id)
+        @pages.delete(id)
       end
 
       new_target_id = new_target_response['targetId']
 
+      session_id = command('Target.attachToTarget', targetId: new_target_id)['sessionId']
+      session = Capybara::Apparition::DevToolsProtocol::Session.new(self, client, session_id)
+
+      @pages[new_target_id] = Page.create(self, session, new_target_id, new_context_id, ignore_https_errors: ignore_https_errors,
+                                          js_errors: js_errors, extensions: @extensions,
+                                          url_blacklist: @url_blacklist, url_whitelist: @url_whitelist) # .inherit(@info.delete('inherit'))
+      @pages[new_target_id].send(:main_frame).loaded!
+
       timer = Capybara::Helpers.timer(expire_in: 10)
-      until @targets.get(new_target_id)&.page&.usable?
+      until @pages[new_target_id].usable?
         if timer.expired?
           puts 'Timedout waiting for reset'
           raise TimeoutError.new('reset')
@@ -110,6 +114,45 @@ def reset
       true
     end
 
+    def refresh_pages(opener:)
+      new_pages = command('Target.getTargets')['targetInfos'].select do |ti|
+        (ti['openerId'] == opener.target_id) && (ti['type'] == 'page') && (ti['attached'] == false)
+      end
+      sessions = new_pages.map do |page|
+        target_id = page['targetId']
+        session_result = client.send_cmd('Target.attachToTarget', targetId: target_id)
+        [target_id, session_result]
+      end
+
+      sessions = sessions.map do |(target_id, session_result)|
+        session = Capybara::Apparition::DevToolsProtocol::Session.new(self, client, session_result.result['sessionId'])
+        [target_id, session]
+      end
+
+      sessions.each do |(target_id, session)|
+        session.async_commands 'Page.enable', 'Network.enable', 'Runtime.enable', 'Security.enable', 'DOM.enable'
+      end
+
+      # sessions.each do |(target_id, session_result)|
+      #   session = Capybara::Apparition::DevToolsProtocol::Session.new(self, client, session_result.result['sessionId'])
+      sessions.each do |(target_id, session)|
+        page_options = { ignore_https_errors: ignore_https_errors, js_errors: js_errors,
+                       url_blacklist: @url_blacklist, url_whitelist: @url_whitelist }
+        new_page = Page.create(self, session, target_id, opener.browser_context_id, page_options).inherit(opener)
+        @pages[target_id] = new_page
+      end
+
+      # new_pages.each do |page|
+      #   target_id = page['targetId']
+      #   session_id = command('Target.attachToTarget', targetId: target_id)['sessionId']
+      #   session = Capybara::Apparition::DevToolsProtocol::Session.new(self, client, session_id)
+      #   page_options = { ignore_https_errors: ignore_https_errors, js_errors: js_errors,
+      #                  url_blacklist: @url_blacklist, url_whitelist: @url_whitelist }
+      #   new_page = Page.create(self, session, page['targetId'], opener.browser_context_id, page_options).inherit(opener)
+      #   @pages[target_id] = new_page
+      # end
+    end
+
     def resize(width, height, screen: nil)
       current_page.set_viewport width: width, height: height, screen: screen
     end
@@ -128,20 +171,22 @@ def network_traffic(type = nil)
     def extensions=(filenames)
       @extensions = filenames
       Array(filenames).each do |name|
-        begin
-          current_page.command('Page.addScriptToEvaluateOnNewDocument', source: File.read(name))
-        rescue Errno::ENOENT
-          raise ::Capybara::Apparition::BrowserError.new('name' => "Unable to load extension: #{name}", 'args' => nil)
-        end
+        current_page(allow_nil: true)&.add_extension(name)
       end
     end
 
     def url_whitelist=(whitelist)
-      current_page&.url_whitelist = whitelist
+      @url_whitelist = whitelist
+      @pages.each do |_id, page|
+        page.url_whitelist = whitelist
+      end
     end
 
     def url_blacklist=(blacklist)
-      current_page&.url_blacklist = blacklist
+      @url_blacklist = blacklist
+      @pages.each do |_id, page|
+        page.url_blacklist = blacklist
+      end
     end
 
     attr_writer :debug
@@ -167,8 +212,13 @@ def command_for_session(session_id, name, params)
       raise
     end
 
-    def current_page
-      current_target.page
+    def current_page(allow_nil: false)
+      @pages[@current_page_handle] || begin
+        puts "No current page: #{@current_page_handle} : #{caller}" if ENV['DEBUG']
+        @current_page_handle = nil
+        raise NoSuchWindowError unless allow_nil
+        @current_page_handle
+      end
     end
 
     def console_messages(type = nil)
@@ -177,15 +227,6 @@ def console_messages(type = nil)
 
   private
 
-    def current_target
-      @targets.get(@current_page_handle) || begin
-        puts "No current page: #{@current_page_handle}"
-        puts caller
-        @current_page_handle = nil
-        raise NoSuchWindowError
-      end
-    end
-
     def log(message)
       @logger&.puts message if ENV['DEBUG']
     end
@@ -257,35 +298,36 @@ def log(message)
     # end
 
     def initialize_handlers
-      @client.on 'Target.targetCreated' do |info|
-        puts "Target Created Info: #{info}" if ENV['DEBUG']
-        target_info = info['targetInfo']
-        if !@targets.target?(target_info['targetId'])
-          # @targets.add(target_info['targetId'], DevToolsProtocol::Target.new(self, target_info))
-          @targets.add(target_info['targetId'], target_info)
-          puts "**** Target Added #{info}" if ENV['DEBUG']
-        elsif ENV['DEBUG']
-          puts "Target already existed #{info}"
-        end
-        @current_page_handle ||= target_info['targetId'] if target_info['type'] == 'page'
-      end
+      # @client.on 'Target.targetCreated' do |info|
+      #   byebug
+      #   puts "Target Created Info: #{info}" if ENV['DEBUG']
+      #   target_info = info['targetInfo']
+      #   if !@pages.key?(target_info['targetId'])
+      #     @pages.add(target_info['targetId'], target_info)
+      #     puts "**** Target Added #{info}" if ENV['DEBUG']
+      #   elsif ENV['DEBUG']
+      #     puts "Target already existed #{info}"
+      #   end
+      #   @current_page_handle ||= target_info['targetId'] if target_info['type'] == 'page'
+      # end
 
       @client.on 'Target.targetDestroyed' do |info|
         puts "**** Target Destroyed Info: #{info}" if ENV['DEBUG']
-        @targets.delete(info['targetId'])
+        @pages.delete(info['targetId'])
       end
 
-      @client.on 'Target.targetInfoChanged' do |info|
-        puts "**** Target Info Changed: #{info}" if ENV['DEBUG']
-        target_info = info['targetInfo']
-        target = @targets.get(target_info['targetId'])
-        if target
-          target.update(target_info)
-        else
-          puts '****No target for the info change- creating****' if ENV['DEBUG']
-          @targets.add(target_info['targetId'], target_info)
-        end
-      end
+      # @client.on 'Target.targetInfoChanged' do |info|
+      #   byebug
+      #   puts "**** Target Info Changed: #{info}" if ENV['DEBUG']
+      #   target_info = info['targetInfo']
+      #   page = @pages[target_info['targetId']]
+      #   if page
+      #     page.update(target_info)
+      #   else
+      #     puts '****No target for the info change- creating****' if ENV['DEBUG']
+      #     @pages.add(target_info['targetId'], target_info)
+      #   end
+      # end
     end
   end
 end
diff --git a/lib/capybara/apparition/browser/header.rb b/lib/capybara/apparition/browser/header.rb
index c728838..2d255c0 100644
--- a/lib/capybara/apparition/browser/header.rb
+++ b/lib/capybara/apparition/browser/header.rb
@@ -8,7 +8,7 @@ def headers
       end
 
       def headers=(headers)
-        @targets.pages.each do |page|
+        @pages.each do |_id, page|
           page.perm_headers = headers.dup
           page.temp_headers = {}
           page.temp_no_redirect_headers = {}
@@ -23,7 +23,7 @@ def add_headers(headers)
 
       def add_header(header, permanent: true, **_options)
         if permanent == true
-          @targets.pages.each do |page|
+          @pages.each do |_id, page|
             page.perm_headers.merge! header
             page.update_headers
           end
diff --git a/lib/capybara/apparition/browser/window.rb b/lib/capybara/apparition/browser/window.rb
index a4973b2..31e694d 100644
--- a/lib/capybara/apparition/browser/window.rb
+++ b/lib/capybara/apparition/browser/window.rb
@@ -8,42 +8,39 @@ def current_window_handle
       end
 
       def window_handles
-        @targets.window_handles
+        @pages.keys
       end
 
       def switch_to_window(handle)
-        target = @targets.get(handle)
-        unless target&.page
-          target = @targets.get(find_window_handle(handle))
+        page = @pages[handle]
+        unless page
+          page = @pages[find_window_handle(handle)]
           warn 'Finding window by name, title, or url is deprecated, please use a block/proc ' \
                'with Session#within_window/Session#switch_to_window instead.'
         end
-        raise NoSuchWindowError unless target&.page
+        raise NoSuchWindowError unless page
 
-        target.page.wait_for_loaded
-        @current_page_handle = target.id
+        page.wait_for_loaded
+        @current_page_handle = page.target_id
       end
 
       def open_new_window
-        context_id = current_target.context_id
-        info = command('Target.createTarget', url: 'about:blank', browserContextId: context_id)
-        target_id = info['targetId']
-        target = DevToolsProtocol::Target.new(self, info.merge('type' => 'page', 'inherit' => current_page))
-        target.page # Ensure page object construction happens
-        begin
-          puts "Adding #{target_id} - #{target.info}" if ENV['DEBUG']
-          @targets.add(target_id, target)
-        rescue ArgumentError
-          puts 'Target already existed' if ENV['DEBUG']
-        end
+        context_id = current_page.browser_context_id
+        target_id = command('Target.createTarget', url: 'about:blank', browserContextId: context_id)['targetId']
+        session_id = command('Target.attachToTarget', targetId: target_id)['sessionId']
+        session = Capybara::Apparition::DevToolsProtocol::Session.new(self, client, session_id)
+        @pages[target_id] = Page.create(self, session, target_id, context_id, ignore_https_errors: ignore_https_errors, js_errors: js_errors,
+                                        url_whitelist: @url_whitelist, extensions: @extensions, url_blacklist: @url_blacklist).inherit(current_page(allow_nil: true))
+        @pages[target_id].send(:main_frame).loaded!
         target_id
       end
 
       def close_window(handle)
         @current_page_handle = nil if @current_page_handle == handle
-        win_target = @targets.delete(handle)
-        warn 'Window was already closed unexpectedly' if win_target.nil?
-        win_target&.close
+        page = @pages.delete(handle)
+
+        warn 'Window was already closed unexpectedly' unless page
+        command('Target.closeTarget', targetId: handle)
       end
     end
 
diff --git a/lib/capybara/apparition/dev_tools_protocol/session.rb b/lib/capybara/apparition/dev_tools_protocol/session.rb
index f66563f..a779bc2 100644
--- a/lib/capybara/apparition/dev_tools_protocol/session.rb
+++ b/lib/capybara/apparition/dev_tools_protocol/session.rb
@@ -3,12 +3,11 @@
 module Capybara::Apparition
   module DevToolsProtocol
     class Session
-      attr_reader :browser, :connection, :target_id, :session_id
+      attr_reader :browser, :connection, :session_id
 
-      def initialize(browser, connection, target_id, session_id)
+      def initialize(browser, connection, session_id)
         @browser = browser
         @connection = connection
-        @target_id = target_id
         @session_id = session_id
         @handlers = []
       end
diff --git a/lib/capybara/apparition/dev_tools_protocol/target.rb b/lib/capybara/apparition/dev_tools_protocol/target.rb
deleted file mode 100644
index afda77f..0000000
--- a/lib/capybara/apparition/dev_tools_protocol/target.rb
+++ /dev/null
@@ -1,68 +0,0 @@
-# frozen_string_literal: true
-
-require 'capybara/apparition/dev_tools_protocol/session'
-
-module Capybara::Apparition
-  module DevToolsProtocol
-    class Target
-      def initialize(browser, info)
-        @browser = browser
-        @info = info.dup
-        @page = nil
-      end
-
-      def info
-        @info.dup.freeze
-      end
-
-      def update(new_info)
-        @info ||= {}
-        @info.merge!(new_info)
-        info
-      end
-
-      def id
-        @info['targetId']
-      end
-
-      def context_id
-        @info['browserContextId']
-      end
-
-      def type
-        @info['type']
-      end
-
-      def title
-        @info['title']
-      end
-
-      def url
-        @info['url']
-      end
-
-      def page
-        @page ||= begin
-          if info['type'] == 'page'
-            Page.create(@browser, create_session, id,
-                        ignore_https_errors: @browser.ignore_https_errors,
-                        js_errors: @browser.js_errors).inherit(@info.delete('inherit'))
-          else
-            nil
-          end
-        end
-      end
-
-      def close
-        @browser.command('Target.closeTarget', targetId: id)
-      end
-
-    private
-
-      def create_session
-        session_id = @browser.command('Target.attachToTarget', targetId: id)['sessionId']
-        Session.new(@browser, @browser.client, id, session_id)
-      end
-    end
-  end
-end
diff --git a/lib/capybara/apparition/dev_tools_protocol/target_manager.rb b/lib/capybara/apparition/dev_tools_protocol/target_manager.rb
deleted file mode 100644
index 2341f0a..0000000
--- a/lib/capybara/apparition/dev_tools_protocol/target_manager.rb
+++ /dev/null
@@ -1,48 +0,0 @@
-# frozen_string_literal: true
-
-require 'capybara/apparition/dev_tools_protocol/target'
-
-module Capybara::Apparition
-  module DevToolsProtocol
-    class TargetManager
-      def initialize(browser)
-        @browser = browser
-        @targets = {}
-      end
-
-      def get(id)
-        @targets[id]
-      end
-
-      def of_type(type)
-        @targets.select { |_id, target| target.type == type }
-      end
-
-      def add(id, target)
-        raise ArgumentError, 'Target already exists' if @targets.key?(id)
-
-        @targets[id] = if target.is_a? DevToolsProtocol::Target
-          target
-        else
-          DevToolsProtocol::Target.new(@browser, target)
-        end
-      end
-
-      def delete(id)
-        @targets.delete(id)
-      end
-
-      def pages
-        @targets.values.select { |target| target.type == 'page' }.map(&:page)
-      end
-
-      def target?(id)
-        @targets.key?(id)
-      end
-
-      def window_handles
-        @targets.values.select { |target| target.type == 'page' }.map(&:id).compact
-      end
-    end
-  end
-end
diff --git a/lib/capybara/apparition/driver.rb b/lib/capybara/apparition/driver.rb
index 735cae7..84daea4 100644
--- a/lib/capybara/apparition/driver.rb
+++ b/lib/capybara/apparition/driver.rb
@@ -46,14 +46,14 @@ def needs_server?
 
     def browser
       @browser ||= begin
-        browser = Browser.new(client, browser_logger)
-        browser.js_errors = options.fetch(:js_errors, true)
-        browser.ignore_https_errors = options.fetch(:ignore_https_errors, false)
-        browser.extensions = options.fetch(:extensions, [])
-        browser.debug      = options.fetch(:debug, false)
-        browser.url_blacklist = options[:url_blacklist] || []
-        browser.url_whitelist = options[:url_whitelist] || []
-        browser
+        Browser.new(client, browser_logger) do |browser|
+          browser.js_errors = options.fetch(:js_errors, true)
+          browser.ignore_https_errors = options.fetch(:ignore_https_errors, false)
+          browser.extensions = options.fetch(:extensions, [])
+          browser.debug      = options.fetch(:debug, false)
+          browser.url_blacklist = options[:url_blacklist] || []
+          browser.url_whitelist = options[:url_whitelist] || []
+        end
       end
     end
 
diff --git a/lib/capybara/apparition/page.rb b/lib/capybara/apparition/page.rb
index 8a5ccc4..df7ad66 100644
--- a/lib/capybara/apparition/page.rb
+++ b/lib/capybara/apparition/page.rb
@@ -10,38 +10,43 @@ class Page
     attr_reader :modal_messages
     attr_reader :mouse, :keyboard
     attr_reader :viewport_size
+    attr_reader :browser_context_id
     attr_accessor :perm_headers, :temp_headers, :temp_no_redirect_headers
     attr_reader :network_traffic
+    attr_reader :target_id
 
-    def self.create(browser, session, id, ignore_https_errors: false, screenshot_task_queue: nil, js_errors: false)
-      session.command 'Page.enable'
+    def self.create(browser, session, id, browser_context_id,
+                    ignore_https_errors: false, **options)
+      session.async_command 'Page.enable'
 
       # Provides a lot of info - but huge overhead
       # session.command 'Page.setLifecycleEventsEnabled', enabled: true
 
-      page = Page.new(browser, session, id, ignore_https_errors, screenshot_task_queue, js_errors)
+      page = Page.new(browser, session, id, browser_context_id, options)
 
       session.async_commands 'Network.enable', 'Runtime.enable', 'Security.enable', 'DOM.enable'
-      session.command 'Security.setIgnoreCertificateErrors', ignore: !!ignore_https_errors
+      session.async_command 'Security.setIgnoreCertificateErrors', ignore: !!ignore_https_errors
       if Capybara.save_path
-        session.command 'Page.setDownloadBehavior', behavior: 'allow', downloadPath: Capybara.save_path
+        session.async_command 'Page.setDownloadBehavior', behavior: 'allow', downloadPath: Capybara.save_path
       end
       page
     end
 
-    def initialize(browser, session, id, _ignore_https_errors, _screenshot_task_queue, js_errors)
-      @target_id = id
+    def initialize(browser, session, target_id, browser_context_id,
+                   js_errors: false, url_blacklist: [], url_whitelist: [], extensions: [])
+      @target_id = target_id
+      @browser_context_id = browser_context_id
       @browser = browser
       @session = session
       @keyboard = Keyboard.new(self)
       @mouse = Mouse.new(self, @keyboard)
       @modals = []
       @modal_messages = []
-      @frames = Capybara::Apparition::FrameManager.new(id)
+      @frames = Capybara::Apparition::FrameManager.new(@target_id)
       @response_headers = {}
       @status_code = 0
-      @url_blacklist = []
-      @url_whitelist = []
+      @url_blacklist = url_blacklist || []
+      @url_whitelist = url_whitelist || []
       @credentials = nil
       @auth_attempts = []
       @proxy_credentials = nil
@@ -61,6 +66,10 @@ def initialize(browser, session, id, _ignore_https_errors, _screenshot_task_queu
 
       register_js_error_handler # if js_errors
 
+      extensions.each do |name|
+        add_extension(name)
+      end
+
       setup_network_interception if browser.proxy_auth
     end
 
@@ -78,6 +87,12 @@ def reset
       @perm_headers = {}
     end
 
+    def add_extension(filename)
+      command('Page.addScriptToEvaluateOnNewDocument', source: File.read(filename))
+    rescue Errno::ENOENT
+      raise ::Capybara::Apparition::BrowserError.new('name' => "Unable to load extension: #{filename}", 'args' => nil)
+    end
+
     def add_modal(modal_response)
       @last_modal_message = nil
       @modals.push(modal_response)
@@ -287,7 +302,7 @@ def frame_url
     end
 
     def set_viewport(width:, height:, screen: nil)
-      wait_for_loaded
+      # wait_for_loaded
       @viewport_size = { width: width, height: height }
       result = @browser.command('Browser.getWindowForTarget', targetId: @target_id)
       begin
@@ -373,6 +388,14 @@ def js_error
 
     attr_reader :url_blacklist, :url_whitelist
 
+    def current_frame
+      @frames.current
+    end
+
+    def main_frame
+      @frames.main
+    end
+
   private
 
     def eval_wrapped_script(wrapper, script, args)
@@ -416,8 +439,7 @@ def register_event_handlers
 
       @session.on 'Page.windowOpen' do |params|
         puts "**** windowOpen was called with: #{params}" if ENV['DEBUG']
-        # TODO: find a better way to handle this
-        sleep 0.4 # wait a bit so the window has time to start loading
+        @browser.refresh_pages(opener: self)
       end
 
       @session.on 'Page.frameAttached' do |params|
@@ -441,10 +463,12 @@ def register_event_handlers
       end
 
       @session.on 'Page.frameStartedLoading' do |params|
+        puts "Setting loading for #{params['frameId']}" if ENV['DEBUG']
         @frames.get(params['frameId'])&.loading(-1)
       end
 
       @session.on 'Page.frameStoppedLoading' do |params|
+        puts "Setting loaded for #{params['frameId']}" if ENV['DEBUG']
         @frames.get(params['frameId'])&.loaded!
       end
 
@@ -618,14 +642,6 @@ def block_request(id, reason)
       async_command 'Network.continueInterceptedRequest', errorReason: reason, interceptionId: id
     end
 
-    def current_frame
-      @frames.current
-    end
-
-    def main_frame
-      @frames.main
-    end
-
     def go_history(delta)
       history = command('Page.getNavigationHistory')
       entry = history['entries'][history['currentIndex'] + delta]
diff --git a/lib/capybara/apparition/page/frame.rb b/lib/capybara/apparition/page/frame.rb
index 406794c..e074a4f 100644
--- a/lib/capybara/apparition/page/frame.rb
+++ b/lib/capybara/apparition/page/frame.rb
@@ -47,10 +47,12 @@ def loader_id
     end
 
     def loading(id)
+      puts "Setting loading to #{id}" if ENV['DEBUG']
       self.loader_id = id
     end
 
     def reloading!
+      puts "Reloading" if ENV['DEBUG']
       self.loader_id = @prev_loader_id
     end
 
@@ -64,6 +66,7 @@ def loaded?
 
     def loaded!
       @prev_loader_id = loader_id
+      puts "Setting loaded - was #{loader_id}" if ENV['DEBUG']
       self.loader_id = nil
     end
 
diff --git a/lib/capybara/apparition/page/frame_manager.rb b/lib/capybara/apparition/page/frame_manager.rb
index aed0e57..226ce70 100644
--- a/lib/capybara/apparition/page/frame_manager.rb
+++ b/lib/capybara/apparition/page/frame_manager.rb
@@ -7,7 +7,7 @@ class FrameManager
     def initialize(id)
       @frames = {}
       @frames_mutex = Mutex.new
-      add(id)
+      add(id).loading(-1)
       @main_id = @current_id = id
     end
 
diff --git a/spec/capybara-webkit/driver_spec.rb b/spec/capybara-webkit/driver_spec.rb
index 72d9e1d..05f42c6 100644
--- a/spec/capybara-webkit/driver_spec.rb
+++ b/spec/capybara-webkit/driver_spec.rb
@@ -2284,7 +2284,6 @@ def which_for(character)
     end
 
     it 'waits for the new window to load' do
-      pending 'Need to fix this'
       visit('/new_window?sleep=1')
       driver.within_window(driver.window_handles.last) do
         expect(driver.find_xpath('//p').first.text).to eq 'finished'
@@ -2292,7 +2291,6 @@ def which_for(character)
     end
 
     it 'waits for the new window to load when the window location has changed' do
-      pending "I don't think we have enough control to actually track this"
       visit('/new_window?sleep=2')
       driver.execute_script("setTimeout(function() { window.location = 'about:blank' }, 1000)")
       driver.within_window(driver.window_handles.last) do