Bug 1368965 - assert.contentBrowser has to check for closed chrome window. draft
authorHenrik Skupin <mail@hskupin.info>
Tue, 13 Jun 2017 18:08:22 +0200
changeset 594283 e33ff5b68074882c2e4ee0f5009db4b0d7969c9d
parent 594056 ad3f1138ce6f199408ad58d65c7476636e924909
child 594284 d5a92f001649cb7236d584bd8586dd377ec3186f
push id63976
push userbmo:hskupin@gmail.com
push dateWed, 14 Jun 2017 20:00:27 +0000
bugs1368965
milestone56.0a1
Bug 1368965 - assert.contentBrowser has to check for closed chrome window. Due to the current caching of this.tab in browser.js the assertion for the contentBrowser would fail. As such additionally assert that the currently selected chrome window has not been closed yet. Also assert.window should better check the 'closed' property instead of handling a possible TypeError. MozReview-Commit-ID: B79czg3dsux
testing/marionette/assert.js
testing/marionette/test_assert.js
--- a/testing/marionette/assert.js
+++ b/testing/marionette/assert.js
@@ -98,41 +98,40 @@ assert.content = function (context, msg 
  * @return {ChromeWindow}
  *     |win| is returned unaltered.
  *
  * @throws {NoSuchWindowError}
  *     If |win| has been closed.
  */
 assert.window = function (win, msg = "") {
   msg = msg || "Unable to locate window";
-  return assert.that(w => {
-    try {
-      return w && w.document.defaultView;
-
-    // If the window is no longer available a TypeError is thrown.
-    } catch (e if e.name === "TypeError") {
-      return null;
-    }
-  }, msg, NoSuchWindowError)(win);
+  return assert.that(w => w && !w.closed,
+      msg,
+      NoSuchWindowError)(win);
 };
 
 /**
  * Asserts that |context| is a valid browsing context.
  *
  * @param {browser.Context} context
  *     Browsing context to test.
  * @param {string=} msg
  *     Custom error message.
  *
  * @throws {NoSuchWindowError}
  *     If |context| is invalid.
  */
 assert.contentBrowser = function (context, msg = "") {
+  // TODO: The contentBrowser uses a cached tab, which is only updated when
+  // switchToTab is called. Because of that an additional check is needed to
+  // make sure that the chrome window has not already been closed.
+  assert.window(context && context.window);
+
   msg = msg || "Current window does not have a content browser";
-  assert.that(c => c && c.contentBrowser,
+  assert.that(c => c.contentBrowser,
       msg,
       NoSuchWindowError)(context);
 };
 
 /**
  * Asserts that there is no current user prompt.
  *
  * @param {modal.Dialog} dialog
--- a/testing/marionette/test_assert.js
+++ b/testing/marionette/test_assert.js
@@ -102,31 +102,32 @@ add_test(function test_string() {
   assert.string("foo");
   assert.string(`bar`);
   Assert.throws(() => assert.string(42), InvalidArgumentError);
 
   run_next_test();
 });
 
 add_test(function test_window() {
-  assert.window({ document: { defaultView: true }});
+  assert.window({closed: false});
 
-  let deadWindow = { get document() { throw new TypeError("can't access dead object"); }};
-
-  for (let typ of [null, undefined, deadWindow]) {
+  for (let typ of [null, undefined, {closed: true}]) {
     Assert.throws(() => assert.window(typ), NoSuchWindowError);
   }
 
   run_next_test();
 });
 
 add_test(function test_contentBrowser() {
-  assert.contentBrowser({contentBrowser: 42});
+  assert.contentBrowser({contentBrowser: 42, window: {closed: false}});
 
-  for (let typ of [null, undefined, {contentBrowser: null}]) {
+  let closedWindow = {contentBrowser: 42, window: {closed: true}};
+  let noContentBrowser = {contentBrowser: null, window: {closed: false}};
+
+  for (let typ of [null, undefined, closedWindow, noContentBrowser]) {
     Assert.throws(() => assert.contentBrowser(typ), NoSuchWindowError);
   }
 
   run_next_test();
 });
 
 add_test(function test_object() {
   assert.object({});