Bug 1399076 - Error when weakref of web element is destroyed. r=automatedtester
authorAndreas Tolfsen <ato@sny.no>
Tue, 12 Sep 2017 13:18:52 +0100
changeset 429917 724a1b6b12e3366e755ddc22ea1fba3c37d5bc3a
parent 429916 ec61b7a902c2b09a1e7cf6ef9c7eeb46f658368c
child 429918 634905bd50ef0080ac933d8e4613c5577cb52b7e
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1399076
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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()