Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure WebExtensions inject content script instrumentation before any page scripts run #242

Closed
englehardt opened this issue Nov 19, 2018 · 7 comments
Assignees
Labels
extension Relates to the WebExtension written in TS and JS

Comments

@englehardt
Copy link
Collaborator

The instrumentation should block page script execution until all of the instrumentation injected by the js_instrument is set up. We should determine whether this is actually an issue or if we are only experience this during the initial page load as described in #224. See also: gbaptista/luminous#55 and EFForg/privacybadger#1865.

Note that even if this isn't an issue right now, we may experience it once we fix #68.

@englehardt
Copy link
Collaborator Author

In #291 we enabled "testing mode" for all content script injections. When we fix this, we should reverse that and rely on the testing parameter once again.

@jallmann
Copy link

As far as I (and @nhnt11) could determine, this should not be an issue with the current implementation, as soon as the extension is fully loaded.
Content scripts are registered with runAt = "document_start", and as stated here Searchfox, this should guarantee content-script injection before any page scripts run.
I tried to verify with a small test extension and test page, and I can at least state the absence of counterexamples. It's easy, though, to delay the registering of the content script upon extension startup, so that pages load and page scripts run without the content script being injected on time (or at all). So I guess this falls back to #224, ensuring extension startup to finish before submitting any requests.

@nhnt11
Copy link
Contributor

nhnt11 commented Aug 21, 2019

@jallmann Thanks for that investigation! I'm curious - could you share your test extension somewhere? Did you try putting a small delay in the content script and then checking if page scripts are affected by that delay?

@jallmann
Copy link

jallmann commented Sep 4, 2019

This is the extension I built for testing:
https://github.com/jallmann/webExtTest

The repo includes a test page and script with some console prints to illustrate the order of events.
The extension is currently configured to register content scripts 5 seconds after it loads. If the test page is loaded wihtin that interval, the page script runs without the content script being injected at all.

Did you try putting a small delay in the content script and then checking if page scripts are affected by that delay?

I not sure what you mean. Some artificial, blocking delay in the content script? I haven't tried that, but I would expect it to delay page script execution as well.

@nhnt11
Copy link
Contributor

nhnt11 commented Sep 20, 2019

This is the extension I built for testing:
https://github.com/jallmann/webExtTest

Thanks!

The repo includes a test page and script with some console prints to illustrate the order of events.
The extension is currently configured to register content scripts 5 seconds after it loads. If the test page is loaded wihtin that interval, the page script runs without the content script being injected at all.

That sounds right. 👍

Did you try putting a small delay in the content script and then checking if page scripts are affected by that delay?

I not sure what you mean. Some artificial, blocking delay in the content script? I haven't tried that, but I would expect it to delay page script execution as well.

Yup, that's what I meant.

@jallmann
Copy link

Yes, blocking the content script for some time delays execution of the page script as well, just as expected.

@nhnt11
Copy link
Contributor

nhnt11 commented Sep 25, 2019

I think we can close this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Relates to the WebExtension written in TS and JS
Projects
None yet
Development

No branches or pull requests

3 participants