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

Issue 29677555: Issue 6299 - Avoid main thread lock on URL loading (Closed)

Created:
Jan. 23, 2018, 6:12 a.m. by anton
Modified:
Jan. 23, 2018, 11:51 a.m.
Reviewers:
diegocarloslima, jens
CC:
René Jeschke
Visibility:
Public.

Description

Issue 6299 - Avoid main thread lock on URL loading

Patch Set 1 #

Total comments: 1

Patch Set 2 : Updated versions #

Total comments: 2

Patch Set 3 : added "final" for domain #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -37 lines) Patch
M libadblockplus-android-webview/AndroidManifest.xml View 1 1 chunk +2 lines, -2 lines 0 comments Download
M libadblockplus-android-webview/build.gradle View 1 1 chunk +2 lines, -2 lines 0 comments Download
M libadblockplus-android-webview/pom.xml View 1 1 chunk +1 line, -1 line 0 comments Download
M libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java View 1 2 5 chunks +20 lines, -25 lines 0 comments Download
M libadblockplus-android-webviewapp/AndroidManifest.xml View 1 1 chunk +2 lines, -2 lines 0 comments Download
M libadblockplus-android-webviewapp/build.gradle View 1 1 chunk +2 lines, -2 lines 0 comments Download
M libadblockplus-android-webviewapp/pom.xml View 1 2 chunks +2 lines, -2 lines 0 comments Download
M libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5
anton
https://codereview.adblockplus.org/29677555/diff/29677556/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/29677555/diff/29677556/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#newcode1016 libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:1016: if (domain == null) Not sure if it can ...
Jan. 23, 2018, 6:14 a.m. (2018-01-23 06:14:10 UTC) #1
jens
https://codereview.adblockplus.org/29677555/diff/29677559/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/29677555/diff/29677559/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#newcode1015 libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:1015: String domain = provider.getEngine().getFilterEngine().getHostFromURL(url); Minor, but I think domain ...
Jan. 23, 2018, 10:08 a.m. (2018-01-23 10:08:29 UTC) #2
anton
https://codereview.adblockplus.org/29677555/diff/29677559/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/29677555/diff/29677559/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java#newcode1015 libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:1015: String domain = provider.getEngine().getFilterEngine().getHostFromURL(url); On 2018/01/23 10:08:29, jens wrote: ...
Jan. 23, 2018, 10:44 a.m. (2018-01-23 10:44:06 UTC) #3
diegocarloslima
On 2018/01/23 10:44:06, anton wrote: > https://codereview.adblockplus.org/29677555/diff/29677559/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java > File > libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java > (right): > > ...
Jan. 23, 2018, 11:25 a.m. (2018-01-23 11:25:56 UTC) #4
jens
Jan. 23, 2018, 11:28 a.m. (2018-01-23 11:28:52 UTC) #5 
On 2018/01/23 11:25:56, diegocarloslima wrote:
> On 2018/01/23 10:44:06, anton wrote:
> >
>
https://codereview.adblockplus.org/29677555/diff/29677559/libadblockplus-andr...
> > File
> >
>
libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java
> > (right):
> > 
> >
>
https://codereview.adblockplus.org/29677555/diff/29677559/libadblockplus-andr...
> >
>
libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java:1015:
> > String domain = provider.getEngine().getFilterEngine().getHostFromURL(url);
> > On 2018/01/23 10:08:29, jens wrote:
> > > Minor, but I think domain could be final.
> > 
> > fixed, uploaded new patch set
> 
> LGTM

LGTM

Powered by Google App Engine
This is Rietveld