Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 8954027: android: Crash fixes (Closed)

Created:
Nov. 27, 2012, 10:57 a.m. by Andrey Novikov
Modified:
Nov. 28, 2012, 8:42 a.m.
Reviewers:
Felix Dahlke
Visibility:
Public.

Description

android: Crash fixes

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -18 lines) Patch
M AndroidManifest.xml View 2 chunks +2 lines, -2 lines 0 comments Download
M src/org/adblockplus/android/AdblockPlus.java View 1 chunk +5 lines, -0 lines 2 comments Download
M src/org/adblockplus/android/Preferences.java View 3 chunks +22 lines, -16 lines 3 comments Download

Messages

Total messages: 4
Andrey Novikov
Nov. 27, 2012, 10:57 a.m. (2012-11-27 10:57:51 UTC) #1
Felix Dahlke
http://codereview.adblockplus.org/8954027/diff/1/src/org/adblockplus/android/AdblockPlus.java File src/org/adblockplus/android/AdblockPlus.java (right): http://codereview.adblockplus.org/8954027/diff/1/src/org/adblockplus/android/AdblockPlus.java#newcode459 src/org/adblockplus/android/AdblockPlus.java:459: if (js == null) This is the same safe ...
Nov. 28, 2012, 8:12 a.m. (2012-11-28 08:12:56 UTC) #2
Andrey Novikov
http://codereview.adblockplus.org/8954027/diff/1/src/org/adblockplus/android/AdblockPlus.java File src/org/adblockplus/android/AdblockPlus.java (right): http://codereview.adblockplus.org/8954027/diff/1/src/org/adblockplus/android/AdblockPlus.java#newcode459 src/org/adblockplus/android/AdblockPlus.java:459: if (js == null) On 2012/11/28 08:12:56, Felix H. ...
Nov. 28, 2012, 8:25 a.m. (2012-11-28 08:25:01 UTC) #3
Felix Dahlke
Nov. 28, 2012, 8:37 a.m. (2012-11-28 08:37:37 UTC) #4
LGTM, your call if you want to address my remaining comment.

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

http://codereview.adblockplus.org/8954027/diff/1/src/org/adblockplus/android/...
src/org/adblockplus/android/Preferences.java:106: // Initialize subscription
list
I think it makes sense to put this part into an initSubscriptionList function,
but I don't insist of course :)

Powered by Google App Engine
This is Rietveld