Open Bug 1410184 Opened 7 years ago Updated 1 year ago

Investigate replacing the JS CSS lexer with Stylo's lexer in WebAssembly

Categories

(DevTools :: Inspector, enhancement, P3)

Product:
DevTools
Component:
Inspector
Type:
enhancement

Tracking

(Not tracked)

People

(Reporter: jryans, Unassigned)

References

(Depends on 1 open bug)

Details

(Whiteboard: [designer-tools][stylo-])

In bug 1265787, we landed a JS version of Gecko's CSS lexer, so that the inspector could use it when running in a tab. Now that the platform is moving towards Stylo, it would be good to adapt this to be based on Stylo's lexer instead. At the same time, it would also be an interesting test case for compiling Rust to WebAssembly, which should improve speed over the pure JS version we have today.
Whiteboard: [designer-tools]
status-firefox57=wontfix unless someone thinks this bug should block 57
status-firefox57: --- → wontfix
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Tagging this bug "[stylo-]" to remove it from Style bug triage queries because this is not a Stylo bug.
Whiteboard: [designer-tools] → [designer-tools][stylo-]
Slowly making some progress on this task. The Rust WASM tooling advances quickly, so I had been waiting a bit to let it mature first. I hit a conflict between wasm-bindgen and cssparser while exploring this, but I have sent a PR to workaround it: https://github.com/rustwasm/wasm-bindgen/pull/165
Product: Firefox → DevTools
No longer blocks: stylo-everywhere
No longer blocks: 1418874
After thinking about bug 1410716 a bit more, I think it is pretty likely that changing to use WASM for the lexer would regress that bug to some extent, because WASM cannot access normal JS objects, and thus we have to duplicate the CSS content into WASM's memory before invoking it. And I think a proper solution to that bug is to construct lexer only once for each stylesheet, so that we don't repeatedly duplicate the content. That would probably be a prerequisite to this bug then.
Depends on: 1410716
Assignee: jryans → nobody
Status: ASSIGNED → NEW
Severity: normal → S3

In some early analysis for Bug 1836873 , I came across our JS-written CSS lexer again, which I guess should be modified to account for the new syntax added for CSS Nesting.

I was wondering if that would be a good time to try to revive this specific bug instead of replicating work that was already done on the platform.
Another solution that we could consider is to allow (privileged) JS to directly call the Stylo Lexer, so we won't have to go through the WASM conversion (at the time this bug was created, we wanted to get rid of privileged code in DevTools, but this isn't the case anymore).

Emilio, what do you think about this? Does this sounds like something that would be hard to do/not worth the benefit?

Flags: needinfo?(emilio)

What kind of API would you need from the platform? cssparser is rather standalone so compiling to wasm shouldn't be hard in theory... But if a WebIDL/XPIDL api is simpler that's fine too. Do you just want a tokenized range of source text? Something else

Flags: needinfo?(emilio) → needinfo?(nchevobbe)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

What kind of API would you need from the platform? cssparser is rather standalone so compiling to wasm shouldn't be hard in theory... But if a WebIDL/XPIDL api is simpler that's fine too. Do you just want a tokenized range of source text? Something else

Yeah a tokenizer would be great so we can replace https://searchfox.org/mozilla-central/rev/27e4816536c891d85d63695025f2549fd7976392/devtools/shared/css/parsing-utils.js#42-111

Flags: needinfo?(nchevobbe)
You need to log in before you can comment on or make changes to this bug.