Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] Reporter
	Description • 12 years ago

As of bug 765993 whether you are getting source mapped sources or actual sources is dependent on a pref, instead we should have some chrome to let the user turn this on and off.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 1 • 12 years ago

Works, but needs tests.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 2 • 12 years ago

Now the stack frame list is updated correctly. Still needs tests.

I think I'll file a followup for displaying breakpoints after switching over to the other source representation, but we should either add a new protocol request, or use the sourcemaps library in the frontend to find the corresponding lines.

Also, followups for figuring out what to do with the variables view, watch expressions and conditional breakpoints (unless coffeescript expressions are always valid/untranslated JS expressions?).

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 3 • 12 years ago

Simplified onReconfigure() and added a leaky test.
Not on purpose.

	Victor Porof [:vporof][:vp]
	Comment 4 • 12 years ago

Comment on attachment 727770 [details] [diff] [review]
Patch v3

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

::: browser/devtools/debugger/debugger-view.js
@@ +56,5 @@
>      this.Variables.searchPlaceholder = L10N.getStr("emptyVariablesFilterText");
>      this.Variables.emptyText = L10N.getStr("emptyVariablesText");
>      this.Variables.onlyEnumVisible = Prefs.variablesOnlyEnumVisible;
>      this.Variables.searchEnabled = Prefs.variablesSearchboxVisible;
> +    this.Variables.originalSource = Prefs.sourceMapsEnabled;

Why are you attaching this property here (just curious)? Isn't it a bit premature, since nothing is happening with the variables view yet?

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 5 • 12 years ago

(In reply to Victor Porof [:vp] from comment #4)
> ::: browser/devtools/debugger/debugger-view.js
> @@ +56,5 @@
> >      this.Variables.searchPlaceholder = L10N.getStr("emptyVariablesFilterText");
> >      this.Variables.emptyText = L10N.getStr("emptyVariablesText");
> >      this.Variables.onlyEnumVisible = Prefs.variablesOnlyEnumVisible;
> >      this.Variables.searchEnabled = Prefs.variablesSearchboxVisible;
> > +    this.Variables.originalSource = Prefs.sourceMapsEnabled;
> 
> Why are you attaching this property here (just curious)? Isn't it a bit
> premature, since nothing is happening with the variables view yet?

No reason, plain oversight, thanks.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 6 • 12 years ago

A couple of fixes, but the test still leaks.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 7 • 12 years ago

I've been sidetracked from the leak today, trying to figure out why browser_dbg_bug740825_conditional-breakpoints-01.js fails intermittently (more often than not) with this patch. I don't see any relation to the failure, but the test depends on Debugger:AfterFramesCleared, which fires after a timeout, and furthermore conditional breakpoints cause extra protocol requests (due to bug 812172), so it must somehow affect the timing of those events.

I'm seriously considering fixing bug 812172, just to avoid this quagmire.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 8 • 12 years ago

I finally fixed the intermittent failures in the conditional breakpoints test. Getting back to the leak now.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 9 • 12 years ago

Some cleanups, some arrow functions and sending of the sourcemap pref in chrome debugging, too. Because I might want to port the devtools to CoffeeScript next week.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 10 • 12 years ago

Apparently the leaks I was experiencing were caused by the main source maps patch in bug 772119. Rebased, and now all devtools xpcshell and b-c tests, as well as browser m-oth tests pass.

Rob, this is the UI part that was demoed during the work week. The toggle in the gear menu switches between the original and generated sources.

	Panos Astithas (he/him) [:past] (please ni?) Assignee
	Comment 11 • 12 years ago

Try push:
https://tbpl.mozilla.org/?tree=Try&rev=ba2e1fe341e8

	Victor Porof [:vporof][:vp]
	Comment 12 • 12 years ago

https://hg.mozilla.org/integration/fx-team/rev/bd899fcceeab

	Rob Campbell [:rc] (:robcee)
	Comment 13 • 12 years ago

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