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

Unified Diff: safari/contentBlocking.js

Issue 29503587: Issue 5464 - Upgrade to new asynchronous version of abp2blocklist (Closed) Base URL: https://hg.adblockplus.org/adblockpluschrome/
Patch Set: Patch RegExpFilter.typeMap with WEBRTC Created Aug. 14, 2017, 5:21 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
« no previous file with comments | « dependencies ('k') | safari/ext/common.js » ('j') | safari/ext/common.js » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: safari/contentBlocking.js
===================================================================
--- a/safari/contentBlocking.js
+++ b/safari/contentBlocking.js
@@ -32,74 +32,111 @@
{
port.off("safari.legacyAPISupported", onLegacyAPISupported);
resolve(msg.legacyAPISupported);
}
port.on("safari.legacyAPISupported", onLegacyAPISupported);
});
let contentBlockingActive = false;
let afterContentBlockingFinished = null;
+let pendingContentBlockerUpdate = null;
let contentBlockListDirty = true;
let lastSetContentBlockerError;
function clearBlockCounters()
{
ext.pages.query({}, pages =>
{
for (let page of pages)
page.browserAction.setBadge();
});
}
-function setContentBlocker(callback)
+function setContentBlocker()
{
- // When given the same rules as last time setContentBlocker will always give
- // null (success) to the callback, even when there was actually an error. We
- // cache the last result therefore so that we can provide a consistent result
- // and also to avoid wastefully regenerating an identical blocklist.
- if (!contentBlockListDirty)
+ return new Promise((resolve, reject) =>
{
- callback(lastSetContentBlockerError);
- return;
- }
+ // Reset state and either fulfill or reject this promise.
+ let completePromise = error =>
+ {
+ lastSetContentBlockerError = error;
+ contentBlockListDirty = false;
- let contentBlockerList = new ContentBlockerList();
- for (let subscription of FilterStorage.subscriptions)
- if (!subscription.disabled)
- for (let filter of subscription.filters)
- contentBlockerList.addFilter(filter);
+ if (error)
+ reject(error);
+ else
+ resolve();
+ };
+
+ // When given the same rules as last time setContentBlocker will always
+ // resolve with null (success), even when there was actually an
+ // error. We cache the last result therefore so that we can provide a
+ // consistent result and also to avoid wastefully regenerating an identical
+ // blocklist.
+ if (!contentBlockListDirty)
+ {
+ completePromise(lastSetContentBlockerError);
+ return;
+ }
- contentBlockListDirty = false;
- safari.extension.setContentBlocker(
- // There is a strange bug in setContentBlocker for Safari 9 where if both
- // the callback parameter is provided and the rules aren't converted to a
- // JSON string it fails. Worse still it actually performs the callback twice
- // too, firstly with an empty string and then with an Error:
- // "Extension compilation failed: Failed to parse the JSON String."
- // To mitigate this we convert the rules to JSON here and also ignore
- // callback values of "". (Usually the callback is performed with either
- // null for success or an Error on failure.)
- // Bug #26322821 filed on bugreport.apple.com
- JSON.stringify(contentBlockerList.generateRules()),
- function(error)
+ let contentBlockerList = new ContentBlockerList();
+ for (let subscription of FilterStorage.subscriptions)
+ {
+ if (!subscription.disabled)
+ {
+ for (let filter of subscription.filters)
+ contentBlockerList.addFilter(filter);
+ }
+ }
+
+ contentBlockerList.generateRules().then(rules =>
{
- if (error == "")
- return;
+ safari.extension.setContentBlocker(
+ // There is a strange bug in setContentBlocker for Safari 9 where if
+ // both the callback parameter is provided and the rules aren't
+ // converted to a JSON string it fails. Worse still it actually
+ // performs the callback twice too, firstly with an empty string and
+ // then with an Error: "Extension compilation failed: Failed to parse
+ // the JSON String." To mitigate this we convert the rules to JSON here
+ // and also ignore callback values of "". (Usually the callback is
+ // performed with either null for success or an Error on failure.)
+ // Bug #26322821 filed on bugreport.apple.com
+ JSON.stringify(rules),
+ function(error)
+ {
+ if (error == "")
+ return;
- lastSetContentBlockerError = error;
- callback(error);
- }
- );
+ completePromise(error);
+ }
+ );
+ })
+ .catch(completePromise);
+ });
}
function updateContentBlocker(isStartup, legacyAPISupported)
{
+ // Another update can be requested while one is still in progress (e.g. the
+ // user adds filter lists in quick succession). When this happens, save the
+ // request and execute it later.
+ if (afterContentBlockingFinished)
+ {
+ pendingContentBlockerUpdate = {
+ params: Array.from(arguments),
+ // Save the current dirty state so we can set it later before calling
+ // this function again.
+ setDirty: contentBlockListDirty
+ };
+ return;
+ }
+
afterContentBlockingFinished = new Promise(resolve =>
{
- setContentBlocker(error =>
+ let setContentBlockerHandler = error =>
{
if (error instanceof Error)
{
let suppressErrorMessage = false;
// If the content blocking API fails the first time it's used the
// legacy blocking API (if available) won't have been disabled.
if (!contentBlockingActive && legacyAPISupported)
@@ -115,19 +152,42 @@
alert(error.message);
}
else if (!contentBlockingActive)
{
contentBlockingActive = true;
clearBlockCounters();
}
+ afterContentBlockingFinished = null;
resolve(contentBlockingActive);
- afterContentBlockingFinished = null;
- });
+ };
+
+ // There's no need for a catch handler at the end because the handler is
+ // not expected to throw any errors. If this changes, however, then the
+ // error must be handled appropriately.
+ setContentBlocker().then(setContentBlockerHandler,
+ setContentBlockerHandler);
Sebastian Noack 2017/08/15 15:25:01 It seems somewhat dirty to me to register the same
Manish Jethani 2017/08/16 10:23:01 Done (but see comments).
+ })
+ .then(active =>
+ {
+ // If there's another update pending, execute it now.
+ if (pendingContentBlockerUpdate)
+ {
+ let {params, setDirty} = pendingContentBlockerUpdate;
+ pendingContentBlockerUpdate = null;
+
+ if (setDirty)
+ contentBlockListDirty = true;
+
+ updateContentBlocker.apply(null, params);
+ return afterContentBlockingFinished;
+ }
+
+ return active;
});
}
if (contentBlockingSupported)
{
Promise.all([Prefs.untilLoaded,
FilterNotifier.once("load"),
legacyAPISupported]).then(resolvedValues =>
« no previous file with comments | « dependencies ('k') | safari/ext/common.js » ('j') | safari/ext/common.js » ('J')

Powered by Google App Engine
This is Rietveld