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

Issue 29397615: Issue 5053 - Release JsValues explicitly (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 9 months ago by anton
Modified:
2 years, 9 months ago
CC:
René Jeschke
Visibility:
Public.

Description

Issue 5053 - Release JsValues explicitly

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -31 lines) Patch
M libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java View 1 chunk +13 lines, -3 lines 2 comments Download
M libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java View 10 chunks +155 lines, -26 lines 3 comments Download
M libadblockplus-android/src/org/adblockplus/libadblockplus/android/AndroidWebRequest.java View 2 chunks +26 lines, -2 lines 0 comments Download

Messages

Total messages: 12
anton
2 years, 9 months ago (2017-03-29 12:14:36 UTC) #1
sergei
LGTM. Is the number of users of JsEngine before disposing is usually (there are still ...
2 years, 9 months ago (2017-03-29 12:30:56 UTC) #2
anton
On 2017/03/29 12:30:56, sergei wrote: > LGTM. > > Is the number of users of ...
2 years, 9 months ago (2017-03-29 12:58:01 UTC) #3
sergei
On 2017/03/29 12:58:01, anton wrote: > On 2017/03/29 12:30:56, sergei wrote: > > LGTM. > ...
2 years, 9 months ago (2017-03-29 12:59:00 UTC) #4
anton
On 2017/03/29 12:59:00, sergei wrote: > On 2017/03/29 12:58:01, anton wrote: > > On 2017/03/29 ...
2 years, 9 months ago (2017-03-29 19:14:31 UTC) #5
anton
On 2017/03/29 19:14:31, anton wrote: > On 2017/03/29 12:59:00, sergei wrote: > > On 2017/03/29 ...
2 years, 9 months ago (2017-03-29 19:19:03 UTC) #6
diegocarloslima
https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java File libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java (right): https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#newcode996 libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:996: if (debugMode) Isn't this check redundant here, since d() ...
2 years, 9 months ago (2017-03-30 14:19:29 UTC) #7
anton
https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java File libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java (right): https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#newcode996 libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:996: if (debugMode) On 2017/03/30 14:19:29, diegocarloslima wrote: > Isn't ...
2 years, 9 months ago (2017-03-30 14:34:15 UTC) #8
diegocarloslima
https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java File libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java (right): https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java#newcode236 libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java:236: JsValue jsTitle = jsSubscription.getProperty("title"); On 2017/03/30 14:34:15, anton wrote: ...
2 years, 9 months ago (2017-03-30 14:47:18 UTC) #9
anton
On 2017/03/30 14:47:18, diegocarloslima wrote: > https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java > File > libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java > (right): > > ...
2 years, 9 months ago (2017-03-30 14:49:12 UTC) #10
diegocarloslima
On 2017/03/30 14:47:18, diegocarloslima wrote: > https://codereview.adblockplus.org/29397615/diff/29397616/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java > File > libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java > (right): > > ...
2 years, 9 months ago (2017-03-30 14:49:45 UTC) #11
Felix Dahlke
2 years, 9 months ago (2017-03-30 14:52:48 UTC) #12
LGTM - not pretty, but I don't see another way. I guess try-with-resources will
make this pretty again.
Sign in to reply to this message.

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