1232014 - Stub out wasm source files in the Debugger

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

Once initial wasm support lands (mid-Q1?, no bug AFAIK), we need to stub out support for this in the debugger, so that we just display the source item in the debugger -- we don't need to display anything in the source code panel (a message maybe indicating that this is bleeding edge and coming soon?), but just basic groundwork for eventual support.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Give wasm Debugger.Sources the text/wasm mimetype and use their .text property.

Comment 2

[Eddy Bruel [:ejpbruel]]

Comment on attachment 8728913 [details] [diff] [review]
Give wasm Debugger.Sources the text/wasm mimetype and use their .text property.

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

Easy patch. I have a few questions, but otherwise looks good to go.

::: devtools/server/actors/source.js
@@ +338,4 @@
>        if (this.source &&
>            this.source.text !== "[no source]" &&
>            this._contentType &&
> +          (this._contentType.indexOf('javascript') !== -1 ||

Nit: I know the line above it also didn't do it, but I prefer using "" over '' for strings.

::: devtools/server/actors/utils/TabSources.js
@@ +306,5 @@
>          if (url.indexOf("Scratchpad/") === 0 ||
>              url.indexOf("javascript:") === 0 ||
>              url === "debugger eval code") {
>            spec.contentType = "text/javascript";
> +        } else if (aSource.introductionType === "wasm") {

Gah, yet another special case here :-(

My understanding is that introductionType is meant to indicate *what* introduced this particular source, be it an event handler, an eval statement, the Function constructor, etc. Within that light, does it make sense to consider wasm as another thing that can introduce a source?

Assuming you've already thought that through, how about moving this if branch to the top, and adding a comment like 'if this source was introduced by wasm, assume its MIME type is type/wasm. Otherwise, try to obtain the mime type from the URL'.

Comment 3

[Shu-yu Guo [:shu]]

(In reply to Eddy Bruel [:ejpbruel] from comment #2)
> Comment on attachment 8728913 [details] [diff] [review]
> Give wasm Debugger.Sources the text/wasm mimetype and use their .text
> property.
> 
> Review of attachment 8728913 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Easy patch. I have a few questions, but otherwise looks good to go.
> 
> ::: devtools/server/actors/utils/TabSources.js
> @@ +306,5 @@
> >          if (url.indexOf("Scratchpad/") === 0 ||
> >              url.indexOf("javascript:") === 0 ||
> >              url === "debugger eval code") {
> >            spec.contentType = "text/javascript";
> > +        } else if (aSource.introductionType === "wasm") {
> 
> Gah, yet another special case here :-(
> 
> My understanding is that introductionType is meant to indicate *what*
> introduced this particular source, be it an event handler, an eval
> statement, the Function constructor, etc. Within that light, does it make
> sense to consider wasm as another thing that can introduce a source?
> 

This is really a philosophical question. Wasm code doesn't have "sources" at all, they have to be synthesized. In that sense, these sources are "introduced by" wasm. Originally I had an additional .format property on Debugger.Source that returned "js" or "wasm", but I felt that was redundant. This isn't set in stone, but I see no compelling argument to changing it for the initial landing.

> Assuming you've already thought that through, how about moving this if
> branch to the top, and adding a comment like 'if this source was introduced
> by wasm, assume its MIME type is type/wasm. Otherwise, try to obtain the
> mime type from the URL'.

Sure, will do.

Comment 4

[Shu-yu Guo [:shu]]

A nit on the review about "assume its content type". It isn't really an assumption like the JS cases, because having an intro type of "wasm" means we know exactly what type its source text should be treated as.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d89e927c2b7b

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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