wchen342 / ungoogled-chromium-android

Android build for ungoogled-chromium
https://uc.droidware.info
GNU General Public License v3.0
129 stars 56 forks source link

Allow to install Chrome extensions #8

Closed thfi closed 4 years ago

thfi commented 5 years ago

Please make it possible to install Chrome extensions in Ungoogled Chromium for Android. Another Chromium/Chrome-based browser, Kiwi, was also able to achieve this.

wchen342 commented 5 years ago

This is actually not a easy task. I had a quick look at the browser you mentioned. Turns out they haven't open sourced their implementation so it needs to wait. I agree that extensions would be useful though.

wchen342 commented 5 years ago

Correction: I found the project on github but it is 6 months old based on a v69 chromium. It will take time to figure out how they managed to achieve this.

csagan5 commented 5 years ago

Correction: I found the project on github but it is 6 months old based on a v69 chromium. It will take time to figure out how they managed to achieve this.

@wchen342 see https://github.com/kiwibrowser/android/issues/12

There's nothing there.

There is ancient patch here: https://github.com/kiwibrowser/privacy_patches/blob/master/Add-support-for-extensions-on-Android.patch

But you would miss all the fixes/development done afterwards.

dimqua commented 5 years ago

BTW, Yandex.Browser for Android supports Chrome extensions too, but its also proprietary.

DI555 commented 4 years ago

good news btw, the brave team is finally plan to add support of extensions in 1-2 Q2020 (from their reddit blog) ...so will possible to port their modifications about that!!!

wchen342 commented 4 years ago

@DI555 That's good to hear. It will take some time to port the modifications because from what I know it takes quite a lot of modifications to support extensions.

androidacy-user commented 4 years ago

Just wanted to bump this issue :P

csagan5 commented 4 years ago

That's good to hear. It will take some time to port the modifications because from what I know it takes quite a lot of modifications to support extensions.

It would be great if the Brave team also created neatly separated commits for this functionality (following git best practices), so that it's possible for other parties to adopt the feature. If you look at the current repository, that is not the case.

DI555 commented 4 years ago

really, i hope for this time, good news!!! reddit.com/r/Android/comments/g3kgsf/kiwi_browser_is_now_fully_opensource_including/

wchen342 commented 4 years ago

@DI555 Interesting. I had a quick look and I think their implementation is better in the sense that it is a separate module. They said they didn't strictly follow upstream though so no idea how much conflicts/errors will be there. I'm dealing with some real life problems right now (crazy times..) but once things got stable I will start to integrate this.

csagan5 commented 4 years ago

@wchen342 the stand-alone patch is released, see https://github.com/bromite/bromite/issues/533

Haven't tried it though.

wchen342 commented 4 years ago

@csagan5 I'm building a new version right now so I will test this.

wchen342 commented 4 years ago

@csagan5 I cleaned out the patch a bit and here it is: https://gist.github.com/wchen342/619fe595eae8dc0440ad0925ff7e863c. There are still some errors though. I'm currently stuck at sth like

ERROR Input to targets not generated by a dependency.
The file:
  //out/Default/gen/extensions/strings/extensions_strings_af.pak
is listed as an input or source for the targets:
  //chrome/android:chrome_apk_paks_locales_af
  //chrome/android:chrome_bundle_module_paks_locales_af
  //chrome/android:monochrome_apk_paks_locales_af
  //chrome/android:monochrome_bundle_module_paks_locales_af
  //chrome/android:trichrome_chrome_bundle_module_paks_locales_af
but no targets in the build generate that file.

ERROR Input to targets not generated by a dependency.
The file:
  //out/Default/gen/extensions/strings/extensions_strings_as.pak
is listed as an input or source for the targets:
  //chrome/android:chrome_apk_paks_locales_as
  //chrome/android:chrome_bundle_module_paks_locales_as
  //chrome/android:monochrome_apk_paks_locales_as
  //chrome/android:monochrome_bundle_module_paks_locales_as
  //chrome/android:trichrome_chrome_bundle_module_paks_locales_as
but no targets in the build generate that file.

, not sure how to solve this.

Also the patch seems to be not only for extension (it mixes with some other modifications like safe browsing). And they like to spam if true || so needs to double check to make sure nothing is enabled by accident.

csagan5 commented 4 years ago

Input to targets not generated by a dependency.

A change in some BUILD.gn file is needed; you could see how other dependencies work to fix this.

Also the patch seems to be not only for extension

I see; I am not integrating this patch myself, I mentioned it in case you might find it useful.

kiwibrowser commented 4 years ago

@wchen342 I can help you, but I think it's better we do it in Discord ( https://discord.gg/XyMppQq @ arnaud42 ) as there can be lot of back-and-forth discussions.

