1621450 - RDM's TargetList will startListening after a target switch

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, task, P3)

Description

[Julian Descottes [:jdescottes]]

See https://phabricator.services.mozilla.com/D58903
Target Switching for RDM was initially implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=1603370

The current implementation is not explicitly calling startListening, and because of that, the onTargetAvailable callback is not checking for isTopLevel (see phab thread at https://phabricator.services.mozilla.com/D58903?id=214822#inline-361163)

It's a bit confusing as it relies on knowing the internal implementation of target-list. But this is technically correct for the initial target. However, after the first target-switch, startListening() will be automatically called [1]. And at that point RDM's onTargetAvailable might be triggered for non top-level target.

[1] https://searchfox.org/mozilla-central/rev/2fd8ffcf087bc59a8e5c962965bbb7bf230bcd28/devtools/shared/resources/target-list.js#514

It might be safer to just check isTopLevel (and add the call to startListening for consistency).

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1621450 - Check isTopLevel in RDM onTargetAvailable

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a2c533bcdb0
Check isTopLevel in RDM onTargetAvailable r=daisuke

Comment 3

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/158da5d529f3
Backed out changeset 6a2c533bcdb0 for many devtools failures CLOSED TREE

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 6a2c533bcdb0 (Bug 1621450) for many devtools failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2C18.04%2Cx64%2Casan%2Copt%2Cmochitests%2Ctest-linux1804-64-asan%2Fopt-mochitest-devtools-chrome-e10s-3%2Cm%28dt3%29&fromchange=09c8d799a2b40015aa6a7fc1f787b0379ffa0d74&tochange=4fcb55fd17861ea5d642f7e5a08e5f3901736919&selectedJob=293346109

Backout link: https://hg.mozilla.org/integration/autoland/rev/158da5d529f354cd6dce037860f96227ec3b51da

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293346109&repo=autoland&lineNumber=3586

[task 2020-03-16T16:06:31.785Z] 16:06:31     INFO - TEST-START | devtools/client/responsive/test/browser/browser_device_change.js
[task 2020-03-16T16:06:31.805Z] 16:06:31     INFO - Adding Test Device "Fake Phone RDM Test" to the list.
[task 2020-03-16T16:06:31.805Z] 16:06:31     INFO - Entering test bound 
[task 2020-03-16T16:06:31.842Z] 16:06:31     INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/responsive/test/browser/doc_page_state.html
[task 2020-03-16T16:06:32.100Z] 16:06:32     INFO - Tab added and finished loading
[task 2020-03-16T16:06:32.100Z] 16:06:32     INFO - Opening responsive design mode
[task 2020-03-16T16:06:32.108Z] 16:06:32     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - TEST-UNEXPECTED-FAIL | devtools/client/responsive/test/browser/browser_device_change.js | Uncaught exception - at resource://devtools/client/responsive/ui.js:347 - TypeError: can't access property "stopListening", this.targetList is undefined
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - Stack trace:
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - initRDMFrame@resource://devtools/client/responsive/ui.js:347:5
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - init@resource://devtools/client/responsive/ui.js:163:12
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - ResponsiveUI@resource://devtools/client/responsive/ui.js:115:24
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - openIfNeeded@resource://devtools/client/responsive/manager.js:129:18
[task 2020-03-16T16:06:32.109Z] 16:06:32     INFO - async*openRDM@chrome://mochitests/content/browser/devtools/client/responsive/test/browser/head.js:120:28
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - addRDMTaskWithPreAndPost/<@chrome://mochitests/content/browser/devtools/client/responsive/test/browser/head.js:201:31
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - Async*Tester_execTest/<@chrome://mochikit/content/browser-test.js:1062:34
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:1097:11
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - nextTest/<@chrome://mochikit/content/browser-test.js:925:14
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:918:23
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - Leaving test bound 
[task 2020-03-16T16:06:32.110Z] 16:06:32     INFO - Entering test bound 
[task 2020-03-16T16:06:32.166Z] 16:06:32     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "http://example.com/browser/devtools/client/responsive/test/browser/doc_page_state.html" line: 0}]
[task 2020-03-16T16:06:32.263Z] 16:06:32     INFO - Adding a new tab with URL: http://example.com/browser/devtools/client/responsive/test/browser/doc_page_state.html
[task 2020-03-16T16:06:32.896Z] 16:06:32     INFO - GECKO(3626) | console.log: "[DISPATCH] action type:" "CHANGE_DISPLAY_PIXEL_RATIO"
[task 2020-03-16T16:06:32.903Z] 16:06:32     INFO - GECKO(3626) | console.log: "[DISPATCH] action type:" "ADD_VIEWPORT"
[task 2020-03-16T16:06:33.126Z] 16:06:33     INFO - Tab added and finished loading
[task 2020-03-16T16:06:33.126Z] 16:06:33     INFO - Opening responsive design mode
[task 2020-03-16T16:06:33.141Z] 16:06:33     INFO - Not taking screenshot here: see the one that was previously logged
[task 2020-03-16T16:06:33.142Z] 16:06:33     INFO - TEST-UNEXPECTED-FAIL | devtools/client/responsive/test/browser/browser_device_change.js | Got an error with usingBrowserUI true: TypeError: can't access property "stopListening", this.targetList is undefined
[task 2020-03-16T16:06:33.142Z] 16:06:33     INFO - Stack: initRDMFrame@resource://devtools/client/responsive/ui.js:347:5
[task 2020-03-16T16:06:33.142Z] 16:06:33     INFO - init@resource://devtools/client/responsive/ui.js:163:12
....
....
....

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

Also seeing this failure starting with the backed out chances: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293357796&repo=autoland&lineNumber=2890

Comment 6

[Julian Descottes [:jdescottes]]

Weird that lando managed to land this, it should have had a merge conflict!
At least locally I had to merge manually.

Thanks for the heads up, will rebase and update.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a6052600a95
Check isTopLevel in RDM onTargetAvailable r=daisuke

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/5a6052600a95