Closed Bug 1571221 Opened 4 years ago Closed 4 years ago

Add a check for this.selection before calling this this.selection.destroy()

Categories

(DevTools :: Inspector, task, P3)

Product:
DevTools
Component:
Inspector
Type:
task

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: gl, Assigned: shellyc23, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Bug 1571221 - Add a check for 'this.selection' before calling 'destroy()' on it r=gl
47 bytes, text/x-phabricator-request
Details | Review

I got the following console error:

console.error: (new TypeError("this.selection is undefined", "resource://devtools/shared/fronts/inspector.js", 512))

I believe this happens between navigation of pages very quickly with the inspector open. Looking at the inspector.js#479, we can see that this.selection is initialized asynchronously, which can result in the error when it is called in destroy() in inspector.js#512.

We should add a check for this.selection before calling this.selection.destroy() in the case it hasn't been initialized.

Mentor: gl

I would like to take a stab at this

Assignee: nobody → shellyc23
Status: NEW → ASSIGNED

Added an if statement to check for this.selection'. If it is truthy,destroy()` will be called on it.

Pushed by gluong@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/63ffc8b43cc3
Add a check for 'this.selection' before calling 'destroy()' on it r=gl

Backed out for failures on browser_ext_devtools_inspectedWindow.js

backout: https://hg.mozilla.org/integration/autoland/rev/4766d53d47a456248b5a5e505dc7d9d84e2ea7a3

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=63ffc8b43cc36864b3cba67a96a2339e62fe9929&selectedJob=259946702

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259946702&repo=autoland&lineNumber=15776

[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/test-oop-extensions/browser_ext_devtools_inspectedWindow.js | A promise chain failed to handle a rejection: Connection closed, pending request to server0.conn3.child1/inspectorActor3, type getWalker failed
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO -
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - Request stack:
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - request@resource://devtools/shared/protocol/Front.js:166:14
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - generateRequestMethods/</frontProto[name]@resource://devtools/shared/protocol/Front/FrontClassWithSpec.js:49:19
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - _getWalker@resource://devtools/shared/fronts/inspector.js:494:30
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - initialize@resource://devtools/shared/fronts/inspector.js:477:29
[task 2019-08-05T16:14:07.970Z] 16:14:07 INFO - getFront@resource://devtools/shared/protocol/types.js:533:21
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - getInspector@resource://devtools/shared/fronts/targets/target-mixin.js:202:31
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - initInspector/this._initInspector<@resource://devtools/client/framework/toolbox.js:3282:45
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - initInspector@resource://devtools/client/framework/toolbox.js:3306:19
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - loadTool@resource://devtools/client/framework/toolbox.js:2217:12
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - selectTool@resource://devtools/client/framework/toolbox.js:2496:17
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - unloadTool@resource://devtools/client/framework/toolbox.js:3182:14
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - removeAdditionalTool@resource://devtools/client/framework/toolbox.js:2206:10
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - close@chrome://browser/content/parent/ext-devtools-panels.js:267:15
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - unload@resource://gre/modules/ExtensionCommon.jsm:884:11
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - unload@resource://gre/modules/ExtensionParent.jsm:804:11
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - unload@resource://gre/modules/ExtensionParent.jsm:885:11
[task 2019-08-05T16:14:07.971Z] 16:14:07 INFO - closeProxyContext@resource://gre/modules/ExtensionParent.jsm:1082:15
[task 2019-08-05T16:14:07.972Z] 16:14:07 INFO - observe@resource://gre/modules/ExtensionParent.jsm:962:16
[task 2019-08-05T16:14:07.972Z] 16:14:07 INFO - _releaseBrowser@resource://gre/modules/ExtensionParent.jsm:1465:18
[task 2019-08-05T16:14:07.972Z] 16:14:07 INFO - shutdown@resource://gre/modules/ExtensionParent.jsm:1460:12
[task 2019-08-05T16:14:07.972Z] 16:14:07 INFO - close@chrome://browser/content/parent/ext-devtools.js:214:11
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - shutdownForTarget@chrome://browser/content/parent/ext-devtools.js:282:20
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - onToolboxDestroy@chrome://browser/content/parent/ext-devtools.js:442:25
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - emit@resource://devtools/shared/event-emitter.js:190:24
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - emit@resource://devtools/shared/event-emitter.js:271:18
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - createToolbox/<@resource://devtools/client/framework/devtools.js:625:12
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - newListener@resource://devtools/shared/event-emitter.js:149:22
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - emit@resource://devtools/shared/event-emitter.js:190:24
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - emit@resource://devtools/shared/event-emitter.js:271:18
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - _destroyToolbox@resource://devtools/client/framework/toolbox.js:3467:10
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - destroy@resource://devtools/client/framework/toolbox.js:3461:28
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - closeToolbox@resource://devtools/client/framework/devtools.js:668:19
[task 2019-08-05T16:14:07.973Z] 16:14:07 INFO - asynccloseToolboxForTab@chrome://mochitests/content/browser/browser/components/extensions/test/browser/test-oop-extensions/head_devtools.js:80:19
[task 2019-08-05T16:14:07.974Z] 16:14:07 INFO - asynctest_devtools_inspectedWindow_eval_in_page_and_panel@chrome://mochitests/content/browser/browser/components/extensions/test/browser/test-oop-extensions/browser_ext_devtools_inspectedWindow.js:435:9
[task 2019-08-05T16:14:07.974Z] 16:14:07 INFO - AsyncTester_execTest/<@chrome://mochikit/content/browser-test.js:1346:34
[task 2019-08-05T16:14:07.974Z] 16:14:07 INFO - asyncTester_execTest@chrome://mochikit/content/browser-test.js:1381:11
[task 2019-08-05T16:14:07.974Z] 16:14:07 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:1209:14
[task 2019-08-05T16:14:07.974Z] 16:14:07 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:67

Flags: needinfo?(shellyc23)
Flags: needinfo?(gl)

The fails might be fixed with the rework around fission such as Bug 1568151. I will investigate further when those patches land. Keeping the needinfo for to remind myself that we need to land this.

Flags: needinfo?(shellyc23)

The selection ended up getting refactored out of the Inspector init to live as a singleton in the Toolbox. As a result, we don't have to worry about the asynchronous nature of the selection being initialized and destroyed due to the toolbox rapidly opening and closing. Closing this bug since it will be resolved in Bug 1572460.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: needinfo?(gl)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.