Optimize json sanitizing in Wallet

[atuchin-m]

Currently the wallet code:

1. Registers Brave Wallet data files component even if a user doesn't have a wallet.
2. Reads and parses a few json files using data_decoder::JsonSanitizer::Sanitize. That results in spawning 5 'DataDecoderService` processes during the each startup.

These processes have a short live time, but spawning them takes CPU/memory. That could affect the end user experience because it happens during the startup.

Steps to debug (1):
Alternative way: add a break in DataDecoder::ParseJson;

Steps to debug (2):

1. Launch browser with --trace-startup=toplevel --trace-startup-file=/tmp/trace.json --trace-startup-duration=20
2. Wait 20 sec;
3. Close the browser;
4. Open https://ui.perfetto.dev/ (any browser) and load trace.json;
5. Check the number of data_decoder.mojom.DataDecoderService processes;

Possible optimizations:

1. Don't register the component if the user doesn't have a wallet created.
2. Postpone parsing if possible.
3. Do we really need to sanitize files, taking into account that the component updater verifies CRXs before installation?
4. If we still need to verify them we could use a crypt signature instead of sanitizing.
5. Also combining multiple files into the one can help to improve the startup performance.

[petemill]

Another alternative is use the same DataDecoder instance - that should result in only 1 new process afaik.

[atuchin-m]

Also the best option to spawns zero processes if we don't need them ;)

[diracdeltas]

• Do we really need to sanitize files, taking into account that the component updater verifies CRXs before installation?

Are these internal (AKA trusted) CRX's only? If they are general CRX's like those from the extension stores, they probably need to be sanitized even if they pass signature verification.

[atuchin-m]

@diracdeltas

1. There CRX are not general, they are built in Brave backend.
2. Ids (=the first part of the public key hash) are hardcoded in brave-core.
3. Each component CRX has an extra signature: Add publisher proof support for our CRX3 files #873
4. If we really need, we could sanitize some extra files during the installation (Chromium code does it for manifest.json)

[kdenhartog]

@fmarier and I discussed this and think it's fine to optimize here and consider this trusted content:

From @fmarier:

It's trustworthy data because:

It comes from the browser vendor (Brave).
It's shipped via an integrity-protected transport (component updater).
So according to the rule of two, we could do it in a high-privilege process in a memory unsafe language.

in general, if sanitization is cheap (it's not in this case), then we may as well do it because it adds another layer of defenses.

[yrliou]

Thanks for the input @kdenhartog, I'll remove the call to data_decoder::JsonSanitizer::Sanitize in wallet_data_files_installer.cc then.
I did look at sharing a data decoder process, however, data_decoder::JsonSanitizer::Sanitize calls ParseJsonIsolated on desktop which makes it isolated and per process for each request AFAIK.