Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(705)

Issue 4705284891082752: Proxy configurators (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 10 months ago by René Jeschke
Modified:
5 years, 9 months ago
Reviewers:
Felix Dahlke
Visibility:
Public.

Description

Proxy configurators

Patch Set 1 #

Total comments: 55

Patch Set 2 : Update 1 #

Patch Set 3 : FIXME removed #

Patch Set 4 : valueOf #

Total comments: 13

Patch Set 5 : Another round #

Patch Set 6 : Removed command bridge #

Total comments: 8

Patch Set 7 : Last batch of review issues #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1970 lines, -897 lines) Patch
M src/org/adblockplus/android/AdblockPlus.java View 1 5 chunks +1 line, -10 lines 0 comments Download
M src/org/adblockplus/android/AdvancedPreferences.java View 1 2 3 4 9 chunks +24 lines, -45 lines 0 comments Download
M src/org/adblockplus/android/ConfigurationActivity.java View 1 chunk +1 line, -1 line 0 comments Download
M src/org/adblockplus/android/CrashHandler.java View 1 2 3 4 5 6 2 chunks +18 lines, -1 line 0 comments Download
M src/org/adblockplus/android/Preferences.java View 7 chunks +22 lines, -28 lines 0 comments Download
M src/org/adblockplus/android/ProxyConfigurationActivity.java View 2 chunks +2 lines, -2 lines 0 comments Download
A + src/org/adblockplus/android/ProxyServerType.java View 1 2 3 4 1 chunk +7 lines, -3 lines 0 comments Download
M src/org/adblockplus/android/ProxyService.java View 1 2 3 4 5 6 15 chunks +203 lines, -543 lines 0 comments Download
D src/org/adblockplus/android/ProxySettings.java View 1 chunk +0 lines, -248 lines 0 comments Download
A src/org/adblockplus/android/ServiceBinder.java View 1 chunk +98 lines, -0 lines 0 comments Download
M src/org/adblockplus/android/Utils.java View 1 2 3 4 2 chunks +15 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/compat/CmGlobalProxyManager.java View 1 1 chunk +129 lines, -0 lines 0 comments Download
A + src/org/adblockplus/android/compat/CompatibilityException.java View 1 1 chunk +9 lines, -6 lines 0 comments Download
A src/org/adblockplus/android/compat/LinkProperties.java View 1 chunk +140 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/compat/ProxyProperties.java View 1 1 chunk +162 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/CyanogenProxyConfigurator.java View 1 chunk +145 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/IptablesProxyConfigurator.java View 1 1 chunk +234 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/ManualProxyConfigurator.java View 1 2 3 4 5 1 chunk +268 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/NativeProxyConfigurator.java View 1 2 3 4 5 6 1 chunk +230 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/ProxyConfigurator.java View 1 1 chunk +76 lines, -0 lines 0 comments Download
A src/org/adblockplus/android/configurators/ProxyConfigurators.java View 1 1 chunk +148 lines, -0 lines 0 comments Download
A + src/org/adblockplus/android/configurators/ProxyRegistrationType.java View 1 1 chunk +19 lines, -9 lines 0 comments Download
M src/org/adblockplus/brazil/RequestHandler.java View 3 chunks +18 lines, -0 lines 0 comments Download
M src/org/adblockplus/brazil/SSLConnectionHandler.java View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 17
René Jeschke
Pretty huge review, sry. We need to take a generic look at the changes first ...
5 years, 10 months ago (2014-08-11 12:38:22 UTC) #1
Felix Dahlke
Did a first pass of reviewing this - but I'm pretty sure I'll need another ...
5 years, 9 months ago (2014-08-19 09:06:00 UTC) #2
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/AdblockPlus.java File src/org/adblockplus/android/AdblockPlus.java (left): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/AdblockPlus.java#oldcode231 src/org/adblockplus/android/AdblockPlus.java:231: // TODO: Why don't we re-check? On 2014/08/19 09:06:00, ...
5 years, 9 months ago (2014-08-19 10:41:33 UTC) #3
Felix Dahlke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java File src/org/adblockplus/android/BridgeCommand.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java#newcode48 src/org/adblockplus/android/BridgeCommand.java:48: final BridgeCommand e = MAP.get(str); On 2014/08/19 10:41:33, René ...
5 years, 9 months ago (2014-08-19 12:37:26 UTC) #4
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java File src/org/adblockplus/android/BridgeCommand.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java#newcode48 src/org/adblockplus/android/BridgeCommand.java:48: final BridgeCommand e = MAP.get(str); On 2014/08/19 12:37:26, Felix ...
5 years, 9 months ago (2014-08-19 12:57:36 UTC) #5
Felix Dahlke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java File src/org/adblockplus/android/BridgeCommand.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java#newcode48 src/org/adblockplus/android/BridgeCommand.java:48: final BridgeCommand e = MAP.get(str); On 2014/08/19 12:57:36, René ...
5 years, 9 months ago (2014-08-19 13:14:57 UTC) #6
Felix Dahlke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/ProxyService.java File src/org/adblockplus/android/ProxyService.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/ProxyService.java#newcode98 src/org/adblockplus/android/ProxyService.java:98: // FIXME 'StrictMode' is API9 On 2014/08/19 12:57:36, René ...
5 years, 9 months ago (2014-08-19 13:17:50 UTC) #7
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java File src/org/adblockplus/android/BridgeCommand.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/BridgeCommand.java#newcode48 src/org/adblockplus/android/BridgeCommand.java:48: final BridgeCommand e = MAP.get(str); On 2014/08/19 13:14:57, Felix ...
5 years, 9 months ago (2014-08-19 13:23:53 UTC) #8
Felix Dahlke
Alright, will start the second pass then :) http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/ProxyService.java File src/org/adblockplus/android/ProxyService.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5629499534213120/src/org/adblockplus/android/ProxyService.java#newcode98 src/org/adblockplus/android/ProxyService.java:98: // ...
5 years, 9 months ago (2014-08-19 13:28:51 UTC) #9
Felix Dahlke
Alright, second round done. Still looking good :) http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyServerType.java File src/org/adblockplus/android/ProxyServerType.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyServerType.java#newcode27 src/org/adblockplus/android/ProxyServerType.java:27: /** ...
5 years, 9 months ago (2014-08-19 16:14:46 UTC) #10
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyServerType.java File src/org/adblockplus/android/ProxyServerType.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyServerType.java#newcode27 src/org/adblockplus/android/ProxyServerType.java:27: /** SOCKS4 */ On 2014/08/19 16:14:46, Felix H. Dahlke ...
5 years, 9 months ago (2014-08-19 16:33:24 UTC) #11
Felix Dahlke
http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyService.java File src/org/adblockplus/android/ProxyService.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyService.java#newcode473 src/org/adblockplus/android/ProxyService.java:473: case CYANOGENMOD: On 2014/08/19 16:33:24, René Jeschke wrote: > ...
5 years, 9 months ago (2014-08-19 16:47:05 UTC) #12
Felix Dahlke
Looks good now, you didn't get back to me on this yet: http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyService.java#newcode473 I'll do ...
5 years, 9 months ago (2014-08-23 08:18:51 UTC) #13
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyService.java File src/org/adblockplus/android/ProxyService.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5088492568707072/src/org/adblockplus/android/ProxyService.java#newcode473 src/org/adblockplus/android/ProxyService.java:473: case CYANOGENMOD: On 2014/08/19 16:47:05, Felix H. Dahlke wrote: ...
5 years, 9 months ago (2014-08-23 11:33:31 UTC) #14
Felix Dahlke
http://codereview.adblockplus.org/4705284891082752/diff/5655608640405504/src/org/adblockplus/android/CrashHandler.java File src/org/adblockplus/android/CrashHandler.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5655608640405504/src/org/adblockplus/android/CrashHandler.java#newcode179 src/org/adblockplus/android/CrashHandler.java:179: final ProxyProperties proxyProperties = new ProxyProperties(this.host, p, this.excl); We ...
5 years, 9 months ago (2014-08-24 11:51:18 UTC) #15
René Jeschke
http://codereview.adblockplus.org/4705284891082752/diff/5655608640405504/src/org/adblockplus/android/CrashHandler.java File src/org/adblockplus/android/CrashHandler.java (right): http://codereview.adblockplus.org/4705284891082752/diff/5655608640405504/src/org/adblockplus/android/CrashHandler.java#newcode179 src/org/adblockplus/android/CrashHandler.java:179: final ProxyProperties proxyProperties = new ProxyProperties(this.host, p, this.excl); On ...
5 years, 9 months ago (2014-08-24 11:54:13 UTC) #16
Felix Dahlke
5 years, 9 months ago (2014-08-24 11:56:01 UTC) #17
LGTM!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5