Unified Diff: libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java
Issue 29691582:
Issue 6366 - Elemhide thread is accessing released engine (Closed)
Patch Set:
Created Feb. 7, 2018, 1:48 p.m.
Use n/p to move between diff chunks;
N/P to move between comments.
« no previous file with comments
|
« no previous file
|
libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngineProvider.java » ('j')
|
libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')
| Index: libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java |
| diff --git a/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java b/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java |
| index 73d88b377fa1b9f16ce3ed49441b1fc46231fe94..98b67a57e8b88d9e139ebc61160b58d2f193ab4b 100644 |
| --- a/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java |
| +++ b/libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java |
| @@ -818,92 +818,95 @@ public class AdblockWebView extends WebView |
| WebView webview, String url, boolean isMainFrame, |
| boolean isXmlHttpRequest, String[] referrerChainArray) |
| { |
| - // if dispose() was invoke, but the page is still loading then just let it go |
| - if (provider.getCounter() == 0) |
| + synchronized (provider.getEngineLock()) |
| { |
| - e("FilterEngine already disposed, allow loading"); |
| - |
| - // allow loading by returning null |
| - return null; |
| - } |
| - else |
| - { |
| - provider.waitForReady(); |
| - } |
| - |
| - if (isMainFrame) |
| - { |
| - // never blocking main frame requests, just subrequests |
| - w(url + " is main frame, allow loading"); |
| - |
| - // allow loading by returning null |
| - return null; |
| - } |
| - |
| - // whitelisted |
| - if (provider.getEngine().isDomainWhitelisted(url, referrerChainArray)) |
| - { |
| - w(url + " domain is whitelisted, allow loading"); |
| - |
| - // allow loading by returning null |
| - return null; |
| - } |
| - |
| - if (provider.getEngine().isDocumentWhitelisted(url, referrerChainArray)) |
| - { |
| - w(url + " document is whitelisted, allow loading"); |
| - |
| - // allow loading by returning null |
| - return null; |
| - } |
| + // if dispose() was invoke, but the page is still loading then just let it go |
|
diegocarloslima
2018/02/07 14:00:06
Not really related to the issue, but it seems that
anton
2018/02/07 14:03:22
Yes, it should be `invoked`.
Not fixing now as it'
|
| + if (provider.getCounter() == 0) |
| + { |
| + e("FilterEngine already disposed, allow loading"); |
| - // determine the content |
| - FilterEngine.ContentType contentType; |
| - if (isXmlHttpRequest) |
| - { |
| - contentType = FilterEngine.ContentType.XMLHTTPREQUEST; |
| - } |
| - else |
| - { |
| - if (RE_JS.matcher(url).find()) |
| + // allow loading by returning null |
| + return null; |
| + } |
| + else |
| { |
| - contentType = FilterEngine.ContentType.SCRIPT; |
| + provider.waitForReady(); |
| } |
| - else if (RE_CSS.matcher(url).find()) |
| + |
| + if (isMainFrame) |
| { |
| - contentType = FilterEngine.ContentType.STYLESHEET; |
| + // never blocking main frame requests, just subrequests |
| + w(url + " is main frame, allow loading"); |
| + |
| + // allow loading by returning null |
| + return null; |
| } |
| - else if (RE_IMAGE.matcher(url).find()) |
| + |
| + // whitelisted |
| + if (provider.getEngine().isDomainWhitelisted(url, referrerChainArray)) |
| { |
| - contentType = FilterEngine.ContentType.IMAGE; |
| + w(url + " domain is whitelisted, allow loading"); |
| + |
| + // allow loading by returning null |
| + return null; |
| } |
| - else if (RE_FONT.matcher(url).find()) |
| + |
| + if (provider.getEngine().isDocumentWhitelisted(url, referrerChainArray)) |
| { |
| - contentType = FilterEngine.ContentType.FONT; |
| + w(url + " document is whitelisted, allow loading"); |
| + |
| + // allow loading by returning null |
| + return null; |
| } |
| - else if (RE_HTML.matcher(url).find()) |
| + |
| + // determine the content |
| + FilterEngine.ContentType contentType; |
| + if (isXmlHttpRequest) |
| { |
| - contentType = FilterEngine.ContentType.SUBDOCUMENT; |
| + contentType = FilterEngine.ContentType.XMLHTTPREQUEST; |
| } |
| else |
| { |
| - contentType = FilterEngine.ContentType.OTHER; |
| + if (RE_JS.matcher(url).find()) |
| + { |
| + contentType = FilterEngine.ContentType.SCRIPT; |
| + } |
| + else if (RE_CSS.matcher(url).find()) |
| + { |
| + contentType = FilterEngine.ContentType.STYLESHEET; |
| + } |
| + else if (RE_IMAGE.matcher(url).find()) |
| + { |
| + contentType = FilterEngine.ContentType.IMAGE; |
| + } |
| + else if (RE_FONT.matcher(url).find()) |
| + { |
| + contentType = FilterEngine.ContentType.FONT; |
| + } |
| + else if (RE_HTML.matcher(url).find()) |
| + { |
| + contentType = FilterEngine.ContentType.SUBDOCUMENT; |
| + } |
| + else |
| + { |
| + contentType = FilterEngine.ContentType.OTHER; |
| + } |
| } |
| - } |
| - // check if we should block |
| - if (provider.getEngine().matches(url, contentType, referrerChainArray)) |
| - { |
| - w("Blocked loading " + url); |
| + // check if we should block |
| + if (provider.getEngine().matches(url, contentType, referrerChainArray)) |
| + { |
| + w("Blocked loading " + url); |
| - // if we should block, return empty response which results in 'errorLoading' callback |
| - return new WebResourceResponse("text/plain", "UTF-8", null); |
| - } |
| + // if we should block, return empty response which results in 'errorLoading' callback |
| + return new WebResourceResponse("text/plain", "UTF-8", null); |
| + } |
| - d("Allowed loading " + url); |
| + d("Allowed loading " + url); |
| - // continue by returning null |
| - return null; |
| + // continue by returning null |
| + return null; |
| + } |
| } |
| @TargetApi(Build.VERSION_CODES.LOLLIPOP) |
| @@ -971,76 +974,79 @@ public class AdblockWebView extends WebView |
| @Override |
| public void run() |
| { |
| - try |
| + synchronized (provider.getEngineLock()) |
| { |
| - if (provider.getCounter() == 0) |
| - { |
| - w("FilterEngine already disposed"); |
| - selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING; |
| - } |
| - else |
| + try |
| { |
| - provider.waitForReady(); |
| - String[] referrers = new String[] |
| - { |
| - url |
| - }; |
| + if (provider.getCounter() == 0) |
| + { |
| + w("FilterEngine already disposed"); |
| + selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING; |
| + } |
| + else |
| + { |
| + provider.waitForReady(); |
| + String[] referrers = new String[] |
| + { |
| + url |
| + }; |
| - List<Subscription> subscriptions = provider |
| - .getEngine() |
| - .getFilterEngine() |
| - .getListedSubscriptions(); |
| + List<Subscription> subscriptions = provider |
| + .getEngine() |
| + .getFilterEngine() |
| + .getListedSubscriptions(); |
| - try |
| - { |
| - d("Listed subscriptions: " + subscriptions.size()); |
| - if (debugMode) |
| + try |
| + { |
| + d("Listed subscriptions: " + subscriptions.size()); |
| + if (debugMode) |
| + { |
| + for (Subscription eachSubscription : subscriptions) |
| + { |
| + d("Subscribed to " |
| + + (eachSubscription.isDisabled() ? "disabled" : "enabled") |
| + + " " + eachSubscription); |
| + } |
| + } |
| + } |
| + finally |
| { |
| for (Subscription eachSubscription : subscriptions) |
| { |
| - d("Subscribed to " |
| - + (eachSubscription.isDisabled() ? "disabled" : "enabled") |
| - + " " + eachSubscription); |
| + eachSubscription.dispose(); |
| } |
| } |
| - } |
| - finally |
| - { |
| - for (Subscription eachSubscription : subscriptions) |
| + |
| + final String domain = provider.getEngine().getFilterEngine().getHostFromURL(url); |
| + if (domain == null) |
| { |
| - eachSubscription.dispose(); |
| + e("Failed to extract domain from " + url); |
| + selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING; |
| } |
| - } |
| + else |
| + { |
| + d("Requesting elemhide selectors from AdblockEngine for " + url + " in " + this); |
| + List<String> selectors = provider |
| + .getEngine() |
| + .getElementHidingSelectors(url, domain, referrers); |
| - final String domain = provider.getEngine().getFilterEngine().getHostFromURL(url); |
| - if (domain == null) |
| + d("Finished requesting elemhide selectors, got " + selectors.size() + " in " + this); |
| + selectorsString = Utils.stringListToJsonArray(selectors); |
| + } |
| + } |
| + } |
| + finally |
| + { |
| + if (isCancelled.get()) |
| { |
| - e("Failed to extract domain from " + url); |
| - selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING; |
| + w("This thread is cancelled, exiting silently " + this); |
| } |
| else |
| { |
| - d("Requesting elemhide selectors from AdblockEngine for " + url + " in " + this); |
| - List<String> selectors = provider |
| - .getEngine() |
| - .getElementHidingSelectors(url, domain, referrers); |
| - |
| - d("Finished requesting elemhide selectors, got " + selectors.size() + " in " + this); |
| - selectorsString = Utils.stringListToJsonArray(selectors); |
| + finish(selectorsString); |
| } |
| } |
| } |
| - finally |
| - { |
| - if (isCancelled.get()) |
| - { |
| - w("This thread is cancelled, exiting silently " + this); |
| - } |
| - else |
| - { |
| - finish(selectorsString); |
| - } |
| - } |
| } |
| private void onFinished() |
« no previous file with comments
|
« no previous file
|
libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngineProvider.java » ('j')
|
libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java » ('J')