@csagan5 The patch disables safe-browsing and enable enable_plugins are they are inter-dependent (I know on the paper, it's not supposed to be like this, but in practice it is like that in Chromium)

csagan5 commented 4 years ago

The discussions in Discord or other chat systems are not accessible publicly (having public discussions is necessary for open source) and in context of the development being done, but I assume @wchen342 you are already aware of https://github.com/kiwibrowser/src/issues/13#issuecomment-619448584 by @dvalter?

wchen342 commented 4 years ago

@csagan5 Yeah I am aware of that. If I can have a successful build then I can keep this patch at the top of series so it can be applied on a source tree as close as possible to genuine chromium (in my case, after Eloston's patches).

csagan5 commented 4 years ago

I can keep this patch at the top of series so it can be applied on a source tree as close as possible to genuine chromium (in my case, after Eloston's patches).

I see; his patch is here: https://github.com/dvalter/chromium_extension_patches (haven't tried it myself).

wchen342 commented 4 years ago

@csagan5 I tried that one and still got stuck at the same error. This particular gn problem is in a section very convoluted. I may need to dig into gn source code to find out what's really going on.

EDIT: ok I found the problem. Used a easy fix to just remove the dependency. Now I am trying to figure out the minimal modifications to make things work.

wchen342 commented 4 years ago

I splitted the patches into gn, src and resources. Not sure whether there is a better way of managing them since I made the patches in an error-driven way and most of the modifications are correlated. The patches should now build without error on 81.0.4044.129, but the UI is still missing. @csagan5 Do you have any idea on how to better manage the patches?

TotalCaesar659 commented 4 years ago

The patches should now build without error on 81.0.4044.129, but the UI is still missing.

There are new extension menu in flags (https://www.theregister.co.uk/2020/04/07/chrome_hiding_extensions/). It's good idea to implement it when it arrives to stable Chromium for desktop.

dvalter commented 4 years ago

@wchen342 a few hour ago I pushed updates that enabled extensions API stuff and UI. Additionally I published my chromium git that hopefully gives a better view of what I did.

There are two trees corresponding to two patches:

I also updated the status on the current bugs and what I tested.

Over the next few months I expect to have less free time to work with this stuff, so there may not be any reasonable outcome from myself, but I'm going to stay in touch so feel free to ask.

wchen342 commented 4 years ago

@TotalCaesar659 I was talking about the Java/UI part of the patch (that adds the ability of installing extensions) but thanks for the heads up about the flag.

@dvalter I ended up taking a slightly different path from the one kiwi used, but your patch has been a great reference for me and saved me some time writing android specific files. Thanks for the great work!

csagan5 commented 4 years ago

@csagan5 Do you have any idea on how to better manage the patches?

As I understand it's a big changeset, so no silver bullet here to manage the patches. To maintain them over time the best would be to use quilt but I do not have any hint on how to split specifically this big patch for the extensions.

eklavya002 commented 4 years ago

what's the latest progress in this regard?

wchen342 commented 4 years ago

I'm in the process of debugging several startup problems that will cause a crash. I hope I can get some prototype out by next version but depending on how many remaining problems are the time will vary.

wchen342 commented 4 years ago

@dvalter Did you run into a SEGV_MAPERR caused by HostContentSettingsMapFactory/CookieSettingsFactory in chrome_browser_main_extra_parts_profiles.cc? I tried to delay them with if (full_init) but then I ran into another problem where extensions::ExtensionsBrowserClient::Get() is null in chrome/browser/profiles/profile_impl.cc and I cannot find a way to initialize it. Do you have any idea about this?

EDIT: I solved it by delaying the initiliazation depends on extensions.

eklavya002 commented 4 years ago

saw latest build with extension support ,but can't find in release.where is it?

wchen342 commented 4 years ago

@eklavya002 I am still fixing some minor bugs. Will release in 1-2 days.

wchen342 commented 4 years ago

The experimental extension support version is now released. Please see Release.

rodrigoswz commented 4 years ago

I just can't seem to select the unzipped folder completely using the "Android file picker", just a single file.

dvalter commented 4 years ago

@dvalter Did you run into a SEGV_MAPERR caused by HostContentSettingsMapFactory/CookieSettingsFactory in chrome_browser_main_extra_parts_profiles.cc? I tried to delay them with if (full_init) but then I ran into another problem where extensions::ExtensionsBrowserClient::Get() is null in chrome/browser/profiles/profile_impl.cc and I cannot find a way to initialize it. Do you have any idea about this?

EDIT: I solved it by delaying the initiliazation depends on extensions.

I had NULL dereferences in extensions API init, somewhere in socket lazy inits. Sadly I could not figure out that caused that.

Looks like you've achieved some really interesting results, I'm curious to look at it closer ASAP.

wchen342 commented 4 years ago

@rodrigoswz The file picker has different implementations across phone brands/Android versions. I think it will be better if you open an issue with information about your phone/Android version/etc.

wchen342 commented 4 years ago

@dvalter I basically just wrapped things in an if statement, and it works so I kept it that way.

kiwibrowser commented 4 years ago

@dvalter Did you run into a SEGV_MAPERR caused by HostContentSettingsMapFactory/CookieSettingsFactory in chrome_browser_main_extra_parts_profiles.cc? I tried to delay them with if (full_init) but then I ran into another problem where extensions::ExtensionsBrowserClient::Get() is null in chrome/browser/profiles/profile_impl.cc and I cannot find a way to initialize it. Do you have any idea about this? EDIT: I solved it by delaying the initiliazation depends on extensions.

I had NULL dereferences in extensions API init, somewhere in socket lazy inits. Sadly I could not figure out that caused that.

Looks like you've achieved some really interesting results, I'm curious to look at it closer ASAP.

The null references is because there are inter-dependencies that cannot be met at the moment the session profile is loaded the first time. On Android, the session profile is loaded early (earlier than the desktop version), and to load the session profile, it depends on other components (it's actually called "DependsOn(XXXX)" in the code) which are supposed to be lazy-initialised.

However, when you have extensions enabled, there is a circular dependency, so you always end-up with non-met dependencies (hence the undefined pointers)

To work-around that, one solution is to init without extensions the first time ("if (early_initialization)"), vs "if (init_with_extensions)" the second time

wchen342 commented 4 years ago

@kiwibrowser Thanks for the explanation. I figured that the denpends will interlock with each other so I excluded the extension initialization at the first time. It seems to be working fine so far.

philS1 commented 4 years ago

@wchen342: in kiwi browser I can visit the chrome webstore. In the extension enabled version of ungoogled chromium it's not possible. Seems something is missing.

wchen342 commented 4 years ago

@philS1 This is by design. See https://ungoogled-software.github.io/ungoogled-chromium-wiki/faq#can-i-install-extensions-or-themes-from-the-chrome-webstore.

androidacy-user commented 4 years ago

@wchen342 i assume at some point that won't be the case? Not really reasonable to expect non-power users, eg standard end user to know a single thing about unpacking extensions

Sent from my VIEW 1 using FastHub

wchen342 commented 4 years ago

@linuxandria Ok so I did a quick test with vanilla chromium on windows and the web store can recognize it as chrome, so there is some patch in ungoogled-chromium that removed the ability of webstore to detect the browser.

Now about whether this should be reverted needs a whole discussion. I searched the issues in main repo but cannot find an explanation on this so I am going to open an issue there. However personally I feel againt adding back the ability because this will give Google an ability to detect your browser type and potentially tracking your usage of extensions, which is against the goal of this project.

dvalter commented 4 years ago

@linuxandria Ok so I did a quick test with vanilla chromium on windows and the web store can recognize it as chrome, so there is some patch in ungoogled-chromium that removed the ability of webstore to detect the browser.

Now about whether this should be reverted needs a whole discussion. I searched the issues in main repo but cannot find an explanation on this so I am going to open an issue there. However personally I feel againt adding back the ability because this will give Google an ability to detect your browser type and potentially tracking your usage of extensions, which is against the goal of this project.

I've never examined that part of Chromium, but that's my understanding of it based on Firefox, so it may be partially or fully wrong. For me it looks like Chromium provides some sort of private JS API to Chrome Webstore to allow installing extensions. FF/AMO do that for sure [link]. I'm sure it should be tied to domain/URL, so Ungoogled patches should replace chrome.google.com to some unreadable nonsense thus making it impossible for Webstore to actually install anything.

As you've mentioned.this API leaks at minimum a list of installed extensions to Google, so enabling it unpatched isn't privacy-friendly.

In addition if you're going to do it on a mobile device, you have to use a desktop User Agent for Webstore since it's not intended to work on a mobile Chrome. Required code could be found either in Kiwi repo or in my patches.

wchen342 commented 4 years ago

For me it looks like Chromium provides some sort of private JS API to Chrome Webstore to allow installing extensions. FF/AMO do that for sure

I haven't examined the code in details but yes this is my suspection since I tried spoofing user agent before but it doesn't work. That means enabling that will leak a whole bunch of information to Google which is not desired here, but I need confirmation from UC main repo to be sure.

androidacy-user commented 4 years ago

I'm not in favor of destroying any convenience in favor of "privacy" Honestly at least make a toggle in chrome://flags or similar to edgium prompting the user to allow chrome store installation

wchen342 commented 4 years ago

@linuxandria That I agree, but it depends on how this function is implemented and how viable it is to control it from a single switch.

Another solution is probably to bundle this addon but it will depend on the author updating his code.

wchen342 commented 4 years ago

I'm going to close this issue since the extension version has been released and there are too many topics mixed in this issue. Any new problem regarding extensions should open a separate issue.