1150859 - [meta] Remove preprocessor directives from browser code

Status: NEW

(Firefox :: General, task)

Description

[Mike de Boer [:mikedeboer]]

Preprocessor directive in Javascript and CSS code to include and/ or exclude code from a firefox build creates the following list of - non exhaustive - problems:

 - In JS, stack traces often do not match the actual line numbers, which adds a manual extra step to backtrace where the code is actually hit.
 - A build step is necessary to apply changes made in the source code.
 - The previous point also means that a developer needs to memorize for which part of the browser she/ he is working on a (re-)build is necessary.
 - A build step slows down fast (re-)iterative development and rapid prototyping.

Wherever possible we should be trying minimize or even eliminate the use of `#include` and `#ifdef` preprocessors throughout the 'browser/' sub-tree.

'Plan de campagne' as it currently stands:
 - Write a static analysis tool in JS, running on XPCShell to read, parse and analyze the browser JS source files. It will operate in three phases:
    1) List & count the current scope of use of preprocessor directives
    2) For source files that are #include'd, list & count the number of implicit global variables
    3) Track the numbers over time.
 - Depending on practical feasibility, write a source refactor tool in JS, running on XPCShell to use the data produced by aforementioned script to apply (minor) code modifications that are verifiable correct.
 - Hook the analysis script to automation as a pre-receive hook in Mercurial, to prevent adding more preprocessor dependent code. It is possible to use another hook - the earlier we can run the script the better - but for that to know we'd like to request gps' help.
 - Spin off bugs based on the initial results of the analysis script; the intention is to slowly iterate towards moving all #include'd files to be JS modules that are retrieved lazily by browser.js. The #ifdef removal is second priority.

djc and mikedeboer are working on the analysis script - based on Reflect.parse - which will be the cornerstone of this endeavor. The bugs that will be filed based on the results it yields will be filed blocking this (meta) bug.

Comment 1

[Mike de Boer [:mikedeboer]]

(In reply to Mike de Boer [:mikedeboer] from comment #0)
> Preprocessor directive in Javascript and CSS code to include and/ or exclude
> code from a firefox build creates the following list of - non exhaustive -
> problems:

I meant that the *list* is non-exhaustive.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Note that many preprocessor directives in JS code can be replaced with
- AppConstants.jsm (which relies itself on preprocessor directives, but we could reimplement it in C++ if needed);
- OS.Constants (which is a bit heavyweight to use right now, because it is tied to OS.File, but we could easily extract it to a more lightweight module).

Comment 3

[Mike de Boer [:mikedeboer]]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #2)
> Note that many preprocessor directives in JS code can be replaced with
> - AppConstants.jsm (which relies itself on preprocessor directives, but we
> could reimplement it in C++ if needed);

I think that's great! Keeping that file in JS is fine, methinks.

Comment 4

[Dirkjan Ochtman (:djc)]

I wonder if it makes sense to separate the eradication of #include and #ifdef directives into separate bugs, since they will probably need different strategies.

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #2)
> Note that many preprocessor directives in JS code can be replaced with
> - AppConstants.jsm (which relies itself on preprocessor directives, but we
> could reimplement it in C++ if needed);

Hey, look, I already started on that work:
https://dxr.mozilla.org/mozilla-central/source/xpcom/system/nsIXULRuntime.idl#139

I needed this to remove preprocessing in a bunch of test files. Adding more things there is trivial, we could easily use that for everything in AppConstants.jsm:
https://hg.mozilla.org/mozilla-central/rev/a34e6d5519ef

Comment 6

[:Gijs (he/him)]

I've started filing deps to remove these. I'll try to finish this tomorrow / the rest of this week. Please feel free to take and/or sign up as mentors for any of them. I'll post to fx-dev once browser/ is covered.

Comment 7

[Mark Banner (:standard8)]

Just a reminder: as we eradicate preprocessor directives completely from files, we should also update the associated moz.build/jar.mn to avoid the preprocessing and hence speeding up build times and get the full benefits of the removal of the directives.