Marionette hangs until global timeout if the listener to the current content window is lost



4 years ago
a year ago


(Reporter: whimboo, Unassigned)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



4 years ago
I can see a hang of Firefox when the current chrome window gets closed. This can happen by code in the test, or by any malfunctioning code inside of the application. In those cases Marionette should never hang.

Here a testcase:

    def test_hang(self):
        # Open a new browser window
        self.marionette.execute_script("""; """)

        # Switch to the new chrome window

        # Some broken code in Firefox causes the current window to close
        self.marionette.execute_script(""" window.close(); """)

        # Without switching back to the original chrome window, Marionette will be frozen
        with self.marionette.using_context('content'):
            print('*** current URL: %s' % self.marionette.get_url())


4 years ago
Summary: Marionette hangs forever during shutdown if current chrome window gets closed → Marionette hangs forever if current chrome window gets closed and content scripts executed afterward
I looked in to this with :whimboo, and we're losing touch with our content listener (self.marionette.current_window_handle in the client and curBrowser.curFrameId in the server correspond to a window that has been destroyed), so the server broadcasts an async message to no listeners, and hangs perpetually.
This is by design. We inject listeners into where we need to working. If the window/frame is closed then we expect that the user has intended that or are in a situation that you didnt intend to be in.

YOu should always switch to the window you want after closing a window. Going to leave this bug here to see if we can give a better experience if people his this issue.

Comment 3

4 years ago
Given that Marionette observes chrome windows and tabs, can't we check if the desired content window is still around before broadcasting the message? If it is not the case we could raise a not found exception. So it's clear right away what the problem is, whether if this is the fault of the test, or a misbehavior by the application. As said above right now it hangs for 6 minutes (!!) without any output or a not that helpful exception that the screenshot could not be taken. Raising an exception immediately will reduce the full testrun time drastically, especially in cases this problem happens in nearly each test.


4 years ago
Summary: Marionette hangs forever if current chrome window gets closed and content scripts executed afterward → Marionette hangs until global timeout if the listener to the current content window is lost
Keywords: ateam-marionette-server

Comment 4

2 years ago
Created attachment 8805022 [details]

So with the testcase we no longer hang but raise a MarionetteException:

> Failed to gather test failure debug: this.browserForTab is undefined

TEST-UNEXPECTED-ERROR | TestAboutPages.test_hang | MarionetteException: this.browserForTab is undefined
Traceback (most recent call last):
  File "/Volumes/data/code/gecko/testing/marionette/harness/marionette/marionette_test/", line 162, in run
  File "/Volumes/data/code/gecko/_a/", line 24, in test_hang
    print('*** current URL: %s' % self.marionette.get_url())
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/", line 1587, in get_url
    return self._send_message("getCurrentUrl", key="value")
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/", line 27, in _
    return func(*args, **kwargs)
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/", line 731, in _send_message
  File "/Volumes/data/code/gecko/testing/marionette/client/marionette_driver/", line 764, in _handle_error
    raise errors.lookup(error)(message, stacktrace=stacktrace)

Comment 5

a year ago
Anything regarding `window.close()` got fixed in bug 1223277. But there are still possibilities for commands like `execute_script` where code triggers a navigation. We will get some general improvements via bug 1332122 as explained on bug 1368434 comment 4.
Depends on: 1332122

Comment 6

a year ago
I don't think it makes sense to keep this bug open any longer. The original issue was fixed by bug 1223277, which basically is a dupe of.
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1223277
You need to log in before you can comment on or make changes to this bug.