Bug 1399076 - Error when weakref of web element is destroyed. r?automatedtester draft
authorAndreas Tolfsen <ato@sny.no>
Tue, 12 Sep 2017 13:18:52 +0100
changeset 663142 92bb9163e974d4067043bac832e3ccb90835668f
parent 663134 a73cc4e08bf5a005722c95b43f84ab0c8ff2bc7c
child 731114 305d3336514c4e4151aaaab1e48d78ae3a172267
push id79343
push userbmo:ato@sny.no
push dateTue, 12 Sep 2017 18:16:25 +0000
reviewersautomatedtester
bugs1399076
milestone57.0a1
Bug 1399076 - Error when weakref of web element is destroyed. r?automatedtester Take into account that a weak referenced element might have been destroyed in the element staleness check. An error is thrown when the reference object has been destroyed when getting a weakrefs' pointer. We catch this, but element.isStale does not take into account that the el argument in this case can be null, or in this revision of the patch, undefined. MozReview-Commit-ID: 7sr4YGhAotS
testing/marionette/element.js
testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py
--- a/testing/marionette/element.js
+++ b/testing/marionette/element.js
@@ -155,37 +155,40 @@ element.Store = class {
    *
    * @param {string} uuid
    *     Web element reference, or UUID.
    *
    * @returns {Element}
    *     Element associated with reference.
    *
    * @throws {NoSuchElementError}
-   *     If the provided reference is unknown.
+   *     If the web element reference <var>uuid</var> has not been
+   *     seen before.
    * @throws {StaleElementReferenceError}
-   *     If element has gone stale, indicating it is no longer attached to
-   *     the DOM provided in the container.
+   *     If the element has gone stale, indicating it is no longer
+   *     attached to the DOM, or its node document is no longer the
+   *     active document.
    */
   get(uuid) {
-    let el = this.els[uuid];
-    if (!el) {
-      throw new NoSuchElementError("Element reference not seen before: " + uuid);
+    if (!this.has(uuid)) {
+      throw new NoSuchElementError(
+          "Web element reference not seen before: " + uuid);
     }
 
+    let el;
+    let ref = this.els[uuid];
     try {
-      el = el.get();
+      el = ref.get();
     } catch (e) {
-      el = null;
       delete this.els[uuid];
     }
 
     if (element.isStale(el)) {
       throw new StaleElementReferenceError(
-          pprint`The element reference of ${el} stale; ` +
+          pprint`The element reference of ${el || uuid} stale; ` +
               "either the element is no longer attached to the DOM " +
               "or the document has been refreshed");
     }
 
     return el;
   }
 };
 
@@ -628,22 +631,26 @@ element.generateUUID = function() {
  *
  * @param {Element} el
  *     DOM element to check for staleness.
  *
  * @return {boolean}
  *     True if <var>el</var> is stale, false otherwise.
  */
 element.isStale = function(el) {
+  if (!el) {
+    return true;
+  }
+
   let doc = el.ownerDocument;
   let win = doc.defaultView;
-
   if (!win || el.ownerDocument !== win.document) {
     return true;
   }
+
   return !el.isConnected;
 };
 
 /**
  * This function generates a pair of coordinates relative to the viewport
  * given a target element and coordinates relative to that element's
  * top-left corner.
  *
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py
@@ -270,22 +270,22 @@ class TestScreenCaptureChrome(WindowMana
         self.marionette.switch_to_window(self.start_window)
 
     def test_highlight_element_not_seen(self):
         """Check that for not found elements an exception is raised."""
         with self.marionette.using_context('content'):
             self.marionette.navigate(box)
             content_element = self.marionette.find_element(By.ID, "green")
 
-        self.assertRaisesRegexp(NoSuchElementException, "Element reference not seen before",
+        self.assertRaisesRegexp(NoSuchElementException, "Web element reference not seen before",
                                 self.marionette.screenshot, highlights=[content_element])
 
         chrome_document_element = self.document_element
         with self.marionette.using_context('content'):
-            self.assertRaisesRegexp(NoSuchElementException, "Element reference not seen before",
+            self.assertRaisesRegexp(NoSuchElementException, "Web element reference not seen before",
                                     self.marionette.screenshot,
                                     highlights=[chrome_document_element])
 
 
 class TestScreenCaptureContent(WindowManagerMixin, ScreenCaptureTestCase):
 
     def setUp(self):
         super(TestScreenCaptureContent, self).setUp()