Thoughts about working on our CSS parser

Discussions about the development and maturation of the platform code (UXP).
Warning: may contain highly-technical topics.

Moderators: trava90, athenian200

User avatar
athenian200
Contributing developer
Contributing developer
Posts: 1498
Joined: 2018-10-28, 19:56
Location: Georgia

Thoughts about working on our CSS parser

Unread post by athenian200 » 2020-10-01, 12:53

I've been meaning to talk about this subject for a while, but since we're doing more work in this part of the code lately, I thought it would be relevant for me to share what I've learned from working on this. The context is that I've actually been slowly learning this part of the code by doing various minor issues in preparation for eventually doing the CSS parts of WebComponents, because I believe I would need a better sense of how it all fits together and what the pitfalls of extending it are before I attempt to implement something as non-trivial as a pseudo-class along the same lines as the entire scoped styles implementation. I'm a simple ex-Computer Science student with no degree, so I know my limits. Messing with this code is actually incredibly dangerous and not for the faint of heart. Here are the things I've learned about trying to work with it so far:

1. GDB usually won't save you, so you are very much on your own with only assertions and logging to catch potential problems (at least on Unix).

There are multiple reasons for this. The main one is that if you do make a mistake implementing something in the CSS code, the program will more than likely crash so early that the debugger won't even have time to reach the offending part of the code. A core dump will probably show an unhelpful stack trace that leads to something irrelevant like the code that handles reflowing the document rather than the actual error. Even if your implementation is good enough that things don't crash, many elements of the CSS parser are implemented as loops on "hot" code paths that are iterated through so many times during program execution that if you hooked the debugger up, you would find it hits the code so many times that breakpoints become largely useless. So you might as well consider that tool off the table. I primarily develop on OpenIndiana and Oracle Solaris, but I'm told this is definitely the case for GDB on Linux as well. I haven't investigated whether the situation is better for Windows with MSVC, which may have a more advanced debugger. If that's the case, we may have to recommend all CSS parser development be done on Windows.

2. Making sure things don't work when they shouldn't, is just as important as making sure they do work when they should.

The CSS parser tends to reuse the same code for so many different things that if you change one part of it, you're likely changing how things work on a global level for all CSS parsing unless you're very, very careful and add a new special case to the code very carefully. You always want to be on guard for the possibility that you implemented or removed something in a way that affects far more than what you intended, which means you not only have to test whether your changes do what you wanted, you have to explicitly make sure your changes do nothing in situations where you want things to behave the same as before. It's a very hard habit to develop, and I'm still learning it myself. Many parts of the CSS parser appear to be implemented in a certain coding style that strongly encourages code reuse (it might be an early OOP paradigm, but I'm not sure on that). A situation that no longer allows the code to work in exactly the same way for everything it did before often requires creating a new object or case, adding that new case to several lists essentially specifying that it can reuse several parts that are still fine, and then writing the new segment of code you really wanted that's only invoked in your new special case while leaving the old code alone. Implementing a new feature that only calls on existing functionality, however, is relatively easy and often means you can just fill out an obvious "form" that calls on the functionality you want from various lists of things previously implemented as objects or cases.

3. Test the browser thoroughly after making changes here, and avoid making major changes to the CSS parser late in the development cycle of a release.

This part of the code is incredibly fragile and involves a lot of trial and error. It's almost guaranteed that anything you try to implement here (unless it's very straightforwards) will have severe bugs on your first attempt, often in unexpected ways that are hard to diagnose. You want as many people stress-testing the implementation as possible, and without that there's a very high risk of releasing something that's not fully baked. If you were used to working on UXP and having things that could go wrong with your code be fairly straightforward to fix, that stops here and now. CSS parser work is a good example of the reason why our unstable releases are so important to ensuring the integrity of our final releases, and how important it is for people to help us out with beta testing whenever they can. For instance, I accidentally caused percentages to serialize as floating point numbers recently in several cases where percentages can't serialize as percentages, rather than just in the intended case. This was caught soon after release, but ideally we want to catch these things before release.

4. Think carefully about whether CSS parser changes are actually needed to accomplish what you want to do.

In some cases, it's much easier and less risky to change another part of the code that interacts with the CSS parser, for instance DOM, than to actually mess with the CSS parser itself. If what you want to do can be done without touching the CSS parser, it's often preferable to change a different part of the code and leave the CSS parser alone. Usually this won't be possible, but for situations where it is... you should probably do yourself a favor and try to avoid unnecessary changes to it.

Anyway, those are the things I've learned, on an abstract level, about working with the CSS code. I'm also working on trying to describe how data moves from one part of the CSS parser to another and what happens in between, in a way that's easier than just reading all of the code so that people who come along later to work on it can have an idea of what needs to be touched.
"The Athenians, however, represent the unity of these opposites; in them, mind or spirit has emerged from the Theban subjectivity without losing itself in the Spartan objectivity of ethical life. With the Athenians, the rights of the State and of the individual found as perfect a union as was possible at all at the level of the Greek spirit." -- Hegel's philosophy of Mind

New Tobin Paradigm

Re: Thoughts about working on our CSS parser

Unread post by New Tobin Paradigm » 2020-10-01, 13:20

My only comment except to tell you how proud we all are of you for getting to a point where you can write such a post is that perhaps Windows debugging is gonna have to be recommended for dealing with our CSS Parser because if it crashes you can instantly open it in a debugger without prior attachment and perform a stack trace and other debugging operations I am still very new to.

Nix debugging is obviously not going to be sufficient.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35477
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Thoughts about working on our CSS parser

Unread post by Moonchild » 2020-10-01, 13:27

5. Make sure new code you implement that is non-trivial is behind a preference (about:config), if possible/feasible.

This will allow things to be switched off if they turn out to be unstable or break major sites in the wild (see e.g. the recent ResizeObserver issue) as well as allow careful testing only for the brave souls that want to by defaulting it off, even if you think your implementation is complete as-is. Preferences can always be removed later if a feature ends up stable and in final form.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

vannilla
Moon Magic practitioner
Moon Magic practitioner
Posts: 2183
Joined: 2018-05-05, 13:29

Re: Thoughts about working on our CSS parser

Unread post by vannilla » 2020-10-01, 14:31

Thanks for the write-up.
I just want to say that with GDB, stopping at a breakpoint only when a certain condition is met (say, a certain variable has a certain value) is theoreitcally possible, however the exact incantation, especially for UXP, would require a rather deep research.
Never worked with the Windows debugger so I can't tell how different it is. I can only speak about GDB and will refrain from making comparisons.

New Tobin Paradigm

Re: Thoughts about working on our CSS parser

Unread post by New Tobin Paradigm » 2020-10-01, 14:57

Well of course I mean the Visual Studio debugger.

Locked