Closed Bug 1674172 Opened 2 years ago Closed 2 years ago

getMarionetteCommandsActorProxy doesn't check for a valid browsing context


(Remote Protocol :: Marionette, defect, P3)



(Fission Milestone:M7, firefox84 fixed)

84 Branch
Fission Milestone M7
Tracking Status
firefox84 --- fixed


(Reporter: whimboo, Assigned: jdescottes)



(Whiteboard: [marionette-fission-mvp])


(1 file)

Running all the Mn unit tests (here while having the upcoming patches for bug 1673823 applied I can see the following in the logs:

1603992368558	Marionette	DEBUG	2 -> [0,16,"WebDriver:ExecuteScript",{"script":"Components.utils.import(\"resource://gre/modules/Services.jsm\");\n            let ... ],"filename":"testing/marionette/harness/marionette_harness/tests/unit/","sandbox":"default","line":122}]
1603992368586	Marionette	TRACE	[16] Frame script unloaded
1603992368591	Marionette	DEBUG	2 <- [1,16,null,{"value":null}]
JavaScript error: resource://activity-stream/lib/ASRouter.jsm, line 969: NS_ERROR_ILLEGAL_VALUE: Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIObserverService.removeObserver]
console.error: Region.jsm: "Error fetching region" (new TypeError("NetworkError when attempting to fetch resource.", ""))
console.error: Region.jsm: "Failed to fetch region" (new Error("NO_RESULT", "resource://gre/modules/Region.jsm", 376))
1603992368600	Marionette	DEBUG	Closed connection 2
JavaScript error: chrome://marionette/content/actors/commands_parent.js, line 324: TypeError: can't access property "getActor", browsingContextFn().currentWindowGlobal is null

Note that those lines should also exist without my patches.

It means we currently fail to check for null when trying to get the currentWindowGlobal from the browsing context as returned by browsingContextFn().

Julian, mind having a look?

Flags: needinfo?(jdescottes)

Tracking marionette-fission-mvp bugs for Fission Beta milestone (M7).

Fission Milestone: --- → M7

Missed the ni? looking at this right now. Keeping the ni? for now.

I can reproduce on central when running:

./mach test --enable-fission -vv --gecko-log - 

We are hitting the issue which is alluded to in this comment:

              // TODO: Scenarios where the window/tab got closed and
              // currentWindowGlobal is null will be handled in Bug 1662808.
              const actor = browsingContextFn().currentWindowGlobal.getActor(

Which we discussed a bit in the review at

See Also: → 1662808

Added a few logs, we get this issue when trying to use the "cleanUp":

  if (MarionettePrefs.useActors) {
    if (this.getBrowsingContext()) {
      try {
        // reset any global state used by parent actor
      } catch (e) {
        if (e.result != Cr.NS_ERROR_DOM_NOT_FOUND_ERR) {
          throw e;

And the cleanUp implementation in the actor:

  cleanUp() {

We can look at this in different ways:

  • cleanUp doesn't depend on the ParentActor instance. elementIdCache is a singleton in the scope of MarionetteFrameParent.jsm -> we could just expose cleanUp outside of the actor
  • driver.js checks this.getBrowsingContext() but doesn't check for the window, which is mandatory to get the actor -> we could add such a check
  • we could throw Cr.NS_ERROR_DOM_NOT_FOUND_ERR when the window is missing

I don't really see why we should get the actor to perform the cleanUp here, so I'm enclined to go with the first option.

Flags: needinfo?(jdescottes)
Assignee: nobody → jdescottes
Pushed by
[marionette] Move Marionette parent actor cleanUp to a static helper r=marionette-reviewers,whimboo
Pushed by
Fix bc failures on marionette/content/driver.js. r=jdescottes
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.