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.

Sign up for GitHub

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

Jump to bottom

Refactor: Simplify network delegate helpers by having a single request_url #1576

Closed
bbondy opened this issue Oct 15, 2018 · 1 comment · Fixed by brave/brave-core#633
Closed

Refactor: Simplify network delegate helpers by having a single request_url #1576

bbondy opened this issue Oct 15, 2018 · 1 comment · Fixed by brave/brave-core#633
Assignees
@bbondy
Labels
dev-concern QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Milestone
0.55.x - Release ...

Comments

@bbondy
Copy link
Member

bbondy commented Oct 15, 2018

Instead of having a new_url and a request_url, some of the logic would be more simple and less error prone if we had only one.

We could also store the original_request_url so we know if it was changed or not in each network delegate helper.
@bbondy bbondy added this to the 0.55.x milestone Oct 15, 2018
@bbondy bbondy mentioned this issue Oct 15, 2018
Merged
18 tasks
@bbondy bbondy self-assigned this Oct 15, 2018
@bbondy bbondy added the QA/Yes label Oct 15, 2018
@srirambv
Copy link
Contributor

srirambv commented Oct 18, 2018 •
edited by LaurenWags
Loading

Verification Passed on

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Linux
  • Verified general use of browser doesn't cause any crash

Verification Passed on

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (32-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Windows
  • Verified general use of browser doesn't cause any crash

Verified passed with

Brave 0.55.17 Chromium: 70.0.3538.67 (Official Build) (64-bit)
Revision 9ab0cfab84ded083718d3a4ff830726efd38869f-refs/branch-heads/3538@{#1002}
OS Mac OS X
  • Verified general use of browser doesn't cause any crash

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bbondy bbondy

Labels
dev-concern QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes
Projects
None yet
Milestone
0.55.x - Release (Release Channel)
Development

Successfully merging a pull request may close this issue.

Refactor network delegate helpers to not have access to the request brave/brave-core
4 participants
@bbondy @srirambv @LaurenWags @GeetaSarvadnya