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

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.
Jump to:
View side-by-side diff with in-line comments
Download patch
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()

Powered by Google App Engine
This is Rietveld