ABP/Android Proxy switch

ABP/Android Proxy switch

[Felix Dahlke, 2013-03-11 06:57:38]

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/AdblockPlus.java (right):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/AdblockPlus.java:231: if
("org.adblockplus.android.ProxyService".equals(service.service.getClassName())
&& service.pid > 0)
How about ProxyService.getClass().getCanonicalName() (should work IIRC) instead
of hard coding this?

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/AdblockPlus.java:920: System.gc();
I think we shouldn't do this, won't help us work around the memory issue (will
explain that later).

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/Preferences.java (left):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Preferences.java:241: application.stopEngine(true);
Why do this in onStop() and not in onSharedPreferenceChanged()?

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/Starter.java (right):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Starter.java:38: boolean us =
"org.adblockplus.android".equals(intent.getData().getSchemeSpecificPart());
Any idea how we could not hard code the package here? I'd suggest
getClass().getPack().getName(), but it appears this has nothing to do with the
package of the class Starter here. Would
AdblockPlus.getClass().getPack().getName() make more sense?

[Andrey Novikov, 2013-03-11 07:32:54]

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/AdblockPlus.java (right):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/AdblockPlus.java:231: if
("org.adblockplus.android.ProxyService".equals(service.service.getClassName())
&& service.pid > 0)
ProxyService.class.getCanonicalName(), done

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/AdblockPlus.java:920: System.gc();
On 2013/03/11 06:57:38, Felix H. Dahlke wrote:
> I think we shouldn't do this, won't help us work around the memory issue (will
> explain that later).

Done.

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/Preferences.java (left):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Preferences.java:241: application.stopEngine(true);
Because we need to have JS running during all interactive session.

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
File src/org/adblockplus/android/Starter.java (right):

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Starter.java:38: boolean us =
"org.adblockplus.android".equals(intent.getData().getSchemeSpecificPart());
On 2013/03/11 06:57:38, Felix H. Dahlke wrote:
> Any idea how we could not hard code the package here? I'd suggest
> getClass().getPack().getName(), but it appears this has nothing to do with the
> package of the class Starter here. Would
> AdblockPlus.getClass().getPack().getName() make more sense?

Done.

http://codereview.adblockplus.org/9437197/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Starter.java:38: boolean us =
"org.adblockplus.android".equals(intent.getData().getSchemeSpecificPart());
On 2013/03/11 06:57:38, Felix H. Dahlke wrote:
> Any idea how we could not hard code the package here? I'd suggest
> getClass().getPack().getName(), but it appears this has nothing to do with the
> package of the class Starter here. Would
> AdblockPlus.getClass().getPack().getName() make more sense?

Done.

[Felix Dahlke, 2013-03-11 10:00:12]

LGTM :)