userscripts icon indicating copy to clipboard operation
userscripts copied to clipboard

Improve scripts injection flow and logic

Open ACTCD opened this issue 3 years ago • 6 comments

When we complete the refactoring of #331, we could try to further optimize the script injection flow and speed up the scripts injection.

In content.js, we will accomplish scripts injection with just two browser.storage.local.get asynchronous operations:

  • Get extension settings information
    • Extension active switch
    • Scripts active switch
    • Global exclude-match list
    • match/include/exclude...
  • Get the matched scripts code
    • Prepare wrappers for each script
    • Add the corresponding GM APIs in the wrapper
    • Add script code in wrapper
    • Immediately inject scripts

This will do the fastest document-start scripts injection in content scripts, but cannot guarantee full priority over page scripts as there are still asynchronous operations.

In background.js, which is responsible for updating the relevant information in browser.storage.local, we can check and update the scripts corresponding to the url request as early as possible, we could use the following process:

  • Use webNavigation.onBeforeNavigate to get the request url
  • Get cached scripts list and file modification date via browser.storage.local.get
  • Get the current files list and file modification date through the native layer, and get new or modified scripts code
  • Prioritize updating scripts code matching url to browser.storage.local and related indexes, and then update other scripts.

Since the above process is optimized for injection speed, the background process may be completed later than the content scripts injection, which may cause the latest scripts code to not be applied when the page refresh/load of first time after the scripts is updated, but this should be acceptable, we can also provide an option switch to let the user decide whether to inject first or version first(For example, it is convenient for user script developers to debug).

The above is just an idea, not tested and finalized, it may be rejected or not implemented for practical reasons.

ACTCD avatar Sep 28 '22 13:09 ACTCD

@ACTCD These are good idea, but I should note that webNavigation.onBeforeNavigate is not reliable. Sometimes it does not return a url in Safari (I reported this bug to Apple a while ago). I believe with this method this could lead to a scenario where we are often attempting to deliver defunct userscripts and/or not getting the newest userscripts as you mentioned in your comment.

quoid avatar Sep 28 '22 16:09 quoid

@quoid We can choose any trigger early enough, I haven't evaluated them, but I believe we can surely find one or more available from below:

webNavigation.onBeforeNavigate
webRequest.onBeforeRequest
webRequest.onBeforeSendHeaders
webRequest.onSendHeaders
webRequest.onHeadersReceived
webRequest.onResponseStarted
webRequest.onCompleted
tabs.onUpdated
tabs.onZoomChange
webNavigation.onCommitted
...

Worst case is we don't use url and just update all scripts info, it just lacks priority and won't have much impact.

ACTCD avatar Sep 28 '22 16:09 ACTCD

@ACTCD All of the webNavigation have the same issue I described.

tabs is not very viable from my testing, see here

We can not use webRequest because we have a non-persistent background page, which is required for iOS and webRequest required a persistent background page.

quoid avatar Sep 28 '22 16:09 quoid

@quoid Thank you so much for your investigation and clarification, it looks like we have the worst possible situation.

For the above idea, we mainly need a trigger as early as possible to prepare and check for scripts updates in advance, as I said above, we can also just forgo the use of url and update the scripts cache sequentially, it should not have too much diffrence.

Another solution, maybe we could find an api in the native layer (swift) to monitor file changes in the scripts folder, actively push the updated files to the background and update the cache information in the storage.

ACTCD avatar Sep 28 '22 17:09 ACTCD

Could you elaborate more on how this will work?

  • From content.js we still call REQ_USERSCRIPTS by page sender.url?
    • this is now sent from content.js or is still sent to background.js which makes sendNativeMessage call and returns matched scripts?
  • once we get matched scripts from content.js, where and how do we store those scripts? What does that object look like?
  • is the purpose of webNavigation.onBeforeNavigate to update all script data in browser.storage?
    • does that mean every single navigation event we are asking for all scripts?

quoid avatar Sep 28 '22 17:09 quoid

@quoid This is still a very early idea and a lot of work needs to be done before it, I will update with more details in due course.

ACTCD avatar Sep 28 '22 18:09 ACTCD