From e40a66e2945bcabb7233f7397da0849e4cbd70f2 Mon Sep 17 00:00:00 2001 From: Raymond Hill Date: Thu, 12 Apr 2018 17:32:38 -0400 Subject: [PATCH] code review of efcab145978b: fix collected errors; replace Promises with callbacks --- platform/chromium/vapi-background.js | 127 +++++++++++---------------- 1 file changed, 53 insertions(+), 74 deletions(-) diff --git a/platform/chromium/vapi-background.js b/platform/chromium/vapi-background.js index b4d1fed01..4ba0a1595 100644 --- a/platform/chromium/vapi-background.js +++ b/platform/chromium/vapi-background.js @@ -37,10 +37,22 @@ vAPI.cantWebsocket = chrome.webRequest.ResourceType instanceof Object === false || chrome.webRequest.ResourceType.WEBSOCKET !== 'websocket'; -// https://issues.adblockplus.org/ticket/5695 -// - Good idea, adopted: cleaner way to detect user-stylesheet support. +vAPI.lastError = function() { + return chrome.runtime.lastError; +}; + +// https://github.com/gorhill/uBlock/issues/875 +// https://code.google.com/p/chromium/issues/detail?id=410868#c8 +// Must not leave `lastError` unchecked. +vAPI.resetLastError = function() { + void chrome.runtime.lastError; +}; + vAPI.supportsUserStylesheets = vAPI.webextFlavor.soup.has('user_stylesheet'); -vAPI.insertCSS = chrome.tabs.insertCSS; + +vAPI.insertCSS = function(tabId, details) { + return chrome.tabs.insertCSS(tabId, details, vAPI.resetLastError); +}; var noopFunc = function(){}; @@ -103,12 +115,6 @@ vAPI.browserSettings = (function() { } })(), - // https://github.com/gorhill/uBlock/issues/875 - // Must not leave `lastError` unchecked. - noopCallback: function() { - void chrome.runtime.lastError; - }, - // Calling with `true` means IP address leak is not prevented. // https://github.com/gorhill/uBlock/issues/533 // We must first check wether this Chromium-based browser was compiled @@ -170,12 +176,12 @@ vAPI.browserSettings = (function() { if ( setting ) { cpn.webRTCMultipleRoutesEnabled.clear({ scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } else { cpn.webRTCMultipleRoutesEnabled.set({ value: false, scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } } catch(ex) { console.error(ex); @@ -188,14 +194,14 @@ vAPI.browserSettings = (function() { if ( setting ) { cpn.webRTCIPHandlingPolicy.clear({ scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } else { // https://github.com/uBlockOrigin/uAssets/issues/333#issuecomment-289426678 // - Leverage virtuous side-effect of strictest setting. cpn.webRTCIPHandlingPolicy.set({ value: 'disable_non_proxied_udp', scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } } catch(ex) { console.error(ex); @@ -214,12 +220,12 @@ vAPI.browserSettings = (function() { if ( !!details[setting] ) { chrome.privacy.network.networkPredictionEnabled.clear({ scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } else { chrome.privacy.network.networkPredictionEnabled.set({ value: false, scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } } catch(ex) { console.error(ex); @@ -231,12 +237,12 @@ vAPI.browserSettings = (function() { if ( !!details[setting] ) { chrome.privacy.websites.hyperlinkAuditingEnabled.clear({ scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } else { chrome.privacy.websites.hyperlinkAuditingEnabled.set({ value: false, scope: 'regular' - }, this.noopCallback); + }, vAPI.resetLastError); } } catch(ex) { console.error(ex); @@ -271,7 +277,6 @@ vAPI.tabs = {}; // network requests. vAPI.isBehindTheSceneTabId = function(tabId) { - if ( typeof tabId === 'string' ) { debugger; } return tabId < 0; }; @@ -284,11 +289,9 @@ vAPI.anyTabId = -2; // one of the existing tab // To remove when tabId-as-integer has been tested enough. var toChromiumTabId = function(tabId) { - if ( typeof tabId === 'string' ) { debugger; } - if ( typeof tabId !== 'number' || isNaN(tabId) || tabId === -1 ) { - return 0; - } - return tabId; + return typeof tabId === 'number' && !isNaN(tabId) && tabId > 0 ? + tabId : + 0; }; /******************************************************************************/ @@ -401,7 +404,7 @@ vAPI.tabs.get = function(tabId, callback) { chrome.tabs.query( { active: true, currentWindow: true }, function(tabs) { - if ( chrome.runtime.lastError ) { /* noop */ } + void chrome.runtime.lastError; callback( Array.isArray(tabs) && tabs.length !== 0 ? tabs[0] : null ); @@ -417,7 +420,7 @@ vAPI.tabs.get = function(tabId, callback) { } chrome.tabs.get(tabId, function(tab) { - if ( chrome.runtime.lastError ) { /* noop */ } + void chrome.runtime.lastError; callback(tab); }); }; @@ -528,7 +531,7 @@ vAPI.tabs.open = function(details) { targetURLWithoutHash = pos === -1 ? targetURL : targetURL.slice(0, pos); chrome.tabs.query({ url: targetURLWithoutHash }, function(tabs) { - if ( chrome.runtime.lastError ) { /* noop */ } + void chrome.runtime.lastError; var tab = Array.isArray(tabs) && tabs[0]; if ( !tab ) { wrapper(); @@ -562,12 +565,7 @@ vAPI.tabs.replace = function(tabId, url) { targetURL = vAPI.getURL(targetURL); } - chrome.tabs.update(tabId, { url: targetURL }, function() { - // https://code.google.com/p/chromium/issues/detail?id=410868#c8 - if ( chrome.runtime.lastError ) { - /* noop */ - } - }); + chrome.tabs.update(tabId, { url: targetURL }, vAPI.resetLastError); }; /******************************************************************************/ @@ -576,14 +574,7 @@ vAPI.tabs.remove = function(tabId) { tabId = toChromiumTabId(tabId); if ( tabId === 0 ) { return; } - var onTabRemoved = function() { - // https://code.google.com/p/chromium/issues/detail?id=410868#c8 - if ( chrome.runtime.lastError ) { - /* noop */ - } - }; - - chrome.tabs.remove(tabId, onTabRemoved); + chrome.tabs.remove(tabId, vAPI.resetLastError); }; /******************************************************************************/ @@ -592,17 +583,10 @@ vAPI.tabs.reload = function(tabId, bypassCache) { tabId = toChromiumTabId(tabId); if ( tabId === 0 ) { return; } - var onReloaded = function() { - // https://code.google.com/p/chromium/issues/detail?id=410868#c8 - if ( chrome.runtime.lastError ) { - /* noop */ - } - }; - chrome.tabs.reload( tabId, { bypassCache: bypassCache === true }, - onReloaded + vAPI.resetLastError ); }; @@ -615,12 +599,8 @@ vAPI.tabs.select = function(tabId) { if ( tabId === 0 ) { return; } chrome.tabs.update(tabId, { active: true }, function(tab) { - if ( chrome.runtime.lastError ) { - /* noop */ - } - if ( !tab ) { - return; - } + void chrome.runtime.lastError; + if ( !tab ) { return; } chrome.windows.update(tab.windowId, { focused: true }); }); }; @@ -630,9 +610,7 @@ vAPI.tabs.select = function(tabId) { vAPI.tabs.injectScript = function(tabId, details, callback) { var onScriptExecuted = function() { // https://code.google.com/p/chromium/issues/detail?id=410868#c8 - if ( chrome.runtime.lastError ) { - /* noop */ - } + void chrome.runtime.lastError; if ( typeof callback === 'function' ) { callback(); } @@ -835,21 +813,28 @@ vAPI.messaging.onPortMessage = (function() { details.runAt = 'document_start'; } var cssText; - const cssPromises = []; + var countdown = 0; + var countdownHandler = function() { + void chrome.runtime.lastError; + countdown -= 1; + if ( countdown === 0 && typeof callback === 'function' ) { + callback(); + } + }; for ( cssText of msg.add ) { + countdown += 1; details.code = cssText; - cssPromises.push(chrome.tabs.insertCSS(tabId, details)); + chrome.tabs.insertCSS(tabId, details, countdownHandler); } if ( typeof chrome.tabs.removeCSS === 'function' ) { for ( cssText of msg.remove ) { + countdown += 1; details.code = cssText; - cssPromises.push(chrome.tabs.removeCSS(tabId, details)); + chrome.tabs.removeCSS(tabId, details, countdownHandler); } } - if ( typeof callback === 'function' ) { - Promise.all(cssPromises).then(() => { - callback(); - }, null); + if ( countdown === 0 && typeof callback === 'function' ) { + callback(); } break; } @@ -990,9 +975,10 @@ vAPI.contextMenu = chrome.contextMenus && { _callback: null, _entries: [], _createEntry: function(entry) { - chrome.contextMenus.create(JSON.parse(JSON.stringify(entry)), function() { - void chrome.runtime.lastError; - }); + chrome.contextMenus.create( + JSON.parse(JSON.stringify(entry)), + vAPI.resetLastError + ); }, onMustUpdate: function() {}, setEntries: function(entries, callback) { @@ -1038,13 +1024,6 @@ vAPI.commands = chrome.commands; /******************************************************************************/ /******************************************************************************/ -vAPI.lastError = function() { - return chrome.runtime.lastError; -}; - -/******************************************************************************/ -/******************************************************************************/ - // This is called only once, when everything has been loaded in memory after // the extension was launched. It can be used to inject content scripts // in already opened web pages, to remove whatever nuisance could make it to