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

Status: RESOLVED DUPLICATE of bug 1223277

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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(""" window.open(); """)

        # Switch to the new chrome window
        self.marionette.switch_to_window('13')

        # 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
        #self.marionette.switch_to_window('3')
        with self.marionette.using_context('content'):
            print('*** current URL: %s' % self.marionette.get_url())

Comment 1

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 2

[David Burns :automatedtester]

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

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: testcase

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

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.