ungoogled-software / ungoogled-chromium-android

Android build for ungoogled-chromium
GNU General Public License v3.0
491 stars 43 forks source link

Updating Android-specific patches for v78; new patches #5

Closed csagan5 closed 4 years ago

csagan5 commented 4 years ago

This PR does the following:

NOTE: this is to be considered a work-in-progress, since ungoogled-chromium is not yet at v78, I opened it to discuss with @wchen342 if he likes this approach

wchen342 commented 4 years ago

@csagan5 Yeah this can be helpful. I think there needs to be some guildline on what patches is necessary for ungoogled-chromium though, cause Bromite covers a lot more functions than ungoogled-chromium intends to have. @Eloston do you have any suggestions?

Also some of the Unobtainium patches are outdated and overlap with what I already have, like the kill-Vision/GCM and kill-TOS-and-metrics-opt-out ones. I didn't test but I think you cannot apply them if my patches are already applied.

csagan5 commented 4 years ago

ungoogled-chromium is not yet at v78 I was wrong here by the way, as I see the repo has been updated for a while now.

I think there needs to be some guildline on what patches is necessary for ungoogled-chromium though, cause Bromite covers a lot more functions

Yes, in this PR I am only proposing patches which are relevant; I had to update both the Unobtainium and the ungoogled-chromium-android patches to resolve conflicts and thus I put them one after another while building.

Also some of the Unobtainium patches are outdated and overlap with what I already have, like the kill-Vision/GCM and kill-TOS-and-metrics-opt-out ones. I didn't test but I think you cannot apply them if my patches are already applied.

The whole series here is tested to apply without conflicts and build; I did however not build with all the other ungoogled-chromium patches which would normally be applied before, as I am testing on top of a Bromite hybrid tree for now.

Basically what I did is give precedence to your patches while solving conflicts and still keep the delta of the kill-* patches, which you did not have (although not sure how useful they are). The kill-* patches in this PR only maintain that remainder delta.

Eloston commented 4 years ago

@wchen342 I think what is essential is listed in ungoogled-chromium's README (in decreasing importance):

  1. Remove all remaining background requests to any web services while building and running the browser
  2. Remove all code specific to Google web services
  3. Remove all uses of pre-made binaries from the source code, and replace them with user-provided alternatives when possible.
  4. Disable features that inhibit control and transparency, and add or modify features that promote them (these changes will almost always require manual activation or enabling).

I think we can extend point 2 to include any Google services and frameworks. Also, point 4 is open-ended, so technically any new feature from Bromite is allowable as long as it toggleable in runtime configuration and disabled by default (there are exceptions depending on the specifics of the feature).

All the patches in this PR seem to fit ungoogled-chromium's objectives. I don't know have enough context on Android to accept this PR, though.

wchen342 commented 4 years ago

The whole series here is tested to apply without conflicts and build; I did however not build with all the other ungoogled-chromium patches which would normally be applied before, as I am testing on top of a Bromite hybrid tree for now.

Ok then I will update the main repo to v78 first and test the patches. Once the build is successful then I can merge this.

csagan5 commented 4 years ago

Ok, thanks for the feedback guys; my thinking was that the patches made by @wchen342 replaced the kill-* patches from Unobtainium, so I was surprised to find some remainder difference and honestly I do not know whether they are necessary or an improvement, it just feels like a waste to discard them at this point.

My secondary goal is to "upstream" as many patches as possible from Bromite into ungoogled-chromium/ungoogled-chromium-android, as I see fit.

@wchen342 ~I will update the patches~ (they are already up-to-date) in this PR with the latest version I have; I will also start in the future to test the full build the same way you do.

wchen342 commented 4 years ago

so I was surprised to find some remainder difference and honestly I do not know whether they are necessary or an improvement

When I made the patches I tried to make as few changes as possible so future updates won't bacome a huge pain. I only made the changes such that Google Play packages are no longer required to build the apk, but I did not try to disable every function related to them as long as those functions are not using proprietary packages.

My secondary goal is to "upstream" as many patches as possible from Bromite into ungoogled-chromium/ungoogled-chromium-android

Sure, I see Bromite has some useful functions like DNS-over-https.

There is a problems though, and I should probably open an issue for this in the main repo too. So based on @Eloston 's reply above any additional function should be changable at runtime, presumably through flags. The problem is, there are already too many options to turn on/off in chrome://flags. It is hard to view on a computer screen and will be even harder to view on a phone screen.

Eloston commented 4 years ago

The problem is, there are already too many options to turn on/off in chrome://flags. It is hard to view on a computer screen and will be even harder to view on a phone screen.

This is not ideal, but there are no easy alternatives:

  1. Add a new chrome://settings page. However, some of these options are processed late or are multiple layers away from the profile settings, so this is not trivial to implement.
  2. Add some way to expand/collapse groups of options, or add new tab (alongside Available/Unavailable) inside the chrome://flags page.
  3. Add a new page like chrome://flags that extends the same underlying mechanism (this is similar to Option 2, but it's a completely new page).
csagan5 commented 4 years ago

I see Bromite has some useful functions like DNS-over-https.

The real deal there would be to implement some UI to enter a custom URL, I could not get around to do that, it needs some work.

based on @Eloston 's reply above any additional function should be changable at runtime

For DNS-over-HTTPS it's already working like that, and disabled by default; but I was not thinking to submit the patch for that feature (although you can always grab it independently).

I am more thinking about patches that do not require a toggle because they are disabling more cloud integrations, and these should be the majority of Bromite's patches; of course if someone can think of a way one might want to keep them, I would agree with @Eloston and the toggle approach.

csagan5 commented 4 years ago

Another feature worth mentioning is the subresource filter, I did quite some work for it in Bromite; see my comments in https://github.com/GrapheneOS/Vanadium/pull/42

csagan5 commented 4 years ago

I tried to make as few changes as possible so future updates won't become a huge pain.

I see what you mean; I have the same goal, but unfortunately for patches which "remove" code like these in this PR, we are out of luck.

The best approach to be honest would be to disable the .java file from the relevant BUILD.gn and replace it with a new file which contains the stubbed class, this way we avoid the rebase hell with each release.

wchen342 commented 4 years ago

The patches mostly works but need some small fixes. I will bring up those in the update to a new version.