1441462 - Layout sidebar panel crash on stripe.com/connect

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Patrick Brosset <:pbro>]

Attachment: sidebar.PNG

Here's what I did:

- opened stripe.com/connect
- opened the inspector with the layout tab visible
- clicked on each and every checkbox in the list of grids, one by one

At some stage, not sure which grid, the whole sidebar panel crashed and became empty.
The rest of the tools still worked, and the page was fine too.

See the screenshot.

Also, here's what was in the browser console when that happened:

TypeError: gridFragments[0] is undefined
Stack trace:
getTotalWidthAndHeight@resource://devtools/shared/base-loader.js -> resource://devtools/client/inspector/grids/components/GridOutline.js:177:11
componentWillReceiveProps@resource://devtools/shared/base-loader.js -> resource://devtools/client/inspector/grids/components/GridOutline.js:78:31
callComponentWillReceiveProps@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:6244:5
updateClassInstance@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:6419:7
updateClassComponent@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:7537:22
beginWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:7883:16
performUnitOfWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9706:16
workLoop@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9753:26
renderRoot@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9832:9
performWorkOnRoot@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10465:24
performWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10418:7
batchedUpdates@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10537:9
batchedUpdates@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:2203:12
dispatchEvent@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:3353:5
EventListener.handleEvent*listen@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:3163:7
trapBubbledEvent@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:3313:10
listenTo@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:3676:9
ensureListeningTo@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:11899:3
setInitialDOMProperties@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:11980:9
setInitialProperties$1@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:12130:3
finalizeInitialChildren@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:12637:5
completeWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:8306:19
completeUnitOfWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9631:18
performUnitOfWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9709:14
workLoop@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9753:26
renderRoot@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:9832:9
performWorkOnRoot@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10465:24
performWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10418:7
requestWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10329:7
scheduleWorkImpl@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10188:11
scheduleWork@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:10152:12
enqueueSetState@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react-dom.js:6131:7
Component.prototype.setState@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/vendor/react.js:346:3
addTab@resource://devtools/shared/base-loader.js -> resource://devtools/client/shared/components/tabs/TabBar.js:113:5
addTab@resource://devtools/shared/base-loader.js -> resource://devtools/client/inspector/toolsidebar.js:97:5
addExistingTab@resource://devtools/shared/base-loader.js -> resource://devtools/client/inspector/toolsidebar.js:118:5
addRuleView@chrome://devtools/content/inspector/inspector.js:645:7
async*setupSidebar@chrome://devtools/content/inspector/inspector.js:709:11
async*_deferredOpen@chrome://devtools/content/inspector/inspector.js:277:5
async*Inspector.prototype.init<@chrome://devtools/content/inspector/inspector.js:163:18
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:926:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:810:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:774:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
onPacket/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1339:9
DevTools RDP*request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1279:14
generateRequestMethods/</frontProto[name]@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1433:14
WalkerFront<.querySelector<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/fronts/inspector.js:158:12
_getDefaultNodeForSelection/<@chrome://devtools/content/inspector/inspector.js:345:14
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:926:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:810:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:774:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
onPacket/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1339:9
DevTools RDP*request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1279:14
generateRequestMethods/</frontProto[name]@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1433:14
InspectorFront<.getPageStyle<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/fronts/inspector.js:483:12
_getPageStyle@chrome://devtools/content/inspector/inspector.js:305:12
Inspector.prototype.init<@chrome://devtools/content/inspector/inspector.js:156:11
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
promise callback*_handleResultValue@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:386:7
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:318:13
process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:926:23
walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:810:7
Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:743:11
schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:774:7
completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7
onPacket/<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1339:9
DevTools RDP*request@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1279:14
generateRequestMethods/</frontProto[name]@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1433:14
initCssProperties<@resource://devtools/shared/base-loader.js -> resource://devtools/shared/fronts/css-properties.js:242:16
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
TaskImpl@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:272:3
asyncFunction@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:246:14
Inspector.prototype.init<@chrome://devtools/content/inspector/inspector.js:153:33
_run@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:310:39
TaskImpl@resource://devtools/shared/base-loader.js -> resource://devtools/shared/task.js:272:3

Comment 1

[Patrick Brosset <:pbro>]

Tracking 60 since we really should fix this in time for 60.
I could not reproduce on 59.

Comment 2

[Julien Cristau [:jcristau]]

Is anyone looking at this?  Do we have a regression range?

Comment 3

[Patrick Brosset <:pbro>]

So, we have 2 options here:
- either hide the grid from the list if it has no fragments
- or show it but handle the lack of fragments correctly

On the example I gave in comment 0, the lack of fragments comes from the fact that some grid elements are display:none when the page loads.

With option 1, in updateGridPanel (in grid-inspector.js), it would be easy enough to skip over gridFronts that don't have fragments.
Anyway, whenever there's a reflow, we refresh the panel, so as soon as display:none goes away, the grids would be displayed in the panel.

With option 2, the fix is just as easy: everywhere we access gridFragments[0], we should first check that the array isn't empty.
This way, the hidden grids still appear in the list, but clicking on them doesn't do anything visible (no outline appears, and no highlighter either).

Gabriel: what do you think is the right solution to choose here?

Comment 4

[Patrick Brosset <:pbro>]

I'll push a commit for option 2. I think it's probably the best solution.

Comment 5

[Patrick Brosset <:pbro>]

Attachment: Bug 1441462 - Avoid crashing the gridOutline when there are no fragments;

Review commit: https://reviewboard.mozilla.org/r/225518/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225518/

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 8956596 [details]
Bug 1441462 - Avoid crashing the gridOutline when there are no fragments;

https://reviewboard.mozilla.org/r/225518/#review231404

Comment 7

[Patrick Brosset <:pbro>]

Thanks Gabe. Here's a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f307361c41e8a6d04fba70ef5f6ae5b2c44aca18&group_state=expanded

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8aaf118e2e16
Avoid crashing the gridOutline when there are no fragments; r=gl

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8aaf118e2e16