Bromite: User scripts component

[uazo]

Is your feature request related to privacy?

No, but it could become

Is there a patch available for this feature somewhere?

No.

Describe the solution you would like

as some users have said, I also find privacy pop-ups boring, especially google ones, since in my phone they are quite lagged

Describe alternatives you have considered

initially I thought about reusing the Inject-scripts-for-AMP-tracking-ads-and-video.patch but then I saw that there is the possibility to activate the support for userscripts, which is somehow better because it leaves no trace in the dom, as opposed to that patch.

as a personal test (and also a bit of a challenge) I tried to reactivate user scripts in bromite (https://www.chromium.org/developers/design-documents/user-scripts) taking them from the extensions and moving them to a new one component, managing to make them work (https://github.com/uazo/bromite/blob/gitpages/userscripts_v2.7z my work)
Obviously, as my first test, there is no ui (and by the way it could be quite long to implement it), we could think to proceed in steps.

I wanted to understand if there was interest and, if so, if we wanted to discuss how to implement it, also considering the security aspects, updates and possible integration with chromium developer tools.

current state:

• [x] base code + reading from folder
• [x] preferences (enable/disable)
  
  
  
  • [x] add link to help page (actually https://github.com/bromite/bromite/wiki/UserScripts)
  
  
• [x] ui management (enable/disable/remove script)
  
  
  
  • [x] consider changing proguard settings for R (see https://stackoverflow.com/a/2889844)
  
  
  • [x] add icons (enable/disable)
  
  
• [x] add script from file dialog
  
  
  
  • [ ] remove excessive permissions prompt from filedialog (camera + audio)
  
  
  • [x] check how to transform a content:// (URI) to file:// (PATH)
  
  
  • [x] get real script filename from a content://
  
  
  • [x] when importing from file, skip non ascii chars used in description to prevent [FATAL:web_string.cc(112)] Check failed: ContainsOnlyASCII()
  
  
  • [x] reload list after add and show toast with error (check how works CallByNative)
  
  
  • [x] check if a file is real user script (es refuse txt without javascript)
  
  
• [x] understand if installation from download makes sense (entry point https://source.chromium.org/chromium/chromium/src/+/master:chrome/android/java/src/org/chromium/chrome/browser/download/DownloadUtils.java;l=360), actually we have to download the file
  
  
  
  • [x] check how to request permission from the user
  
  
• [x] add feature flag
• [ ] consider if we want to add a way to prevent crash in the startup
• [ ] code cleaning
  
  
  
  • [ ] remove unused code
  
  
  • [x] remove unused BrowserContextHandle from bridge
  
  
  • [ ] try to simply rendering code
  
  
  • [ ] change all ifdef removing extension word
  
  
  • [ ] move all logging into specifics functions (or check how log settings works)
  
  
• [ ] ui management (2) (script details + flag enabled in incognito + view source code)
  
  
  
  • [ ] check if we can use an "interstitial" page to install a userscript
  
  
  • [ ] localize error strings
  
  
• [ ] security verification
  
  
  
  • [ ] consider whether to use a sha256 to verify a possible modification of the file without the user's knowledge (if we give the possibility to choose the scripts folder)
  
  
  • [ ] check if we can deny network access to injected scripts (check ScriptContext)
  
  
• [ ] understand if it makes sense to use zip files (for adding css directly, userscripts.js doesn't support them directly) code https://source.chromium.org/chromium/chromium/src/+/master:components/services/unzip/public/cpp/unzip.cc
• [ ] understand if self-updating makes sense
  
  
  
  • [ ] check how the source can be verified (script is unsigned by default)
  
  
• [ ] add management in devtools (like snippets, code https://source.chromium.org/chromium/chromium/src/+/master:third_party/devtools-frontend/src/front_end/snippets/ScriptSnippetFileSystem.js)
  
  
  
  • [ ] add flag for "fall-back" in devtools (code https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/android/devtools_server.cc)
  
  

[csagan5]

I tried to reactivate user scripts
taking them from the extensions and moving them to a new component

What is the status of this feature upstream? is --enable-user-scripts still working?

[uazo]

That switch was removed on 2009-11-24
https://source.chromium.org/chromium/chromium/src/+/faed6e1304f7ba31ed12d42ea3dc694021690afd

but that code is alive (see for example https://source.chromium.org/chromium/chromium/src/+/6d96d3d1cb83d433c2b6a7c2f29c15900296f3c8)
since it is used for declarative scripts used by extensions

[csagan5]

How would this work on Android? I have checked your 7z file but since it is not a patch I cannot see exactly what you changed without extracting it on a branch.

possible integration with chromium developer tools.

What integration do you mean here?

[uazo]

I cannot see exactly what you changed without extracting it on a branch.

it's because I haven't changed anything, it's all new, taking code from extensions and removing the superfluous, they are not active in android.
it is a new component, like those already found in the base code. I preferred this way, given the need to keep maintenance easy, it is something we can discuss.

How would this work on Android?

technically the component attaches itself to the rendering process (it is the default behavior of any existing component) and, responding to events in chromium, injects javascript code declaring when and what.
the browser process, via mojo, sends the scripts to the rendering engine, since it cannot access the disk to retrieve any scripts.
all things I've already found, of course, credit to google teams.

Of all this, I haven't done this last thing yet, but the code already provides for it. for now i just statically declared the scripts, you can see it in components\user_scripts\content\renderer\user_script_set_manager.cc

What can be injected is visible here https://source.chromium.org/chromium/chromium/src/+/master:extensions/common/user_script.h while here https://source.chromium.org/chromium/chromium/src/+/master:extensions/browser/extension_user_script_loader.cc and here https://source.chromium.org/chromium/chromium/src/+/master:extensions/common/manifest_handlers/content_scripts_handler.cc the classes that loads from file.

What integration do you mean here?

I mean that it would also be possible to think of adding the control of user scripts through the chromium developer tools, to make their development easier. this is just an idea, I don't know how to do it yet.

[csagan5]

It is a quite big change, almost comparable to extensions support. I would have to rely on your to maintain it in future versions if it were to be merged.

I understand the integration with developer tools (I guess you mean via remote debugging), but what about Android regular users? It would need some UI to load/activate/disable such user scripts?

[uazo]

I guess you mean via remote debugging

yes, I mean that.

but what about Android regular users? It would need some UI to load/activate/disable such user scripts?

yes, they would have to be developed.
in the next few days I will prepare a proposal to discuss.

I would have to rely on your to maintain it in future versions if it were to be merged.

let's talk about. my idea is to minimize the interventions: developing a new component, the maintenance is the opposite of what you are used to, so it is not a patch from the chromium code, but it is a patch to the chromium code.
I hope I have explained myself, the language does not help me.

[csagan5]

I understand the idea of a component; still, you will see that even with components maintenance is a burden.

@uazo you can already create a PR rather than a 7z archive?

[uazo]

you can already create a PR rather than a 7z archive?

you can see it here.

https://github.com/uazo/bromite/blob/enable-user-scripts/build/patches/add-user-scripts.patch

sorry for the delay, but I had the branch blocked by the other patches.

[csagan5]

you can see it here.

https://github.com/uazo/bromite/blob/enable-user-scripts/build/patches/add-user-scripts.patch

I have not had time to test this yet, but what are the incomplete parts you can individuate?

[DI555]

@uazo , could you put there support and for UserStyles (User CSS) also, please ?

[uazo]

version 3 [wip]:
https://github.com/uazo/bromite/blob/112c6d8aa511f2d6b50245374fc127ea7c777d1b/build/patches/wip-user-scripts-v3.patch

Added:

• use of folder snippets for userscripts under app folder

currently for all files in that folder parse and use them if they are correct. any error messages are in the log.

what is missing:

• ui
• security verification: consider whether to use a sha256 to verify a possible modification of the file without the user's knowledge
• understand if it makes sense to use zip files (for adding css directly, .userscripts.js doesn't support them)
• understand if installation from download is possible
• understand if self-updating makes sense
• add management in devtools
• code cleaning

[csagan5]

• security verification: consider whether to use a sha256 to verify a possible modification of the file without the user's knowledge

This is not necessary: Android has sandboxing, no other app can modify another app's files.

• understand if installation from download is possible

The AdBlock updater in Bromite already stores files locally, it could be used as a reference.

• understand if self-updating makes sense

This should be considered only later on, when there is an UI.

• ui

This might take a lot of time, I would look at it last. We still do not have #811

[uazo]

This is not necessary: Android has sandboxing, no other app can modify another app's files.

yes, but I thought so in case we wanted to leave the user the possibility to choose the scripts folder, so that eventually he could choose to do a sync

The AdBlock updater in Bromite already stores files locally, it could be used as a reference.

not quite like that, I wanted to use this
https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/download/chrome_download_manager_delegate.cc;l=659

This should be considered only later on, when there is an UI.

Sure

This might take a lot of time, I would look at it last. We still do not have #811

uhm, I'm working on it now because it's a key part. it is not very long (with cut&paste cr code), of course if each test did not take 10 minutes between compiling and installing it would be better! let's see, a new pc is coming.

it's the tests that worry me more and memory management for any leaks

[csagan5]

so that eventually he could choose to do a sync

I do not think that it is possible. Android 11 implements even stronger sandboxing between apps/OS.

it's the tests that worry me more and memory management for any leaks

There is surely ways to detect leaks in Chromium; see https://www.chromium.org/developers/testing/leaksanitizer and https://www.chromium.org/developers/leak-detection

If you componentize the code properly you can even test it stand-alone, outside of Chromium (and then use anything you want, valgrind etc).

[csagan5]

@uazo is this related/useful? https://bugs.chromium.org/p/chromium/issues/detail?id=1054624

[uazo]

is this related/useful? https://bugs.chromium.org/p/chromium/issues/detail?id=1054624

not currently, for what I'm doing. I don't directly use the api of the extensions but only their logic.
But it's interesting how you got there: is there any way to register for notifications to a specific bug area? it would also be useful if it were by file or by directory.

in the meantime I ask you: I just can't fully understand the logic behind of base::BindOnce() that is driving me crazy, any suggestions of where to look for good documentation? I didn't find much in the code.

Anoher thing, would you be inclined to modify the proguard settings excluding R?
if, instead of changing the preferences every time directly in the xml, we change SettingsUtils.addPreferencesFromResource() and through the reflections we search for all the xml in R that ended in ".addon.xml", we would not have overlaps between different patches.
it's just an idea but in debug (without proguard) i tried and it works: please wait for me to put up the updated patch so I can show you the code (as soon as I understand how the hell it works BindOnce!)

[csagan5]

But it's interesting how you got there: is there any way to register for notifications to a specific bug area? it would also be useful if it were by file or by directory.

I found this one while looking for something else; you can create saved queries in the Chromium issue tracker and get email notifications for them.

in the meantime I ask you: I just can't fully understand the logic behind of base::BindOnce() that is driving me crazy, any suggestions of where to look for good documentation? I didn't find much in the code.

I am not an expert but it seems to be a way to invoke a function only once, based on its pointer. See
https://github.com/chromium/chromium/blob/87.0.4280.106/base/callback.h#L30 and then below there is the documentation for Then:

  // Then() returns a new OnceCallback that receives the same arguments as
  // |this|, and with the return type of |then|. The returned callback will:
  // 1) Run the functor currently bound to |this| callback.
  // 2) Run the |then| callback with the result from step 1 as its single
  //    argument.
  // 3) Return the value from running the |then| callback.
  //
  // Since this method generates a callback that is a replacement for `this`,
  // `this` will be consumed and reset to a null callback to ensure the
  // originally-bound functor can be run at most once.

Anoher thing, would you be inclined to modify the proguard settings excluding R?

You mean something like this? https://stackoverflow.com/a/2889844

instead of changing the preferences every time directly in the xml, we change SettingsUtils.addPreferencesFromResource() and through the reflections we search for all the xml in R that ended in ".addon.xml", we would not have overlaps between different patches.

It will bloat a bit the APK, but that is not a big concern. Do we have many XML issues with patches?

it's just an idea but in debug (without proguard) i tried and it works: please wait for me to put up the updated patch so I can show you the code (as soon as I understand how the hell it works BindOnce!)

Ok.

[uazo]

some screenshot:

| Settings: | Enable/Disable script: |
| ------------- | ------------- |
| | |

File Picker:

Sample for https://greasyfork.org/it/scripts/34613-youtube-multi-downloader-v6-0-youtubeconverter-io-mp3-fhd-mp4-hd-sd-m4a-3gp:

[uazo]

I found this one while looking for something else; you can create saved queries in the Chromium issue tracker and get email notifications for them.

thanks, I'll try.

be a way to invoke a function only once, based on its pointer

yes, what I didn't understand is how. for now I found that it actually creates a function that allows the regeneration of parameters within the other thread: what I did not understand in fact is why some types of parameters accepted them and others did not, and I solved it by understanding that certain parameters can be recreated only if defined as constants. here i miss the basics of c ++, it has changed a lot since my university days.

You mean something like this? https://stackoverflow.com/a/2889844

Yes perfect! I just have to figure out where it is in the source ...

Do we have many XML issues with patches?

actually and just to simplify switching between different releases. the fewer interventions I can do in the source code base, the better, right?

instead of changing the preferences every time directly in the xml, we change SettingsUtils.addPreferencesFromResource()

See here:
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L230
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L435
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L495

[csagan5]

what I did not understand in fact is why some types of parameters accepted them and others did not

They are constraints created by the template. Since the function is run by another thread only some operations are allowed, to not have memory leaks or invalid access between threads.

actually and just to simplify switching between different releases. the fewer interventions I can do in the source code base, the better, right?
See here:
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L230
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L435
https://github.com/uazo/bromite/blob/f6d9f2ebcc384c3e75d921fbb3f3bb309122096e/build/patches/add-user-scripts.v4.patch#L495

There are pros/cons; the addon XML would also have linting checks applied? About missing resources etc

[uazo]

the addon XML would also have linting checks applied?

Yes