1214248 - [meta] Push DevTools for Dev. Edition 47

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Axel Kratel]

Tracking bug for Push Notification devtool features landing in Firefox Developer Edition 46.

Comment 1

[Axel Kratel]

Jan, 

https://bugzilla.mozilla.org/show_bug.cgi?id=1209369 is now split into the following bugs. 

  1212797 about:debugging List registrations instead of live Service Workers
  1209699 about:debugging Push sample test for push service workers
  1209746 about:debugging items should include item-appropriate metadata
  1215379 about:debugging items should include means to pause a worker on service worker load in debugger
  1215385 about:debugging once debugging session is started on SW, the SW stays alive/won't be suspended
  1215386 about:debugging should provide SW lifecycle mngnt: start, stop, unregister

Please review Bug 1209369 and let me know if you want it split even further. Thanks.

Comment 2

[Axel Kratel]

Eddy, 

These are the bugs I found that are relevant for SW debugging to work properly, please let me know if this list is complete or if we should add more. 

1150444 Promises don't work properly in debugging sessions for workers
1175550 main 'SW debugging' bug
1178726 Proper pausing of workers
1211903 WorkerDebugger should live on the main thread
1212333 WorkerDebuggerManager should live on the main thread
1212344 [meta] Clean up the worker debugger API. 
1178721 SuspendTimeouts should suspend worker events without freezing workers

dank je wel.

Comment 3

[Axel Kratel]

Brian, I added these to the list, let me know if there are more. 

1215117 Make console input field work inside a worker toolbox 
1201962	enable service worker logging in web console by default	
1209353	Worker debugger console / scratchpad do not work

Comment 4

[Jan Keromnes [:janx]]

(In reply to Axel Kratel from comment #1)
> Jan, 
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1209369 is now split into the
> following bugs. 
> 
>   1212797 about:debugging List registrations instead of live Service Workers
>   1209699 about:debugging Push sample test for push service workers
>   1209746 about:debugging items should include item-appropriate metadata
>   1215379 about:debugging items should include means to pause a worker on
> service worker load in debugger
>   1215385 about:debugging once debugging session is started on SW, the SW
> stays alive/won't be suspended
>   1215386 about:debugging should provide SW lifecycle mngnt: start, stop,
> unregister
> 
> Please review Bug 1209369 and let me know if you want it split even further.
> Thanks.

That's OK for now, thanks a lot Axel!

Also flagging this push meta-bug as a blocker to the service worker meta-bug.

Comment 5

[Eddy Bruel [:ejpbruel]]

Hi Axel. This bug list is a good start. Here are some comments I have:

Bug 1150444 isn’t about promises not working properly in debugging sessions for workers. It’s about an intermittent test failure caused by the way we deal with freezing/thawing workers. This test will be obsolete once bug 1178726 lands, and consequently removed, so this bug should resolve itself.

Bug 1175550 was really about a quick prototype that I hoped to land before my 2 months of leave earlier this year. At the time, I didn’t realise that shared workers and service workers are really quite different beasts, so I think we should repurpose that bug to be only about shared workers, and remove it from the meta bug.

Bug 1178726 is about cleaning up how we deal with freezing/thawing workers. Now that we no longer freeze workers when the main thread is paused, this API has become obsolete. This is really just polishing, and doesn’t involve any user visible changes. Consequently, this is not a strict requirement for push notifications. I recommend we remove it from the meta bug.

Bug 1209369 is about more than service worker discovery in my opinion: it includes the features we need to make these service workers debuggable, such as keeping the service worker alive while the debugger is active (bug 1215385), and breaking on start (bug 1215379). On the other hand, other functionality that we need, such as forcibly creating an underlying worker for a service worker so we can load a debugger in it, are missing. I think this bug would be a good candidate as a meta bug for service worker debugging.

Bugs 1209699 and 1209746 are already dependencies of bug 1209369. Since that bug is already a dependency of bug bug 1214248, making the other two bugs direct dependencies of the latter is a bit redundant and unnecessary noise.

Bug 1211903, 1212333 and 1212344, are polish bugs and don’t involve user invisible changes. In general, I recommend we leave non-critical bugs out of the meta bug for push notifications, to reduce noise.

Bug 1212797 should be a dependency of bug 1209369.

Comment 6

[Brian Grinstead [:bgrins]]

(In reply to Axel Kratel from comment #3)
> Brian, I added these to the list, let me know if there are more. 
> 
> 1215117 Make console input field work inside a worker toolbox 
> 1201962	enable service worker logging in web console by default	
> 1209353	Worker debugger console / scratchpad do not work

There's also:

Bug 1215120 - Errors / objects logged in a worker come through as [Object object]

And I'm planning to break a few more out of Bug 1215117 and handle them separately as per https://bugzilla.mozilla.org/show_bug.cgi?id=1215117#c2.  Should I set those to block this bug when I file them?

Comment 7

[Axel Kratel]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> (In reply to Axel Kratel from comment #3)
> > Brian, I added these to the list, let me know if there are more. 
> > 
> > 1215117 Make console input field work inside a worker toolbox 
> > 1201962	enable service worker logging in web console by default	
> > 1209353	Worker debugger console / scratchpad do not work
> 
> There's also:
> 
> Bug 1215120 - Errors / objects logged in a worker come through as [Object
> object]
> 
> And I'm planning to break a few more out of Bug 1215117 and handle them
> separately as per https://bugzilla.mozilla.org/show_bug.cgi?id=1215117#c2. 
> Should I set those to block this bug when I file them?


Just make them block 1215117, that should be enough.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Is this still aimed at 46? Or maybe 47 judging from comments here: https://bugzilla.mozilla.org/show_bug.cgi?id=1209699#c11

Comment 9

[Jan Keromnes [:janx]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8)
> Is this still aimed at 46? Or maybe 47 judging from comments here:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1209699#c11

Hi Liz, we weren't able to land bug 1212797 in time for 46 (because of bug 1242482, bug 1241841, and a few others) which prevented several other bugs in this list from landing, so we decided to skip 46 and ride the train to 47.

[Tracking Requested - why for this release]: Meta-bug for new Service Worker & Push features that we'd like to announce with Developer Edition 47.

Comment 10

[Benjamin Kerensa [:bkerensa]]

Tracking per request in Comment #9

Comment 11

[Jan Keromnes [:janx]]

Marking this meta-bug as fixed for Dev. Edition 47! \o/ Thanks a lot to everyone involved in making this happen.

The scope slightly diverged from the User Story, because most Service Worker and Push features were added to the about:debugging page instead of the Debugger tool.

The following bugs are not yet addressed, but were no longer required for Dev Edition 47 (i.e. they're follow-ups):

Debugger core should support service workers (ejpbruel)
1175550 main 'SW debugging' bug
1212344 [meta] Clean up the worker debugger API. 

Console needs to be usable from SW scope/context while debugging a SW (bgrins)
1209353	Worker debugger console / scratchpad do not work

About:debugging should support service workers (janx)
1209369 SW Discovery for developers
1209746 about:debugging items should include item-appropriate metadata
1215379 about:debugging items should include means to pause a worker on service worker load in debugger
1215386 about:debugging should provide SW lifecycle mngnt: start, stop, unregister