Safebrowsing removal in windows

[Luka-Filipovic]

Why does windows build require more in depth removal of safebrowsing compared to other platforms? Would insertion of windows-fix-building-without-safebrowsing.patch to other platform builds results in better safebrowsing removal since big part of code removed is not windows specific?

[Eloston]

Why does windows build require more in depth removal of safebrowsing compared to other platforms?

A while ago, I had problems trying to update the Windows version without removing Safe Browsing entirely. I think this pattern has since carried forward.

Would insertion of windows-fix-building-without-safebrowsing.patch to other platform builds results in better safebrowsing removal since big part of code removed is not windows specific?

I've wondered about this for a while. It'd be beneficial for macOS, and could solve some bugs in Linux builds relating to the partially removed Safe Browsing code.

@Zoraver Since you've updated ungoogled-chromium in recent versions, what are your thoughts on merging changes from this Windows patch back into ungoogled-chromium?

[Luka-Filipovic]

I will give it a try applying it on macos build. So far I realised everything in fix-disabling-safebrowsing.patch in macos repo is done in windows-fix-building-without-safebrowsing.patch as well, and there's probably something similar in linux repos.

[Luka-Filipovic]

Successfully built and working on macOS with some minor changes. Merging windows-fix-building-without-safebrowsing.patch into fix-building-without-safebrowsing.patch results in 6k lines long patch, which is painful to work with. Maybe finding some meaningful logic to split it in more smaller patches?

@Eloston can this issue be moved to core repo?

[Zoraver]

@Eloston I don't think that merging windows-fix-building-without-safebrowsing.patch back into ungoogled-chromium is a good idea. Although the macOS-specific fix-disabling-safebrowsing.patch does contain a subset of the changes, it is less than one tenth the size and not much of a hassle to maintain. As far as I know, no similar patch is required to build on Linux.

From my perspective, merging it back to ungoogled-chromium will just increase the (already nontrivial) amount of work required to port ungoogled-chromium to a new major version of Chromium with little to no payoff for people who don't use Windows. This will likely result in longer delays between the release of a new major version of Chromium and ungoogled-chromium being available for it.

[Luka-Filipovic]

Maybe it can be left as optinal, so each version bump can be done without it and it would be left to the bravest among us to work on. That patch is mostly removal of large chunks of code or removal of safebrowsing headers. Large chunks removal can be modified by using flags, it would make patch smaller and would bring less headache when the new version is released. As for headers removal, maybe it could be automated to remove certain headers in certain files? The main reason why I opened this issue is to question what's the difference under the hood comparing windows and other platforms UC. Is it the same product since one has 5k more lines to remove?

[Eloston]

@Eloston can this issue be moved to core repo?

The core repo currently lives under my personal account, whereas this repo lives under the ungoogled-software organization. GitHub won't allow me to move issues across users/organizations. As a workaround, we could cross-link this issue to a new one, but I don't think that's immediately necessary.

As far as I know, no similar patch is required to build on Linux.

The reason I suggested the idea is we will sometimes have obscure crashing bugs (e.g. migrating Chrome profiles to ungoogled-chromium) because we don't completely patch out the components we do not want. Technically it is safer to completely remove components we do not want, because it forces us to properly fix all code paths during compile time.

However, I agree with your point that our current development workflow would make updates prohibitively slow. Considering the majority of use-cases (most don't migrate profiles from Chrome) and platforms we support (Linux is probably the most popular in our user base), it is probably not a good idea to merge this Safe Browsing patch into the main repo right now. I'll expand on this point a bit more below.

As for headers removal, maybe it could be automated to remove certain headers in certain files?

This is a part of my idea in https://github.com/Eloston/ungoogled-chromium/issues/1087. In theory, that new tool would make it feasible to maintain a huge Safe Browsing patch. However, we'd need to do quite a bit of research first to test the feasibility of the tool. For example, I'm interested to see how practical it is to use AST representations of the C++ code to automate common patch-breaking code changes.

In summary, I don't think there's much we can do about this issue until we develop that better tool. Of course, I'm also open to other suggestions.