close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Silne30, Assigned: Silne30)

Tracking

({pi-marionette-firefox-puppeteer})

unspecified
mozilla55
pi-marionette-firefox-puppeteer
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce:



    def test_main_tab_scalars(self):
        with self.marionette.using_context(self.marionette.CONTEXT_CHROME):
            tab2 = self.browser.tabbar.open_tab()
            self.browser.tabbar.switch_to(tab2)
            self.browser.tabbar.close_tab(tab2, force=True)
        self.marionette.restart(in_app=True, clean=False)
        assert 1 == 1


Actual results:

TEST-UNEXPECTED-ERROR | test_simple_test.py TestMainTabScalars.test_main_tab_scalars | JavascriptException: Element reference not seen before: 5e42b3dd-44bd-4a75-8589-ac0c13dfbf95
Traceback (most recent call last):
  File "/home/jdorlus/Desktop/firefox/testing/marionette/harness/marionette_harness/marionette_test/testcases.py", line 197, in run
    self.tearDown()
  File "/home/jdorlus/Desktop/firefox/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py", line 100, in tearDown
    super(TelemetryTestCase, self).tearDown()
  File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/mixins.py", line 99, in tearDown
    self._check_and_fix_leaked_handles()
  File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/mixins.py", line 60, in _check_and_fix_leaked_handles
    self.browser.tabbar.close_all_tabs([self.browser.tabbar.tabs[0]])
  File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py", line 45, in tabs
    tabs = self.toolbar.find_elements(By.TAG_NAME, 'tab')
  File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 59, in find_elements
    return self.marionette.find_elements(method, target, self.id)
  File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 1964, in find_elements
    "findElements", body, key="value" if self.protocol == 1 else None)
  File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/decorators.py", line 23, in _
    return func(*args, **kwargs)
  File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 735, in _send_message
    self._handle_error(err)
  File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 768, in _handle_error
    raise errors.lookup(error)(message, stacktrace=stacktrace)


Expected results:

Test should have passed.
The problem here is that the second tab which was opened gets closed via the above code. But it does not switch to the other tab which is still open, before the tearDown code. So _check_and_fix_leaked_handles() is run and seems to fail to find the tabs.
Keywords: ateam-marionette-firefox-puppeteer
Summary: JavascriptException: Element reference not seen before after restarting browser → close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()
(Assignee)

Comment 2

2 years ago
(In reply to Henrik Skupin (:whimboo) from comment #1)
> The problem here is that the second tab which was opened gets closed via the
> above code. But it does not switch to the other tab which is still open,
> before the tearDown code. So _check_and_fix_leaked_handles() is run and
> seems to fail to find the tabs.

I just amended the test to switch to the original tab that is still opened before the restart and the issue persists even with the switching prior. Let me know if there is something else I should have done.
If you can reproduce it would be great if you can provide a fix for this issue. I don't have the time to do it myself for the moment.
(Assignee)

Updated

2 years ago
Assignee: nobody → jdorlus
(Assignee)

Updated

2 years ago
Flags: needinfo?(ato)
(Assignee)

Comment 4

2 years ago
I have come to find that the element that seems to be providing the problem is the tabbar. That is the element referenced.
OK, clearing my needinfo in that case.
Flags: needinfo?(ato)
John, do you have an update for us? Looks like this bug is causing a problem for our update tests on beta, and we have to get this fixed asap.
Blocks: 1355818
Flags: needinfo?(jdorlus)
Comment hidden (mozreview-request)

Comment 8

2 years ago
mozreview-review
Comment on attachment 8868201 [details]
Bug 1358665 - Fixed test to use puppeteer to restart;

https://reviewboard.mozilla.org/r/139786/#review143116

::: toolkit/components/telemetry/tests/marionette/tests/client/test_main_tab_scalars.py:20
(Diff revision 1)
>              tab3 = self.browser.tabbar.open_tab()
>              self.browser.tabbar.switch_to(tab3)
>              self.browser.tabbar.close_tab(tab3, force=True)
>              self.browser.tabbar.close_tab(tab2, force=True)
> -        self.install_addon()
> +            self.browser.tabbar.switch_to(tab1)
> +        self.restart_browser()

To me this seems to be a wild change. Why would installing the addon be replaced with restarting the browser?
Attachment #8868201 - Flags: review?(chutten) → review-
(Assignee)

Comment 9

2 years ago
mozreview-review-reply
Comment on attachment 8868201 [details]
Bug 1358665 - Fixed test to use puppeteer to restart;

https://reviewboard.mozilla.org/r/139786/#review143116

> To me this seems to be a wild change. Why would installing the addon be replaced with restarting the browser?

This was supposed to be the original behavior but the restart functionality had a bug. This was implemented so there was still test coverage for the ping data.

Comment 10

2 years ago
mozreview-review
Comment on attachment 8868201 [details]
Bug 1358665 - Fixed test to use puppeteer to restart;

https://reviewboard.mozilla.org/r/139786/#review143164

r+ with the comment in the commit message
Attachment #8868201 - Flags: review- → review+
Component: Marionette → Telemetry
Product: Testing → Toolkit
Version: Version 3 → unspecified

Comment 11

2 years ago
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c679e572655
Fixed test to use puppeteer to restart;r=chutten
(Assignee)

Updated

2 years ago
Flags: needinfo?(jdorlus)

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5c679e572655
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
No longer blocks: 1355818
You need to log in before you can comment on or make changes to this bug.