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

[WIP] refactor: (Part I) make the ownership of URLRequestContextGetter more clear #13956

Merged
merged 5 commits into from
Aug 13, 2018

Conversation

deepak1556
Copy link
Member

Previously we were relying on reference counting and cleaning up network code on the IO thread, this doesn't really work well and caused use-after-free. In this new mode, the class that owns URLRequestContextGetter finishes network cleanup and destroys itself on the IO thread. This also allowed to create multiple request context getters with custom URLRequestContext, the plan is to separate the request context for media and also add a switch to create URLRequestContext based on the network service from chromium in follow up PR.

Fixes #13686

Checklist
  • PR description included and stakeholders cc'd
  • npm test passes
  • tests are changed or added
  • relevant documentation is changed or added
  • PR title follows semantic commit guidelines

* Accepts a factory class that can customize the creation of URLRequestContext
* Use a separate request context for media which is derived from the default
* Notify URLRequestContextGetter observers and cleanup on IO thread
* Move most of brightray net/ classes into atom net/
@deepak1556 deepak1556 requested review from a team and zcbenz August 6, 2018 15:48
@deepak1556 deepak1556 force-pushed the request_context_refactor branch 2 times, most recently from 4db1bbf to 8b04587 Compare August 7, 2018 15:07
* Allows to use the default handler from content/ for http{s}, ws{s} schemes.
* Removes the storage of job factory in URLRequestContextGetter.
@codebytere codebytere added this to Active: PR in Flight in 3.0.x / 3.1.x Aug 10, 2018
@codebytere codebytere removed this from Active: PR in Flight in 3.0.x / 3.1.x Aug 10, 2018
@MarshallOfSound
Copy link
Member

CI failures are known flakes on master 👍 Merging in as per discussion with @ckerr and @codebytere

@MarshallOfSound MarshallOfSound merged commit 1c0bb06 into master Aug 13, 2018
@MarshallOfSound MarshallOfSound deleted the request_context_refactor branch August 13, 2018 22:22
ckerr added a commit that referenced this pull request Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants