Once bug 1168853 lands we'll have all the functionality we need to start building a UI for worker debugging. The plan is to land this UI incrementally and early, hiding it behind a pref for the time being. This bug will be about adding the pref to enable worker debugging, and displaying a list of workers for the current tab.
Created attachment 8612924 [details] [diff] [review] Work in progress This patch isn't quite ready for review yet, but I wanted to give you the opportunity to get an early look at it, so you'll have some idea of what I'm doing. This patch does two things: 1. It adds a preference to the toolbox options to enable worker debugging 2. It splits the sources view into a worker view and sources view. One subtlety is that when a worker is frozen (because its page moves into the bfcache), it needs to be hidden in the worker view, even though its still in the worker list (this is a result of the fact that we use the tab window to filter the list of all workers, and that window didn't change when the page was cached). I would like to improve the patch so that the worker list takes up as little space as possible, and shows (No workers) when the list is empty. Also, I'm getting a weird error message about _itemsByElement not being defined when the worker list is updated right after enabling worker debugging in the preferences pane (this shouldn't happen, because _itemsByElement is defined by WidgetMethods).
Cool, mind taking a screenshot of what it looks like? I can try to build the patch in a bit if you don't see this soon.
Created attachment 8615986 [details] [diff] [review] Implement DebuggerView.Workers Alright, I think this patch is now polished enough to be ready for review.
(In reply to James Long (:jlongster) from comment #2) > Cool, mind taking a screenshot of what it looks like? I can try to build the > patch in a bit if you don't see this soon. Do you still need a screenshot of this? I've shown you what it looks like during the devtools team meeting :-)
(In reply to Eddy Bruel [:ejpbruel] from comment #4) > (In reply to James Long (:jlongster) from comment #2) > > Cool, mind taking a screenshot of what it looks like? I can try to build the > > patch in a bit if you don't see this soon. > > Do you still need a screenshot of this? I've shown you what it looks like > during the devtools team meeting :-) FWIW it's great to have a screenshot on the bug for future reference. Plus I missed the demo due to connection issues so I'd like to see it also if you don't mind.
Comment on attachment 8615986 [details] [diff] [review] Implement DebuggerView.Workers Review of attachment 8615986 [details] [diff] [review]: ----------------------------------------------------------------- Looks generally OK, and shouldn't conflict much with my refactor. ::: browser/devtools/debugger/test/browser_dbg_panel-size.js @@ +16,5 @@ > gTab = aTab; > gPanel = aPanel; > gDebugger = gPanel.panelWin; > gPrefs = gDebugger.Prefs; > + gWorkersAndSources = gDebugger.document.getElementById("workers-and-sources-pane"); Did you actually need to change anything in this test other than this ID? I would prefer to minimize changing variable names because we are most likely going to end up with a different UI anyway (that list really need to go somewhere else, the left panel is already too complicated, though I'm not sure where yet) It's a little annoying to land this with a "quick" UI that we know will change, but I understand why we should do that. I'd like to minimize the amount of backtracking we will do when we finalize the UI though.
Attachment #8615986 - Flags: review?(jlong) → review+
Attachment #8615986 - Attachment description: Implement WorkersView → Implement DebuggerView.Workers
Summary: Implement a UI to list the workers in a tab → Implement DebuggerView.Workers
Try push for the DebuggerView.Workers patch (with comments by jlong addressed): https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bb9c6ce236e
The try push shows some test failures for browser_graphs-11b.js but these look like they were caused by earlier commits that have since been backed out (see 288cd0b9c9a3).
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.