1294139 - [debugger.html] Turn new debugger on by default for all channels

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P1)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

This bug is for tracking the change required to turn the debugger.html on as well as the work that is required to turn on the new debugger on by default.

bug 1288511 landed the new debugger off by default behind the pref: devtools.debugger.new-debugger-frontend

Comment 1

[Eddy Bruel [:ejpbruel]]

I'm assuming this is something we are working on right now, so tentatively marking this as P1. James, if that is not correct, feel free to change to whatever you think is appropriate.

Comment 2

[James Long (:jlongster)]

Yep, that sounds right. Sorry I haven't been triaging!

Comment 3

[Bryan Clark (DevTools PM) [:clarkbw]]

Updating title as the new debugger is already turned on by default in Nightly only.

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Adding bug 1314057 for removing the old debugger which requires this to be done plus the other work that will block that bug as well.

Comment 5

[James Long (:jlongster)]

Attachment: 1294139.patch

Comment 6

[Brian Grinstead [:bgrins]]

Comment on attachment 8809136 [details] [diff] [review]
1294139.patch

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

Congrats - nice work all!

Comment 7

[Brian Grinstead [:bgrins]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=649906622269055358ed6cbec66fd45be49c640e

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bab8ca566708
Turn new debugger on by default for all channels r=bgrins

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bab8ca566708

Comment 10

[Yury Delendik (:yury)]

The fix was somewhat reverted by bug 1332114. Re-opening to be resolved for release channels too.

Comment 11

[Patrick Brosset <:pbro>]

I know we're very close to enabling the debugger on all channels, which is really great. All blocker bugs here are marked as resolved.

I have 2 questions:

- Can we take a product decision on the 2 remaining features (that I know of) are missing still: blackboxing and project search? Blackboxing in particular first appeared in Firefox's old debugger and then other devtools adopted it. So it seems like something we'd want to have in our new debugger right from the start.

- Getting more manual QA on this might uncover some things we've missed. The change is so big though that we would need to create a few specific cases that people could test. Has the qe-verify+ flag been set on other (more specific) bugs already? Is this wanted?

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to Patrick Brosset <:pbro> from comment #11)
> - Can we take a product decision on the 2 remaining features (that I know
> of) are missing still: blackboxing and project search? 

For project search I think we can ship without for now.  Its a good feature but I think we have a much better file and function search available than before so I'm comfortable not requiring this at point.

> Blackboxing in
> particular first appeared in Firefox's old debugger and then other devtools
> adopted it. So it seems like something we'd want to have in our new debugger
> right from the start.

I agree.  I think I'm convinced that at least shipping an MVP of the black boxing feature should be required.

> - Getting more manual QA on this might uncover some things we've missed. The
> change is so big though that we would need to create a few specific cases
> that people could test. Has the qe-verify+ flag been set on other (more
> specific) bugs already? Is this wanted?

Yes.  We have a couple critical areas where it would be good to get some QA.  Off the top of my head its this:

* Breakpoints
* Watch Expressions
* Call Stack
* Scopes
* File Search
* Function Search

Comment 13

[Patrick Brosset <:pbro>]

As discussed in comment 12, let's see if we can get some QA for these areas of the new debugger:
* Breakpoints
* Watch Expressions
* Call Stack
* Scopes
* File Search
* Function Search
This is a rather large functional area, so we should describe in more details what should be tested here, but for now, let's use the qe-verify flag here and let's figure things out later.

Comment 14

[Alexandre Poirot [:ochameau]]

I'm very glad to see we setup some process to ship the new debugger!

I think we should also associate manual QA with a branch, or limit the kind of patches that gets in master.
Otherwise it will be very easy to slice in new regressions while QA is proceeding.

I know it is unpleasant and sounds like doing the things we don't like from mozilla-central,
but I do not see how we could ensure a good level of quality without such process.

Comment 15

[Jason Laster [:jlast]]

re: releasing, we discussed releasing a version of the "Enable new frontend" settings checkbox. This will let us soft launch and get valuable user feedback.

It'd be great to get a QA process setup. 

* I can help write a spec 
* How do we want to setup a branch for QA? Could we do branch off of github? I believe it's been discussed

Comment 16

[Bryan Clark (DevTools PM) [:clarkbw]]

Setting target for Fx 56

Comment 17

[Patrick Brosset <:pbro>]

With Nightly 55 cycle being twice as long, I'd like this to be resolved as soon as possible so we have time for manual QA in this cycle before we hit Beta 55.
If bug 1346902 is the last blocker, then what's blocking bug 1346902?

Comment 18

[Alexandre Poirot [:ochameau]]

I think we should run QA right away. The browser toolbox is an edge case. And tbh I'm not sure it is a blocker to push the new debugger on release channel.

During the workweek we were told the new debugger was ready to ship to release,
but the day after I looked at it very briefly and found various obvious bugs:
 - https://github.com/devtools-html/debugger.html/issues/2393
 - https://github.com/devtools-html/debugger.html/issues/2394
 - https://github.com/devtools-html/debugger.html/issues/2395

All has been fixed now (except the last), but I imagine similar issues may still exists.
We should spend time chasing and fixing these kind of bugs and shouldn't wait for it to be enabled on release to do that!

Comment 19

[Patrick Brosset <:pbro>]

(In reply to Alexandre Poirot [:ochameau] from comment #18)
> I think we should run QA right away. The browser toolbox is an edge case.
> And tbh I'm not sure it is a blocker to push the new debugger on release
> channel.
> 
> During the workweek we were told the new debugger was ready to ship to
> release,
> but the day after I looked at it very briefly and found various obvious bugs:
>  - https://github.com/devtools-html/debugger.html/issues/2393
>  - https://github.com/devtools-html/debugger.html/issues/2394
>  - https://github.com/devtools-html/debugger.html/issues/2395
> 
> All has been fixed now (except the last), but I imagine similar issues may
> still exists.
> We should spend time chasing and fixing these kind of bugs and shouldn't
> wait for it to be enabled on release to do that!
Good point. We've already reached out to QA to work out a plan for them to start testing as soon as possible, just waiting on a final confirmation.
You are right that enabling the debugger by default doesn't need to happen first though, and anyway it's already enabled in Nightly. I just want this to happen in the 55 timeframe, so the sooner the better.

Comment 20

[Jason Laster [:jlast]]

Attachment: enable-1.patch

Comment 21

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44f12345553493c0b445d4e9d7e5393de5201d0a

Comment 22

[Julian Descottes [:jdescottes]]

Comment on attachment 8885759 [details] [diff] [review]
enable-1.patch

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

Thanks, looks good! 

I only reviewed the code change and I assume that the debugger project matches all the criteria needed to enable it in beta & release.

Comment 23

[Jason Laster [:jlast]]

Attachment: enable-1.patch

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5687797261d
[debugger.html] Turn new debugger on by default for all channels. r=jdescottes

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f5687797261d

Comment 26

[Emil Ghitta, QA [:emilghitta]]

This was covered by manual testing efforts and received pre-Release sign off report in Fx 56. 

Removing the qe-verify+ flag.