Fix reset crash when replies are destroyed

We attempted to track in-progress replies and abort them in 1.5.1. However, sometimes these replies are destroyed unexpectedly, so attempting to abort them raises errors. This commit tracks when replies are destroyed and removes them from the queue of in-progress replies.
thoughtbot · Jun 5, 2015 · 79088c2 · 79088c2
1 parent 179ca5b
commit 79088c2
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 4 deletions.
diff --git a/spec/driver_spec.rb b/spec/driver_spec.rb
@@ -7,8 +7,12 @@
 describe Capybara::Webkit::Driver do
   include AppRunner
 
-  def visit(url, driver=self.driver)
-    driver.visit("#{AppRunner.app_host}#{url}")
+  def visit(path, driver=self.driver)
+    driver.visit(url(path))
+  end
+
+  def url(path)
+    "#{AppRunner.app_host}#{path}"
   end
 
   context "iframe app" do
@@ -3041,6 +3045,53 @@ def which_for(character)
     end
   end
 
+  context "with unfinished responses" do
+    it_behaves_like "output writer" do
+      let(:driver) do
+        count = 0
+        driver_for_app browser do
+          get "/" do
+            count += 1
+            <<-HTML
+              <html>
+                <body>
+                  <script type="text/javascript">
+                    setTimeout(function () {
+                      xhr = new XMLHttpRequest();
+                      xhr.open('GET', '/async?#{count}', true);
+                      xhr.setRequestHeader('Content-Type', 'text/plain');
+                      xhr.send();
+                    }, 50);
+                  </script>
+                </body>
+              </html>
+            HTML
+          end
+
+          get "/async" do
+            sleep 2
+            ""
+          end
+        end
+      end
+
+      it "aborts unfinished responses" do
+        driver.enable_logging
+        visit "/"
+        sleep 0.5
+        visit "/"
+        sleep 0.5
+        driver.reset!
+        stderr.should abort_request_to("/async?2")
+        stderr.should_not abort_request_to("/async?1")
+      end
+
+      def abort_request_to(path)
+        include(%{Aborting request to "#{url(path)}"})
+      end
+    end
+  end
+
   def driver_url(driver, path)
     URI.parse(driver.current_url).merge(path).to_s
   end

diff --git a/spec/support/app_runner.rb b/spec/support/app_runner.rb
@@ -25,10 +25,10 @@ def run_application(app)
     AppRunner.app = app
   end
 
-  def driver_for_app(&body)
+  def driver_for_app(*driver_args, &body)
     app = Class.new(ExampleApp, &body)
     run_application app
-    build_driver
+    build_driver(*driver_args)
   end
 
   def driver_for_html(html, *driver_args)

diff --git a/src/WebPageManager.cpp b/src/WebPageManager.cpp
@@ -89,6 +89,11 @@ void WebPageManager::requestCreated(QByteArray &url, QNetworkReply *reply) {
   else {
     m_pendingReplies.append(reply);
     connect(reply, SIGNAL(finished()), SLOT(handleReplyFinished()));
+    connect(
+      reply,
+      SIGNAL(destroyed(QObject *)),
+      SLOT(replyDestroyed(QObject *))
+    );
   }
 }
 
@@ -104,6 +109,10 @@ void WebPageManager::replyFinished(QNetworkReply *reply) {
   m_pendingReplies.removeAll(reply);
 }
 
+void WebPageManager::replyDestroyed(QObject *reply) {
+  m_pendingReplies.removeAll((QNetworkReply *) reply);
+}
+
 void WebPageManager::setPageStatus(bool success) {
   logger() << "Page finished with" << success;
   m_started.remove(qobject_cast<WebPage *>(sender()));

diff --git a/src/WebPageManager.h b/src/WebPageManager.h
@@ -48,6 +48,7 @@ class WebPageManager : public QObject {
     void setPageStatus(bool);
     void requestCreated(QByteArray &url, QNetworkReply *reply);
     void handleReplyFinished();
+    void replyDestroyed(QObject *);
 
   signals:
     void pageFinished(bool);