Repository navigation
Setting proxy for safe browsing requests - #108
Conversation
bbondy
left a comment
There was a problem hiding this comment.
Instead let's avoid the patch file by modifying the network traffic like we do for geolocations and updates here:
browser/net/brave_static_redirect_network_delegate_helper.cc
Thanks!
fc34341 to
1dc00a9
Compare
bbondy
left a comment
There was a problem hiding this comment.
Pls make the pattern not include v4 because when v5 happens it wouldn't match and we might miss it. Then add test for v5 too please if you don't mind. Thank you!
f5be645 to
c4dffff
Compare
| source_set("net") { | ||
| configs += [ "//brave/build/geolocation" ] | ||
| configs += [ | ||
| "//brave/build/geolocation", |
There was a problem hiding this comment.
I'm not sure that using configs like this is the best way to do this. I think it would be better to create a normal dependency //brave/browser/safe_browsing and give it a public config. That would be a more typical pattern.
There was a problem hiding this comment.
btw - my reasoning for this is that a dep provides more flexibility to manage configs, sources and add any additional deps that might be required in the future. In most cases (there are some exceptions), configs are treated more like internal implementation details for deps and those details are passed along using public_configs
Fixes: brave/brave-browser#204