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 finish startup task before allowing the platform to submit the first request #224

Closed
englehardt opened this issue Oct 23, 2018 · 16 comments
Assignees
Labels
extension Relates to the WebExtension written in TS and JS

Comments

@englehardt
Copy link
Collaborator

englehardt commented Oct 23, 2018

In #221 we started the move over to WebExtensions. Unfortunately, WebExtensions do not block the first page load on extensions startup, meaning that page content may load before the extension injects instrumentation. See Bug 1378459. We've thus had to account for this in our tests, see: 643e457.

This is a problem for us since most of our crawls are stateless and will experience some delay on every load, meaning we'll miss a small amount of data at the start of each page.

Let's see if we can block on startup within the browser manager until the extension has fully loaded.

@englehardt englehardt added the extension Relates to the WebExtension written in TS and JS label Nov 15, 2018
@englehardt
Copy link
Collaborator Author

Possibly related: gbaptista/luminous#55 and EFForg/privacybadger#1865.

@ghostwords
Copy link
Contributor

There may be two separate kinds of issues here:

@englehardt englehardt changed the title Block on WebExtensions startup before submitting first request Ensure WebExtensions finish startup task before allowing the platform to submit the first request Nov 19, 2018
@englehardt
Copy link
Collaborator Author

@ghostwords thanks for pointing that out. I updated the title and filed #242 to make it clear there are two potential issues.

@englehardt
Copy link
Collaborator Author

englehardt commented Mar 27, 2019

It's now possible to block requests on startup until webextensions are ready. See mozilla/contain-facebook#206 and mdn/sprints#1015 for a description.

@Grossdm

This comment has been minimized.

@englehardt

This comment has been minimized.

@Grossdm

This comment has been minimized.

@nhnt11
Copy link
Contributor

nhnt11 commented Aug 22, 2019

It's now possible to block requests on startup until webextensions are ready. See mozilla/contain-facebook#206 and mdn/sprints#1015 for a description.

Alright, I looked at the code landed in bug 1447551 and it's promising. It indeed allows us to write a blocking WebRequest listener that stalls page loads till the instrumentation is ready. I'm going to try and write a patch for this today.

@jallmann
Copy link

jallmann commented Sep 4, 2019

@nhnt11 Have you made any progress with this patch? Or otherwise, would you mind if I pick it up?

@nhnt11
Copy link
Contributor

nhnt11 commented Sep 20, 2019

@jallmann ugh, I didn't see this. Please feel free to pick it up and let me know if I can help.

@nhnt11
Copy link
Contributor

nhnt11 commented Dec 9, 2019

@englehardt Can we close this?

@englehardt
Copy link
Collaborator Author

@nhnt11 If I remember correctly from a few weeks back you mentioned that you had tested this manually by adding in a big delay into startup and didn't see any missed requests? Is that right?

@nhnt11
Copy link
Contributor

nhnt11 commented Dec 10, 2019

Yes. Specifically, we verified that the extension content script is guaranteed to run before page scripts. I'm not sure if this issue includes the idea of making the extension send an ack to the platform before the platform starts issuing requests.

Edit: Ah, that was #242! My bad, sorry.

@nhnt11
Copy link
Contributor

nhnt11 commented Dec 10, 2019

I think what we verified was that we can indeed use the blocking WebRequest listener to achieve this but I can't remember if we landed it. It doesn't seem like it.

@vringar
Copy link
Contributor

vringar commented May 7, 2020

The second part of the issue (the ack to the plattform) is being tracked in #135 and the first part was successfully implemented so I'm closing this

@vringar vringar closed this as completed May 7, 2020
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

6 participants