From 26802e36df00eefa35302030e40ff2e4a010bda3 Mon Sep 17 00:00:00 2001 From: tophf Date: Fri, 31 Mar 2017 11:46:18 +0300 Subject: [PATCH] Optimize startup: coalesce & debounce prefs.set Previously prefs.set broadcast many messages per each changed pref value to all open tabs, background page, popups. This lead to repeated and needless updates of various things like the toolbar icon, reapplying of styles, and whatnot. It could easily take more than 100ms on an average computer with many tabs open. Now we debounce the broadcast & sync.set and coalesce all values in one object which is then sent just once per destination. --- apply.js | 9 +-- background.js | 13 ++--- edit.js | 4 +- messaging.js | 13 ++--- popup.js | 27 +++++---- storage.js | 157 ++++++++++++++++++++++++-------------------------- update.js | 2 +- 7 files changed, 109 insertions(+), 116 deletions(-) diff --git a/apply.js b/apply.js index 4def780c..41a2c1a3 100644 --- a/apply.js +++ b/apply.js @@ -47,8 +47,7 @@ function applyOnMessage(request, sender, sendResponse) { applyOnMessage(Object.assign(request, {styles}))); return; } - // Also handle special request just for the pop-up - switch (request.method == 'updatePopup' ? request.reason : request.method) { + switch (request.method) { case 'styleDeleted': removeStyle(request.id, document); @@ -80,8 +79,10 @@ function applyOnMessage(request, sender, sendResponse) { replaceAll(request.styles, document); break; - case 'styleDisableAll': - doDisableAll(request.disableAll); + case 'prefChanged': + if ('disableAll' in request.prefs) { + doDisableAll(request.prefs.disableAll); + } break; case 'ping': diff --git a/background.js b/background.js index ad737a8a..c662a52d 100644 --- a/background.js +++ b/background.js @@ -72,14 +72,13 @@ function onBackgroundMessage(request, sender, sendResponse) { () => sendResponse(false)); return KEEP_CHANNEL_OPEN; - case 'styleDisableAll': - request = {prefName: 'disableAll', value: request.disableAll}; - // fallthrough to prefChanged - case 'prefChanged': - // eslint-disable-next-line no-use-before-define - if (typeof request.value == 'boolean' && contextMenus[request.prefName]) { - chrome.contextMenus.update(request.prefName, {checked: request.value}, ignoreChromeError); + for (var prefName in request.prefs) { // eslint-disable-line no-var + if (prefName in contextMenus) { // eslint-disable-line no-use-before-define + chrome.contextMenus.update(prefName, { + checked: request.prefs[prefName], + }, ignoreChromeError); + } } break; } diff --git a/edit.js b/edit.js index ffcffe7a..56c0667b 100644 --- a/edit.js +++ b/edit.js @@ -1824,8 +1824,8 @@ chrome.runtime.onMessage.addListener(function(request, sender, sendResponse) { } break; case "prefChanged": - if (request.prefName == "editor.smartIndent") { - CodeMirror.setOption("smartIndent", request.value); + if ('editor.smartIndent' in request.prefs) { + CodeMirror.setOption('smartIndent', request.prefs['editor.smartIndent']); } break; case 'editDeleteText': diff --git a/messaging.js b/messaging.js index d2e8ecba..a9ef239d 100644 --- a/messaging.js +++ b/messaging.js @@ -20,17 +20,14 @@ function notifyAllTabs(request) { updateIcon(tab); } }); - // notify all open popups - const reqPopup = Object.assign({}, request, {method: 'updatePopup', reason: request.method}); - chrome.runtime.sendMessage(reqPopup); // notify self: the message no longer is sent to the origin in new Chrome - if (typeof applyOnMessage !== 'undefined') { - applyOnMessage(reqPopup); - } - // notify self: pref changed by background page - if (request.method == 'prefChanged' && typeof onBackgroundMessage !== 'undefined') { + if (window.applyOnMessage) { + applyOnMessage(request); + } else if (window.onBackgroundMessage) { onBackgroundMessage(request); } + // notify background page and all open popups + chrome.runtime.sendMessage(request); } diff --git a/popup.js b/popup.js index 553d3ab3..dd64a4d4 100644 --- a/popup.js +++ b/popup.js @@ -18,16 +18,22 @@ getActiveTabRealURL().then(url => { chrome.runtime.onMessage.addListener(msg => { - if (msg.method == 'updatePopup') { - switch (msg.reason) { - case 'styleAdded': - case 'styleUpdated': - handleUpdate(msg.style); - break; - case 'styleDeleted': - handleDelete(msg.id); - break; - } + switch (msg.method) { + case 'styleAdded': + case 'styleUpdated': + handleUpdate(msg.style); + break; + case 'styleDeleted': + handleDelete(msg.id); + break; + case 'prefChanged': + if ('popup.stylesFirst' in msg.prefs) { + const stylesFirst = msg.prefs['popup.stylesFirst']; + const actions = $('body > .actions'); + const before = stylesFirst ? actions : actions.nextSibling; + document.body.insertBefore(installed, before); + } + break; } }); @@ -53,7 +59,6 @@ function initPopup(url) { $('#popup-options-button').onclick = () => chrome.runtime.openOptionsPage(); $('#popup-shortcuts-button').onclick = configureCommands.open; - // styles first? if (!prefs.get('popup.stylesFirst')) { document.body.insertBefore( $('body > .actions'), diff --git a/storage.js b/storage.js index 656fcbf0..a3688bfc 100644 --- a/storage.js +++ b/storage.js @@ -497,7 +497,7 @@ function getApplicableSections({style, matchUrl, strictRegexp = true, stopOnFirs // and as Map in case the string is not the same reference used to add the item //const t0start = performance.now(); const code = section.code; - let isEmpty = code.length < 1000 && cachedStyles.emptyCode.get(code); + let isEmpty = code !== null && code.length < 1000 && cachedStyles.emptyCode.get(code); if (isEmpty === undefined) { isEmpty = !code || !code.trim() || code.indexOf('@namespace') >= 0 @@ -540,9 +540,18 @@ function tryRegExp(regexp) { } -prefs = prefs || new function Prefs() { - const me = this; +function debounce(fn, ...args) { + const timers = debounce.timers = debounce.timers || new Map(); + debounce.run = debounce.run || ((fn, ...args) => { + timers.delete(fn); + fn(...args); + }); + clearTimeout(timers.get(fn)); + timers.set(fn, setTimeout(debounce.run, 0, fn, ...args)); +} + +prefs = prefs || new function Prefs() { const defaults = { 'openEditInWindow': false, // new editor opens in a own browser window 'windowPosition': {}, // detached window position @@ -589,85 +598,85 @@ prefs = prefs || new function Prefs() { }; const values = deepCopy(defaults); - let syncTimeout; // see broadcast() function below + // coalesce multiple pref changes in broadcast + let broadcastPrefs = {}; + + function doBroadcast() { + notifyAllTabs({method: 'prefChanged', prefs: broadcastPrefs}); + broadcastPrefs = {}; + } + + function doSyncSet() { + getSync().set({'settings': values}); + } Object.defineProperty(this, 'readOnlyValues', {value: {}}); - Prefs.prototype.get = function(key, defaultValue) { - if (key in values) { - return values[key]; - } - if (defaultValue !== undefined) { - return defaultValue; - } - if (key in defaults) { - return defaults[key]; - } - console.warn("No default preference for '%s'", key); - }; + Object.assign(Prefs.prototype, { - Prefs.prototype.getAll = function() { - return deepCopy(values); - }; + get(key, defaultValue) { + if (key in values) { + return values[key]; + } + if (defaultValue !== undefined) { + return defaultValue; + } + if (key in defaults) { + return defaults[key]; + } + console.warn("No default preference for '%s'", key); + }, - Prefs.prototype.set = function(key, value, options) { - const oldValue = deepCopy(values[key]); - values[key] = value; - defineReadonlyProperty(this.readOnlyValues, key, value); - if ((!options || !options.noBroadcast) && !equal(value, oldValue)) { - me.broadcast(key, value, options); - } - }; + getAll() { + return deepCopy(values); + }, - Prefs.prototype.remove = key => me.set(key, undefined); + set(key, value, {noBroadcast, noSync} = {}) { + const oldValue = deepCopy(values[key]); + values[key] = value; + defineReadonlyProperty(this.readOnlyValues, key, value); + if (!noBroadcast && !equal(value, oldValue)) { + this.broadcast(key, value, {noSync}); + } + }, - Prefs.prototype.broadcast = function(key, value, options) { - const message = {method: 'prefChanged', prefName: key, value: value}; - notifyAllTabs(message); - chrome.runtime.sendMessage(message); - if (key == 'disableAll') { - notifyAllTabs({method: 'styleDisableAll', disableAll: value}); - } - if (!options || !options.noSync) { - clearTimeout(syncTimeout); - syncTimeout = setTimeout(function() { - getSync().set({'settings': values}); - }, 0); - } - }; + remove: key => this.set(key, undefined), - Object.keys(defaults).forEach(function(key) { - me.set(key, defaults[key], {noBroadcast: true}); + broadcast(key, value, {noSync} = {}) { + broadcastPrefs[key] = value; + debounce(doBroadcast); + if (!noSync) { + debounce(doSyncSet); + } + }, }); - getSync().get('settings', function(result) { - const synced = result.settings; + Object.keys(defaults).forEach(key => { + this.set(key, defaults[key], {noBroadcast: true}); + }); + + getSync().get('settings', ({settings: synced}) => { for (const key in defaults) { if (synced && (key in synced)) { - me.set(key, synced[key], {noSync: true}); - } else { - const value = tryMigrating(key); - if (value !== undefined) { - me.set(key, value); - } + this.set(key, synced[key], {noSync: true}); } } if (typeof contextMenus !== 'undefined') { for (const id in contextMenus) { if (typeof values[id] == 'boolean') { - me.broadcast(id, values[id], {noSync: true}); + this.broadcast(id, values[id], {noSync: true}); } } } }); - chrome.storage.onChanged.addListener(function(changes, area) { + chrome.storage.onChanged.addListener((changes, area) => { if (area == 'sync' && 'settings' in changes) { const synced = changes.settings.newValue; if (synced) { for (const key in defaults) { if (key in synced) { - me.set(key, synced[key], {noSync: true}); + this.set(key, synced[key], {noSync: true}); } } } else { @@ -676,29 +685,6 @@ prefs = prefs || new function Prefs() { } } }); - - function tryMigrating(key) { - if (!(key in localStorage)) { - return undefined; - } - const value = localStorage[key]; - delete localStorage[key]; - localStorage['DEPRECATED: ' + key] = value; - switch (typeof defaults[key]) { - case 'boolean': - return value.toLowerCase() === 'true'; - case 'number': - return Number(value); - case 'object': - try { - return JSON.parse(value); - } catch (e) { - console.log("Cannot migrate from localStorage %s = '%s': %o", key, value, e); - return undefined; - } - } - return value; - } }(); @@ -712,14 +698,18 @@ function setupLivePrefs(IDs) { prefs.set(this.id, isCheckbox(this) ? this.checked : this.value); }); }); - chrome.runtime.onMessage.addListener(function(request) { - if (request.prefName in localIDs) { - updateElement(request.prefName); + chrome.runtime.onMessage.addListener(msg => { + if (msg.prefs) { + for (const prefName in msg.prefs) { + if (prefName in localIDs) { + updateElement(prefName, msg.prefs[prefName]); + } + } } }); - function updateElement(id) { + function updateElement(id, value) { const el = document.getElementById(id); - el[isCheckbox(el) ? 'checked' : 'value'] = prefs.get(id); + el[isCheckbox(el) ? 'checked' : 'value'] = value || prefs.get(id); el.dispatchEvent(new Event('change', {bubbles: true, cancelable: true})); return el; } @@ -818,6 +808,7 @@ function defineReadonlyProperty(obj, key, value) { Object.defineProperty(obj, key, {value: copy, configurable: true}); } + // Polyfill for Firefox < 53 https://bugzilla.mozilla.org/show_bug.cgi?id=1220494 function getSync() { if ('sync' in chrome.storage) { diff --git a/update.js b/update.js index d74290ab..bc85d283 100644 --- a/update.js +++ b/update.js @@ -106,7 +106,7 @@ window.setTimeout(function () { } chrome.runtime.onMessage.addListener(request => { // when user has changed the predefined time interval in the settings page - if (request.method === 'prefChanged' && request.prefName === 'updateInterval') { + if (request.method === 'prefChanged' && 'updateInterval' in request.prefs) { reset(); } // when user just manually checked for updates