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())
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.
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.
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
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)
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
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.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1223277
You need to log in before you can comment on or make changes to this bug.