1431127 - Fix TypeError: panel._selectors.getPause

Status: VERIFIED FIXED

(DevTools :: Debugger, defect, P3)

Description

[Jason Laster [:jlast]]

We started getting this error when we refactored the pause reducer to use isPaused

> Full message: TypeError: panel._selectors.getPause is not a function
> Full stack: setupThreadListeners@chrome://devtools/content/framework/toolbox-process-window.js:167:19
> bindToolboxHandlers/<@chrome://devtools/content/framework/toolbox-process-window.js:161:7
> process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23
> walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
> Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
> schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
> completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
> promise callback*completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:698:7
> Promise.resolve/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:512:36
> Promise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:390:5
> Promise.resolve@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:512:10
> onLoad@resource://devtools/shared/base-loader.js -> resource://devtools/client/framework/toolbox.js:1698:7

Comment 1

[Jason Laster [:jlast]]

Attachment: fix-pause.patch

Comment 2

[Jason Laster [:jlast]]

Here is what it looks like with the bug:
https://shipusercontent.com/2e8ed975f75cf1eb3d3571c8d765cdb7/Screen%20Shot%202018-01-17%20at%2010.50.03%20AM.png

Here is what it looks like with the fix:
https://shipusercontent.com/e7e0f7518edff96c9f36e60cd86d8a02/Screen%20Shot%202018-01-17%20at%2010.47.35%20AM.png

Comment 3

[Jason Laster [:jlast]]

just verified that it continues to work in the old debugger.

Comment 4

[Julian Descottes [:jdescottes]]

Comment on attachment 8943293 [details] [diff] [review]
fix-pause.patch

Review of attachment 8943293 [details] [diff] [review]:
-----------------------------------------------------------------

Good fix for new frontend. Won't work with old frontend, but not a regression and non blocking.

::: devtools/client/framework/toolbox-process-window.js
@@ +163,5 @@
>    }
>  }
>  
>  function setupThreadListeners(panel) {
> +  updateBadgeText(panel.isPaused());

(Since you mentioned it :)) 

It does not work with the old frontend. I think you tested with the old frontend in Firefox, but not the old frontend in the Browser Toolbox. To enable it you need to :
- open BT
- select other panel than debugger
- close BT
- open BT 
- go preferences
- disable new frontend
- switch to debugger

It was already not working previously. We could check if isPaused is available and is a function. Not mandatory IMO.

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8943293 [details] [diff] [review]
fix-pause.patch

Review of attachment 8943293 [details] [diff] [review]:
-----------------------------------------------------------------

Please use a proper commit message!

Comment 6

[Jason Laster [:jlast]]

Attachment: fix-pause-2.patch

Comment 7

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=920efa940a87284db384820397074189bdda1999

Comment 8

[Julian Descottes [:jdescottes]]

Is the try run for a new version of the patch? Otherwise feel free to land this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e42d6d64671
Fix TypeError: panel._selectors.getPause. r=jdescottes

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/9e42d6d64671

Comment 11

[Julian Descottes [:jdescottes]]

Comment on attachment 8943709 [details] [diff] [review]
fix-pause-2.patch

Minor cosmetic feature, but allows to get rid of a JS error.

Approval Request Comment
[Feature/Bug causing the regression]:N/A (a previous debugger bundle update from https://bugzilla.mozilla.org/show_bug.cgi?id=debugger-bundle-updates)
[User impact if declined]: MacOS users developing using the addon or browser toolbox (webextension developers for instance) will not see the red "start/pause" marker on the Firefox instance icon.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: yes.
- use MacOS
- go to about:debugging#addons
- enable addon debugging if not checked already
- click on debug link of any addon
- toolbox opens as new app
ER: the app icon (visible eg. in the dock) shows a "►" character

[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: minor javascript change
[String changes made/needed]: none

Comment 12

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8943709 [details] [diff] [review]
fix-pause-2.patch

Useful fix for Mac developers, let's take this for the 59 beta 4 build.

Comment 13

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/631af655e23e

Comment 14

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 60.0a1 (BuildId:20180206100151) and Firefox 59.0b7 (BuildId:20180205211730) on macOS 10.12.6.