Backed out 4 changesets (bug 1519335) for causing a spike in bug 1641788, per whimboo's request CLOSED TREE
authorBogdan Tara <btara@mozilla.com>
Wed, 03 Jun 2020 18:34:46 +0300
changeset 533713 6ffead9985f06152d026cf95e3d73853ce698dc5
parent 533712 d2c50c8dbb5ddd3188ca4eb9b20d3b162250b72f
child 533714 2ad5a97370854bbbad4d4239e42a123a49cb0fd9
push id37476
push userccoroiu@mozilla.com
push dateWed, 03 Jun 2020 21:49:22 +0000
treeherdermozilla-central@66d3efe9fc7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1519335, 1641788
milestone79.0a1
backs out80a0c09b0211cb649ce3613e660ef3b17751e405
315a7ffd643bb48c6e2daa92b72e8bc2e9a1f9bf
7c82cbd6f034991a2122d441df49898dfe769741
f84fa4157d90bd59e8330c9f34bb452b88d56779
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
Backed out 4 changesets (bug 1519335) for causing a spike in bug 1641788, per whimboo's request CLOSED TREE Backed out changeset 80a0c09b0211 (bug 1519335) Backed out changeset 315a7ffd643b (bug 1519335) Backed out changeset 7c82cbd6f034 (bug 1519335) Backed out changeset f84fa4157d90 (bug 1519335)
testing/marionette/assert.js
testing/marionette/browser.js
testing/marionette/driver.js
testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py
testing/marionette/harness/marionette_harness/tests/unit/test_switch_frame.py
testing/marionette/listener.js
--- a/testing/marionette/assert.js
+++ b/testing/marionette/assert.js
@@ -283,35 +283,16 @@ assert.positiveNumber = function(obj, ms
  *     If <var>obj</var> is not callable.
  */
 assert.callable = function(obj, msg = "") {
   msg = msg || pprint`${obj} is not callable`;
   return assert.that(o => typeof o == "function", msg)(obj);
 };
 
 /**
- * Asserts that <var>obj</var> is an unsigned short number.
- *
- * @param {?} obj
- *     Value to test.
- * @param {string=} msg
- *     Custom error message.
- *
- * @return {number}
- *     <var>obj</var> is returned unaltered.
- *
- * @throws {InvalidArgumentError}
- *     If <var>obj</var> is not an unsigned short.
- */
-assert.unsignedShort = function(obj, msg = "") {
-  msg = msg || pprint`Expected ${obj} to be >= 0 and < 65536`;
-  return assert.that(n => n >= 0 && n < 65536, msg)(obj);
-};
-
-/**
  * Asserts that <var>obj</var> is an integer.
  *
  * @param {?} obj
  *     Value to test.
  * @param {string=} msg
  *     Custom error message.
  *
  * @return {number}
--- a/testing/marionette/browser.js
+++ b/testing/marionette/browser.js
@@ -562,52 +562,52 @@ browser.Context = class {
       this.pendingCommands.push(cb);
     } else {
       cb();
     }
   }
 };
 
 /**
- * The window storage is used to save browsing context ids mapped to weak
+ * The window storage is used to save outer window IDs mapped to weak
  * references of Window objects.
  *
  * Usage:
  *
  *     let wins = new browser.Windows();
- *     wins.set(browser.browsingContext.id, window);
+ *     wins.set(browser.outerWindowID, window);
  *
  *     ...
  *
- *     let win = wins.get(browser.browsingContext.id);
+ *     let win = wins.get(browser.outerWindowID);
  *
  */
 browser.Windows = class extends Map {
   /**
    * Save a weak reference to the Window object.
    *
    * @param {string} id
-   *     Browsing context id.
+   *     Outer window ID.
    * @param {Window} win
    *     Window object to save.
    *
    * @return {browser.Windows}
    *     Instance of self.
    */
   set(id, win) {
     let wref = Cu.getWeakReference(win);
     super.set(id, wref);
     return this;
   }
 
   /**
    * Get the window object stored by provided |id|.
    *
    * @param {string} id
-   *     Browsing context id.
+   *     Outer window ID.
    *
    * @return {Window}
    *     Saved window object.
    *
    * @throws {RangeError}
    *     If |id| is not in the store.
    */
   get(id) {
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -283,17 +283,17 @@ Object.defineProperty(GeckoDriver.protot
   },
 });
 
 Object.defineProperty(GeckoDriver.prototype, "chromeWindowHandles", {
   get() {
     let hs = [];
 
     for (let win of this.windows) {
-      hs.push(getWindowId(win));
+      hs.push(getOuterWindowId(win));
     }
 
     return hs;
   },
 });
 
 GeckoDriver.prototype.QueryInterface = ChromeUtils.generateQI([
   Ci.nsIObserver,
@@ -469,26 +469,26 @@ GeckoDriver.prototype.addFrameCloseListe
  * Create a new browsing context for window and add to known browsers.
  *
  * @param {ChromeWindow} win
  *     Window for which we will create a browsing context.
  *
  * @return {string}
  *     Returns the unique server-assigned ID of the window.
  */
-GeckoDriver.prototype.addBrowser = function(win) {
-  let context = new browser.Context(win, this);
-  let winId = getWindowId(win);
-
-  this.browsers[winId] = context;
+GeckoDriver.prototype.addBrowser = function(window) {
+  let bc = new browser.Context(window, this);
+  let winId = getOuterWindowId(window);
+
+  this.browsers[winId] = bc;
   this.curBrowser = this.browsers[winId];
   if (!this.wins.has(winId)) {
     // add this to seenItems so we can guarantee
     // the user will get winId as this window's id
-    this.wins.set(winId, win);
+    this.wins.set(winId, window);
   }
 };
 
 /**
  * Registers a new browser, win, with Marionette.
  *
  * If we have not seen the browser content window before, the listener
  * frame script will be loaded into it.  If isNewSession is true, we will
@@ -574,66 +574,66 @@ GeckoDriver.prototype.getVisibleText = f
   }
 };
 
 /**
  * Handles registration of new content listener browsers.  Depending on
  * their type they are either accepted or ignored.
  */
 GeckoDriver.prototype.registerBrowser = function(id, be) {
+  let listenerWindow = Services.wm.getOuterWindowWithId(id);
+
   // We want to ignore frames that are XUL browsers that aren't in the "main"
   // tabbrowser, but accept things on Fennec (which doesn't have a
   // xul:tabbrowser), and accept HTML iframes (because tests depend on it),
   // as well as XUL frames. Ideally this should be cleaned up and we should
   // keep track of browsers a different way.
   if (
     this.appId != APP_ID_FIREFOX ||
     be.namespaceURI != XUL_NS ||
     be.nodeName != "browser" ||
     be.getTabBrowser()
   ) {
     // curBrowser holds all the registered frames in knownFrames
     this.curBrowser.register(id, be);
   }
 
-  const context = BrowsingContext.get(id);
-  this.wins.set(id, context.currentWindowGlobal);
-
+  this.wins.set(id, listenerWindow);
   return id;
 };
 
 GeckoDriver.prototype.registerPromise = function() {
   const li = "Marionette:Register";
 
   return new Promise(resolve => {
     let cb = ({ json, target }) => {
-      let { frameId } = json;
-      this.registerBrowser(frameId, target);
+      let { outerWindowID } = json;
+      this.registerBrowser(outerWindowID, target);
 
       if (this.curBrowser.frameRegsPending > 0) {
         this.curBrowser.frameRegsPending--;
       }
 
       if (this.curBrowser.frameRegsPending === 0) {
         this.mm.removeMessageListener(li, cb);
         resolve();
       }
 
-      return { frameId };
+      return { outerWindowID };
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
 GeckoDriver.prototype.listeningPromise = function() {
   const li = "Marionette:ListenersAttached";
 
   return new Promise(resolve => {
     let cb = msg => {
-      if (msg.json.frameId === this.curBrowser.curFrameId) {
+      if (msg.json.outerWindowID === this.curBrowser.curFrameId) {
         this.mm.removeMessageListener(li, cb);
         resolve();
       }
     };
     this.mm.addMessageListener(li, cb);
   });
 };
 
@@ -1385,17 +1385,17 @@ GeckoDriver.prototype.getIdForBrowser = 
     return null;
   }
 
   let permKey = browser.permanentKey;
   if (this._browserIds.has(permKey)) {
     return this._browserIds.get(permKey);
   }
 
-  let winId = browser.browsingContext.id;
+  let winId = browser.outerWindowID;
   if (winId) {
     this._browserIds.set(permKey, winId);
     return winId;
   }
   return null;
 };
 
 /**
@@ -1613,35 +1613,35 @@ GeckoDriver.prototype.switchToWindow = a
  *     the outerId of the window, and returning a boolean indicating
  *     whether the window is the target.
  *
  * @return {Object}
  *     A window handle object containing the window and some
  *     associated metadata.
  */
 GeckoDriver.prototype.findWindow = function(winIterable, filter) {
-  for (const win of winIterable) {
-    const bc = win.docShell.browsingContext;
-    const tabBrowser = browser.getTabBrowser(win);
+  for (let win of winIterable) {
+    let outerId = getOuterWindowId(win);
+    let tabBrowser = browser.getTabBrowser(win);
 
     // In case the wanted window is a chrome window, we are done.
-    if (filter(win, bc.id)) {
-      return { win, id: bc.id, hasTabBrowser: !!tabBrowser };
+    if (filter(win, outerId)) {
+      return { win, outerId, hasTabBrowser: !!tabBrowser };
 
       // Otherwise check if the chrome window has a tab browser, and that it
       // contains a tab with the wanted window handle.
     } else if (tabBrowser && tabBrowser.tabs) {
       for (let i = 0; i < tabBrowser.tabs.length; ++i) {
         let contentBrowser = browser.getBrowserForTab(tabBrowser.tabs[i]);
         let contentWindowId = this.getIdForBrowser(contentBrowser);
 
         if (filter(win, contentWindowId)) {
           return {
             win,
-            id: bc.id,
+            outerId,
             hasTabBrowser: true,
             tabIndex: i,
           };
         }
       }
     }
   }
 
@@ -1660,17 +1660,17 @@ GeckoDriver.prototype.findWindow = funct
  * @param {boolean=} focus
  *     A boolean value which determines whether to focus the window.
  *     Defaults to true.
  */
 GeckoDriver.prototype.setWindowHandle = async function(
   winProperties,
   focus = true
 ) {
-  if (!(winProperties.id in this.browsers)) {
+  if (!(winProperties.outerId in this.browsers)) {
     // Initialise Marionette if the current chrome window has not been seen
     // before. Also register the initial tab, if one exists.
     let registerBrowsers, browserListening;
 
     if (winProperties.hasTabBrowser) {
       registerBrowsers = this.registerPromise();
       browserListening = this.listeningPromise();
     }
@@ -1678,17 +1678,17 @@ GeckoDriver.prototype.setWindowHandle = 
     this.startBrowser(winProperties.win, false /* isNewSession */);
 
     if (registerBrowsers && browserListening) {
       await registerBrowsers;
       await browserListening;
     }
   } else {
     // Otherwise switch to the known chrome window
-    this.curBrowser = this.browsers[winProperties.id];
+    this.curBrowser = this.browsers[winProperties.outerId];
     this.mainFrame = this.curBrowser.window;
     this.curFrame = null;
 
     // .. and activate the tab if it's a content browser.
     if ("tabIndex" in winProperties) {
       await this.curBrowser.switchToTab(
         winProperties.tabIndex,
         winProperties.win,
@@ -1753,20 +1753,16 @@ GeckoDriver.prototype.switchToParentFram
  *     A modal dialog is open, blocking this operation.
  */
 GeckoDriver.prototype.switchToFrame = async function(cmd) {
   assert.open(this.getCurrentWindow());
   await this._handleUserPrompts();
 
   let { id, focus } = cmd.parameters;
 
-  if (typeof id == "number") {
-    assert.unsignedShort(id, `Expected id to be unsigned short, got ${id}`);
-  }
-
   // TODO(ato): element can be either string (deprecated) or a web
   // element JSON Object.  Can be removed with Firefox 60.
   let byFrame;
   if (typeof cmd.parameters.element == "string") {
     byFrame = WebElement.fromUUID(cmd.parameters.element, Context.Chrome);
   } else if (cmd.parameters.element) {
     byFrame = WebElement.fromJSON(cmd.parameters.element);
   }
@@ -3567,22 +3563,22 @@ GeckoDriver.prototype.receiveMessage = f
           );
         } else {
           this.currentFrameElement = null;
         }
       }
       break;
 
     case "Marionette:Register":
-      let { frameId } = message.json;
-      this.registerBrowser(frameId, message.target);
-      return { frameId };
+      let { outerWindowID } = message.json;
+      this.registerBrowser(outerWindowID, message.target);
+      return { outerWindowID };
 
     case "Marionette:ListenersAttached":
-      if (message.json.frameId === this.curBrowser.curFrameId) {
+      if (message.json.outerWindowID === this.curBrowser.curFrameId) {
         this.curBrowser.flushPendingCommands();
       }
       break;
 
     case "Marionette:WebDriver:GetCapabilities":
       return this.capabilities.toJSON();
   }
 };
@@ -3894,18 +3890,18 @@ GeckoDriver.prototype.commands = {
   "WebDriver:SetWindowRect": GeckoDriver.prototype.setWindowRect,
   "WebDriver:SwitchToFrame": GeckoDriver.prototype.switchToFrame,
   "WebDriver:SwitchToParentFrame": GeckoDriver.prototype.switchToParentFrame,
   "WebDriver:SwitchToShadowRoot": GeckoDriver.prototype.switchToShadowRoot,
   "WebDriver:SwitchToWindow": GeckoDriver.prototype.switchToWindow,
   "WebDriver:TakeScreenshot": GeckoDriver.prototype.takeScreenshot,
 };
 
-function getWindowId(win) {
-  return win.docShell.browsingContext.id;
+function getOuterWindowId(win) {
+  return win.windowUtils.outerWindowID;
 }
 
 async function exitFullscreen(win) {
   let cb;
   // Use a timed promise to abort if no window manager is present
   await new TimedPromise(
     resolve => {
       cb = new DebounceCallback(resolve);
--- a/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py
+++ b/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py
@@ -87,17 +87,17 @@ class WindowManagerMixin(object):
         current_windows = self.marionette.chrome_window_handles
         current_tabs = self.marionette.window_handles
 
         def loaded(handle):
             with self.marionette.using_context("chrome"):
                 return self.marionette.execute_script("""
                   Components.utils.import("resource://gre/modules/Services.jsm");
 
-                  const win = BrowsingContext.get(Number(arguments[0])).window;
+                  let win = Services.wm.getOuterWindowWithId(Number(arguments[0]));
                   return win.document.readyState == "complete";
                 """, script_args=[handle])
 
         try:
             if callable(callback):
                 callback(focus)
             else:
                 result = self.marionette.open(type="window", focus=focus, private=private)
@@ -169,14 +169,14 @@ class WindowManagerMixin(object):
                     // The new window shouldn't get focused. As such set the
                     // focus back to the opening window.
                     if (!focus && Services.focus.activeWindow != window) {
                       let focused = waitForFocus(window);
                       window.focus();
                       await focused;
                     }
 
-                    resolve(win.docShell.browsingContext.id);
+                    resolve(win.windowUtils.outerWindowID);
                   })();
                 """, script_args=(url, focus))
 
         with self.marionette.using_context("chrome"):
             return self.open_window(callback=open_with_js, focus=focus)
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_frame.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_frame.py
@@ -1,17 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import
 
 from marionette_driver.by import By
 from marionette_driver.errors import (
-    InvalidArgumentException,
     JavascriptException,
     NoSuchFrameException,
 )
 
 from marionette_harness import MarionetteTestCase
 
 
 class TestSwitchFrame(MarionetteTestCase):
@@ -118,23 +117,21 @@ class TestSwitchFrame(MarionetteTestCase
         self.marionette.switch_to_frame(frame)
 
         element = self.marionette.find_element(By.ID, "email")
         self.assertEquals("email", element.get_attribute("type"))
 
     def test_switch_to_frame_with_out_of_bounds_index(self):
         self.marionette.navigate(self.marionette.absolute_url("test_iframe.html"))
         count = self.marionette.execute_script("return window.frames.length;")
-        for index in [count, 65535]:
-            self.assertRaises(NoSuchFrameException, self.marionette.switch_to_frame, index)
+        self.assertRaises(NoSuchFrameException, self.marionette.switch_to_frame, count)
 
-    def test_switch_to_frame_with_invalid_index(self):
+    def test_switch_to_frame_with_negative_index(self):
         self.marionette.navigate(self.marionette.absolute_url("test_iframe.html"))
-        for index in [-1, 65536]:
-            self.assertRaises(InvalidArgumentException, self.marionette.switch_to_frame, index)
+        self.assertRaises(NoSuchFrameException, self.marionette.switch_to_frame, -1)
 
     def test_switch_to_parent_frame(self):
         frame_html = self.marionette.absolute_url("frameset.html")
         self.marionette.navigate(frame_html)
         frame = self.marionette.find_element(By.NAME, "third")
         self.marionette.switch_to_frame(frame)
 
         # If we don't find the following element we aren't on the right page
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -53,36 +53,23 @@ const { legacyaction } = ChromeUtils.imp
   "chrome://marionette/content/legacyaction.js"
 );
 const { Log } = ChromeUtils.import("chrome://marionette/content/log.js");
 const { navigate } = ChromeUtils.import(
   "chrome://marionette/content/navigate.js"
 );
 const { proxy } = ChromeUtils.import("chrome://marionette/content/proxy.js");
 
-XPCOMUtils.defineLazyGetter(this, "logger", () => Log.getWithPrefix(contentId));
+XPCOMUtils.defineLazyGetter(this, "logger", () =>
+  Log.getWithPrefix(outerWindowID)
+);
 XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
 
-const contentId = content.docShell.browsingContext.id;
-
-const curContainer = {
-  _frame: null,
-  shadowRoot: null,
-
-  get frame() {
-    return this._frame;
-  },
-
-  set frame(frame) {
-    this._frame = frame;
-
-    this.id = this._frame.docShell.browsingContext.id;
-    this.shadowRoot = null;
-  },
-};
+let { outerWindowID } = winUtil;
+let curContainer = { frame: content, shadowRoot: null };
 
 // Listen for click event to indicate one click has happened, so actions
 // code can send dblclick event
 addEventListener("click", event.DoubleClickTracker.setClick);
 addEventListener("dblclick", event.DoubleClickTracker.resetClick);
 addEventListener("unload", event.DoubleClickTracker.resetClick, true);
 
 const seenEls = new element.Store();
@@ -381,26 +368,27 @@ const loadListener = {
           new TimeoutError(`Timeout loading page after ${this.timeout}ms`),
           this.commandID
         );
         break;
     }
   },
 
   observe(subject, topic) {
-    logger.trace(`Received observer notification ${topic}`);
+    const win = curContainer.frame;
+    const winID = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
+    const curWinID = win.windowUtils.outerWindowID;
 
-    const winId = subject.QueryInterface(Ci.nsISupportsPRUint64).data;
-    const bc = BrowsingContext.get(curContainer.id);
+    logger.trace(`Received observer notification ${topic}`);
 
     switch (topic) {
       // In the case when the currently selected frame is closed,
       // there will be no further load events. Stop listening immediately.
       case "outer-window-destroyed":
-        if (bc.window.windowUtils.outerWindowID == winId) {
+        if (curWinID === winID) {
           this.stop();
           sendOk(this.commandID);
         }
         break;
     }
   },
 
   /**
@@ -488,37 +476,35 @@ const loadListener = {
 /**
  * Called when listener is first started up.  The listener sends its
  * unique window ID and its current URI to the actor.  If the actor returns
  * an ID, we start the listeners. Otherwise, nothing happens.
  */
 function registerSelf() {
   logger.trace("Frame script loaded");
 
-  curContainer.frame = content;
-
   sandboxes.clear();
+  curContainer = {
+    frame: content,
+    shadowRoot: null,
+  };
   legacyactions.mouseEventsOnly = false;
   action.inputStateMap = new Map();
   action.inputsToCancel = [];
 
-  let reply = sendSyncMessage("Marionette:Register", {
-    frameId: contentId,
-  });
+  let reply = sendSyncMessage("Marionette:Register", { outerWindowID });
   if (reply.length == 0) {
     logger.error("No reply from Marionette:Register");
     return;
   }
 
-  if (reply[0].frameId === contentId) {
+  if (reply[0].outerWindowID === outerWindowID) {
     logger.trace("Frame script registered");
     startListeners();
-    sendAsyncMessage("Marionette:ListenersAttached", {
-      frameId: contentId,
-    });
+    sendAsyncMessage("Marionette:ListenersAttached", { outerWindowID });
   }
 }
 
 // Eventually we will not have a closure for every single command,
 // but use a generic dispatch for all listener commands.
 //
 // Worth nothing that this shares many characteristics with
 // server.TCPConnection#execute.  Perhaps this could be generalised
@@ -675,21 +661,19 @@ function deregister() {
   removeMessageListener("Marionette:switchToFrame", switchToFrame);
   removeMessageListener("Marionette:switchToParentFrame", switchToParentFrame);
   removeMessageListener("Marionette:switchToShadowRoot", switchToShadowRootFn);
   removeMessageListener("Marionette:waitForPageLoaded", waitForPageLoaded);
 }
 
 function deleteSession() {
   seenEls.clear();
-
   // reset container frame to the top-most frame
-  curContainer.frame = content;
+  curContainer = { frame: content, shadowRoot: null };
   curContainer.frame.focus();
-
   legacyactions.touchIds = {};
   if (action.inputStateMap !== undefined) {
     action.inputStateMap.clear();
   }
   if (action.inputsToCancel !== undefined) {
     action.inputsToCancel.length = 0;
   }
 }
@@ -760,17 +744,18 @@ function emitTouchEvent(type, touch) {
     `Emitting Touch event of type ${type} ` +
       `to element with id: ${touch.target.id} ` +
       `and tag name: ${touch.target.tagName} ` +
       `at coordinates (${touch.clientX}), ` +
       `${touch.clientY}) relative to the viewport`
   );
 
   const win = curContainer.frame;
-  if (win.docShell.asyncPanZoomEnabled && legacyactions.scrolling) {
+  let docShell = win.docShell;
+  if (docShell.asyncPanZoomEnabled && legacyactions.scrolling) {
     let ev = {
       index: 0,
       type,
       id: touch.identifier,
       clientX: touch.clientX,
       clientY: touch.clientY,
       screenX: touch.screenX,
       screenY: touch.screenY,
@@ -1525,19 +1510,17 @@ function switchToParentFrame(msg) {
 }
 
 /**
  * Switch to frame given either the server-assigned element id,
  * its index in window.frames, or the iframe's name or id.
  */
 function switchToFrame(msg) {
   let commandID = msg.json.commandID;
-
-  let foundFrame;
-  let frameWebEl;
+  let foundFrame = null;
 
   // check if curContainer.frame reference is dead
   let frames = [];
   try {
     frames = curContainer.frame.frames;
   } catch (e) {
     // dead comparment, redirect to top frame
     msg.json.id = null;
@@ -1547,17 +1530,16 @@ function switchToFrame(msg) {
   if (
     (msg.json.id === null || msg.json.id === undefined) &&
     msg.json.element == null
   ) {
     // returning to root frame
     sendSyncMessage("Marionette:switchedToFrame", { frameValue: null });
 
     curContainer.frame = content;
-
     if (msg.json.focus) {
       curContainer.frame.focus();
     }
 
     sendOk(commandID);
     return;
   }
 
@@ -1583,89 +1565,89 @@ function switchToFrame(msg) {
 
     if (frames.length > 0) {
       for (let i = 0; i < frames.length; i++) {
         // use XPCNativeWrapper to compare elements; see bug 834266
         let frameEl = frames[i].frameElement;
         let wrappedItem = new XPCNativeWrapper(frameEl);
         let wrappedWanted = new XPCNativeWrapper(wantedFrame);
         if (wrappedItem == wrappedWanted) {
-          foundFrame = frameEl;
+          curContainer.frame = frameEl;
+          foundFrame = i;
         }
       }
     }
 
-    if (!foundFrame) {
+    if (foundFrame === null) {
       // Either the frame has been removed or we have a OOP frame
       // so lets just get all the iframes and do a quick loop before
       // throwing in the towel
       const doc = curContainer.frame.document;
       let iframes = doc.getElementsByTagName("iframe");
       for (let i = 0; i < iframes.length; i++) {
         let frameEl = iframes[i];
         let wrappedEl = new XPCNativeWrapper(frameEl);
         let wrappedWanted = new XPCNativeWrapper(wantedFrame);
         if (wrappedEl == wrappedWanted) {
-          foundFrame = iframes[i];
+          curContainer.frame = iframes[i];
+          foundFrame = i;
         }
       }
     }
   }
 
-  if (!foundFrame) {
+  if (foundFrame === null) {
     if (typeof msg.json.id === "number") {
       try {
-        if (msg.json.id >= 0 && msg.json.id < frames.length) {
-          foundFrame = frames[msg.json.id].frameElement;
-          if (foundFrame !== null) {
-            frameWebEl = seenEls.add(foundFrame.wrappedJSObject);
-          } else {
-            // If foundFrame is null at this point then we have the top
-            // level browsing context so should treat it accordingly.
-            sendSyncMessage("Marionette:switchedToFrame", { frameValue: null });
-            curContainer.frame = content;
+        foundFrame = frames[msg.json.id].frameElement;
+        if (foundFrame !== null) {
+          curContainer.frame = foundFrame;
+          foundFrame = seenEls.add(curContainer.frame);
+        } else {
+          // If foundFrame is null at this point then we have the top
+          // level browsing context so should treat it accordingly.
+          sendSyncMessage("Marionette:switchedToFrame", { frameValue: null });
+          curContainer.frame = content;
 
-            if (msg.json.focus) {
-              curContainer.frame.focus();
-            }
+          if (msg.json.focus) {
+            curContainer.frame.focus();
+          }
 
-            sendOk(commandID);
-            return;
-          }
+          sendOk(commandID);
+          return;
         }
       } catch (e) {
         // Since window.frames does not return OOP frames it will throw
         // and we land up here. Let's not give up and check if there are
         // iframes and switch to the indexed frame there
-        let doc = foundFrame.document;
+        let doc = curContainer.frame.document;
         let iframes = doc.getElementsByTagName("iframe");
         if (msg.json.id >= 0 && msg.json.id < iframes.length) {
-          foundFrame = iframes[msg.json.id];
+          curContainer.frame = iframes[msg.json.id];
+          foundFrame = msg.json.id;
         }
       }
     }
   }
 
-  if (!foundFrame) {
+  if (foundFrame === null) {
     let failedFrame = msg.json.id || msg.json.element;
     let err = new NoSuchFrameError(`Unable to locate frame: ${failedFrame}`);
     sendError(err, commandID);
     return;
   }
 
   // send a synchronous message to let the server update the currently active
   // frame element (for getActiveFrame)
-  if (!frameWebEl) {
-    frameWebEl = seenEls.add(foundFrame.wrappedJSObject);
-  }
+  let frameWebEl = seenEls.add(curContainer.frame.wrappedJSObject);
   sendSyncMessage("Marionette:switchedToFrame", {
     frameValue: frameWebEl.uuid,
   });
 
-  curContainer.frame = foundFrame.contentWindow;
+  curContainer.frame = curContainer.frame.contentWindow;
   if (msg.json.focus) {
     curContainer.frame.focus();
   }
 
   sendOk(commandID);
 }
 
 /**