Fix #25. Possible 'race condition' when node becomes obsolete.

teampoltergeist · Feb 24, 2012 · 3011b27 · 3011b27 · jonleighton · Feb 24, 2012
1 parent fc88a85
commit 3011b27
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 19 deletions.
diff --git a/lib/capybara/poltergeist/client/agent.coffee b/lib/capybara/poltergeist/client/agent.coffee
@@ -7,6 +7,12 @@ class PoltergeistAgent
     @windows  = []
     this.pushWindow(window)
 
+  externalCall: (name, arguments) ->
+    try
+      { value: this[name].apply(this, arguments) }
+    catch error
+      { error: error.toString() }
+
   pushWindow: (new_window) ->
     @windows.push(new_window)
 
@@ -51,8 +57,12 @@ class PoltergeistAgent
 
   nodeCall: (id, name, arguments) ->
     node = this.get(id)
+    throw new PoltergeistAgent.ObsoleteNode if node.isObsolete()
     node[name].apply(node, arguments)
 
+class PoltergeistAgent.ObsoleteNode
+  toString: -> "PoltergeistAgent.ObsoleteNode"
+
 class PoltergeistAgent.Node
   @EVENTS = {
     FOCUS: ['blur', 'focus', 'focusin', 'focusout'],

diff --git a/lib/capybara/poltergeist/client/compiled/agent.js b/lib/capybara/poltergeist/client/compiled/agent.js
@@ -7,6 +7,17 @@ PoltergeistAgent = (function() {
     this.windows = [];
     this.pushWindow(window);
   }
+  PoltergeistAgent.prototype.externalCall = function(name, arguments) {
+    try {
+      return {
+        value: this[name].apply(this, arguments)
+      };
+    } catch (error) {
+      return {
+        error: error.toString()
+      };
+    }
+  };
   PoltergeistAgent.prototype.pushWindow = function(new_window) {
     this.windows.push(new_window);
     this.window = new_window;
@@ -53,10 +64,20 @@ PoltergeistAgent = (function() {
   PoltergeistAgent.prototype.nodeCall = function(id, name, arguments) {
     var node;
     node = this.get(id);
+    if (node.isObsolete()) {
+      throw new PoltergeistAgent.ObsoleteNode;
+    }
     return node[name].apply(node, arguments);
   };
   return PoltergeistAgent;
 })();
+PoltergeistAgent.ObsoleteNode = (function() {
+  function ObsoleteNode() {}
+  ObsoleteNode.prototype.toString = function() {
+    return "PoltergeistAgent.ObsoleteNode";
+  };
+  return ObsoleteNode;
+})();
 PoltergeistAgent.Node = (function() {
   Node.EVENTS = {
     FOCUS: ['blur', 'focus', 'focusin', 'focusout'],

diff --git a/lib/capybara/poltergeist/client/compiled/node.js b/lib/capybara/poltergeist/client/compiled/node.js
@@ -9,19 +9,12 @@ Poltergeist.Node = (function() {
   Node.prototype.parent = function() {
     return new Poltergeist.Node(this.page, this.parentId());
   };
-  Node.prototype.isObsolete = function() {
-    return this.page.nodeCall(this.id, 'isObsolete');
-  };
   _ref = Node.DELEGATES;
   _fn = __bind(function(name) {
     return this.prototype[name] = function() {
       var arguments, _ref2;
       _ref2 = arguments, arguments = 1 <= _ref2.length ? __slice.call(_ref2, 0) : [];
-      if (this.isObsolete()) {
-        throw new Poltergeist.ObsoleteNode;
-      } else {
-        return this.page.nodeCall(this.id, name, arguments);
-      }
+      return this.page.nodeCall(this.id, name, arguments);
     };
   }, Node);
   for (_i = 0, _len = _ref.length; _i < _len; _i++) {

diff --git a/lib/capybara/poltergeist/client/compiled/web_page.js b/lib/capybara/poltergeist/client/compiled/web_page.js
@@ -163,9 +163,21 @@ Poltergeist.WebPage = (function() {
     };
   };
   WebPage.prototype.runCommand = function(name, arguments) {
-    return this.evaluate(function(name, arguments) {
-      return __poltergeist[name].apply(__poltergeist, arguments);
+    var result;
+    result = this.evaluate(function(name, arguments) {
+      return __poltergeist.externalCall(name, arguments);
     }, name, arguments);
+    if (result.error) {
+      switch (result.error) {
+        case "PoltergeistAgent.ObsoleteNode":
+          throw new Poltergeist.ObsoleteNode;
+          break;
+        default:
+          throw result.error;
+      }
+    } else {
+      return result.value;
+    }
   };
   return WebPage;
 }).call(this);
diff --git a/lib/capybara/poltergeist/client/node.coffee b/lib/capybara/poltergeist/client/node.coffee
@@ -9,16 +9,10 @@ class Poltergeist.Node
   parent: ->
     new Poltergeist.Node(@page, this.parentId())
 
-  isObsolete: ->
-    @page.nodeCall(@id, 'isObsolete')
-
   for name in @DELEGATES
     do (name) =>
       this.prototype[name] = (arguments...) ->
-        if this.isObsolete()
-          throw new Poltergeist.ObsoleteNode
-        else
-          @page.nodeCall(@id, name, arguments)
+        @page.nodeCall(@id, name, arguments)
 
   scrollIntoView: ->
     dimensions = @page.validatedDimensions()

diff --git a/lib/capybara/poltergeist/client/web_page.coffee b/lib/capybara/poltergeist/client/web_page.coffee
@@ -129,7 +129,16 @@ class Poltergeist.WebPage
         that[name].apply(that, arguments)
 
   runCommand: (name, arguments) ->
-    this.evaluate(
-      (name, arguments) -> __poltergeist[name].apply(__poltergeist, arguments),
+    result = this.evaluate(
+      (name, arguments) -> __poltergeist.externalCall(name, arguments),
       name, arguments
     )
+
+    if result.error
+      switch result.error
+        when "PoltergeistAgent.ObsoleteNode"
+          throw new Poltergeist.ObsoleteNode
+        else
+          throw result.error
+    else
+      result.value