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

Issue 29760569: Issue 6238 - Download/store notifications.json (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 4 weeks ago by jens
Modified:
2 months, 1 week ago
Reviewers:
diegocarloslima, anton
Visibility:
Public.

Description

Issue 6238 - Download/store notifications.json

Patch Set 1 : #

Total comments: 13

Patch Set 2 : #

Total comments: 6

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+403 lines, -16 lines) Patch
M adblockplussbrowser/AndroidManifest.xml View 1 chunk +4 lines, -0 lines 0 comments Download
M adblockplussbrowser/res/values/sysstrings.xml View 1 chunk +4 lines, -0 lines 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/MainPreferences.java View 2 chunks +3 lines, -0 lines 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/MoreBlockingPreferenceCategory.java View 2 chunks +6 lines, -0 lines 0 comments Download
A adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java View 1 2 3 1 chunk +199 lines, -0 lines 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/Engine.java View 5 chunks +66 lines, -15 lines 0 comments Download
A adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/Notification.java View 1 2 1 chunk +89 lines, -0 lines 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/Subscriptions.java View 3 chunks +7 lines, -1 line 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java View 1 chunk +19 lines, -0 lines 0 comments Download
M adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SubscriptionUtils.java View 2 chunks +6 lines, -0 lines 0 comments Download

Messages

Total messages: 19
jens
4 months, 3 weeks ago (2018-04-26 12:46:41 UTC) #1
anton
https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml File adblockplussbrowser/AndroidManifest.xml (right): https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml#newcode42 adblockplussbrowser/AndroidManifest.xml:42: android:permission="android.permission.BIND_JOB_SERVICE" /> should not we add `android:exported="false"` here? https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java ...
4 months, 2 weeks ago (2018-05-03 07:23:21 UTC) #2
jens
https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml File adblockplussbrowser/AndroidManifest.xml (right): https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml#newcode42 adblockplussbrowser/AndroidManifest.xml:42: android:permission="android.permission.BIND_JOB_SERVICE" /> On 2018/05/03 07:23:20, anton wrote: > should ...
4 months, 2 weeks ago (2018-05-03 07:44:37 UTC) #3
anton
https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml File adblockplussbrowser/AndroidManifest.xml (right): https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/AndroidManifest.xml#newcode42 adblockplussbrowser/AndroidManifest.xml:42: android:permission="android.permission.BIND_JOB_SERVICE" /> On 2018/05/03 07:44:37, jens wrote: > On ...
4 months, 2 weeks ago (2018-05-03 07:56:27 UTC) #4
jens
https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java File adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java (right): https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java#newcode70 adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java:70: editor.putLong(context.getString(keyResId), value).apply(); On 2018/05/03 07:56:26, anton wrote: > On ...
4 months, 2 weeks ago (2018-05-03 08:09:15 UTC) #5
anton
On 2018/05/03 08:09:15, jens wrote: > https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java > File > adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java > (right): > > ...
4 months, 2 weeks ago (2018-05-03 08:15:45 UTC) #6
jens
On 2018/05/03 08:15:45, anton wrote: > On 2018/05/03 08:09:15, jens wrote: > > > https://codereview.adblockplus.org/29760569/diff/29762606/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/util/SharedPrefsUtils.java ...
4 months, 2 weeks ago (2018-05-03 08:37:55 UTC) #7
anton
On 2018/05/03 08:37:55, jens wrote: > On 2018/05/03 08:15:45, anton wrote: > > On 2018/05/03 ...
4 months, 2 weeks ago (2018-05-03 09:08:38 UTC) #8
jens
On 2018/05/03 09:08:38, anton wrote: > On 2018/05/03 08:37:55, jens wrote: > > On 2018/05/03 ...
4 months, 2 weeks ago (2018-05-03 11:20:25 UTC) #9
anton
On 2018/05/03 11:20:25, jens wrote: > On 2018/05/03 09:08:38, anton wrote: > > On 2018/05/03 ...
4 months, 2 weeks ago (2018-05-03 14:16:54 UTC) #10
diegocarloslima
https://codereview.adblockplus.org/29760569/diff/29769555/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java File adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java (right): https://codereview.adblockplus.org/29760569/diff/29769555/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java#newcode185 adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java:185: private int responseCode = 0; No need to initialize ...
4 months, 1 week ago (2018-05-10 13:41:59 UTC) #11
jens
https://codereview.adblockplus.org/29760569/diff/29769555/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java File adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java (right): https://codereview.adblockplus.org/29760569/diff/29769555/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java#newcode185 adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java:185: private int responseCode = 0; On 2018/05/10 13:41:58, diegocarloslima ...
4 months, 1 week ago (2018-05-15 09:17:02 UTC) #12
jens
On 2018/05/15 09:17:02, jens wrote: > https://codereview.adblockplus.org/29760569/diff/29769555/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java > File > adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java > (right): > > ...
4 months, 1 week ago (2018-05-15 09:18:14 UTC) #13
anton
https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java File adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java (right): https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java#newcode187 adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java:187: private String responseText = null; what about removing init ...
4 months, 1 week ago (2018-05-15 09:55:56 UTC) #14
jens
https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java File adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java (right): https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java#newcode187 adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java:187: private String responseText = null; On 2018/05/15 09:55:56, anton ...
4 months, 1 week ago (2018-05-15 09:57:46 UTC) #15
anton
On 2018/05/15 09:57:46, jens wrote: > https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java > File > adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java > (right): > > ...
4 months, 1 week ago (2018-05-15 10:37:06 UTC) #16
diegocarloslima
On 2018/05/15 10:37:06, anton wrote: > On 2018/05/15 09:57:46, jens wrote: > > > https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java ...
4 months ago (2018-05-22 13:04:27 UTC) #17
anton
On 2018/05/22 13:04:27, diegocarloslima wrote: > On 2018/05/15 10:37:06, anton wrote: > > On 2018/05/15 ...
2 months, 2 weeks ago (2018-07-02 10:07:41 UTC) #18
jens
2 months, 2 weeks ago (2018-07-03 09:03:26 UTC) #19
On 2018/07/02 10:07:41, anton wrote:
> On 2018/05/22 13:04:27, diegocarloslima wrote:
> > On 2018/05/15 10:37:06, anton wrote:
> > > On 2018/05/15 09:57:46, jens wrote:
> > > >
> > >
> >
>
https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser...
> > > > File
> > > >
> > >
> >
>
adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java
> > > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.adblockplus.org/29760569/diff/29782558/adblockplussbrowser...
> > > >
> > >
> >
>
adblockplussbrowser/src/main/java/org/adblockplus/sbrowser/contentblocker/engine/DownloadJobService.java:187:
> > > > private String responseText = null;
> > > > On 2018/05/15 09:55:56, anton wrote:
> > > > > what about removing init to `null` here too? (if you've removed 0
above)
> > > > 
> > > > Acknowledged.
> > > > I removed it and added a new patch set
> > > 
> > > LGTM
> > 
> > LGTM
> 
> Is it closed? The ticket is still in `reviewing` state.

We waiting for the China release (which will happen in the next days) before we
land this changes.
Sign in to reply to this message.

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