From 2705432f43bad64bc753a45167eadc5477b4c282 Mon Sep 17 00:00:00 2001 From: gorhill Date: Wed, 2 Dec 2015 00:59:51 -0500 Subject: [PATCH] code review --- platform/firefox/vapi-background.js | 68 +++++++++++++---------------- src/js/messaging.js | 2 +- src/js/pagestore.js | 8 ++-- src/js/tab.js | 35 ++++++--------- src/js/traffic.js | 2 +- 5 files changed, 51 insertions(+), 64 deletions(-) diff --git a/platform/firefox/vapi-background.js b/platform/firefox/vapi-background.js index 896e0cdae..9d52f91b9 100644 --- a/platform/firefox/vapi-background.js +++ b/platform/firefox/vapi-background.js @@ -1923,47 +1923,41 @@ var httpObserver = { // Also: // https://developer.mozilla.org/en-US/Firefox/Multiprocess_Firefox/Limitations_of_chrome_scripts tabIdFromChannel: function(channel) { - var aWindow; - if ( channel.notificationCallbacks ) { - try { - var loadContext = channel - .notificationCallbacks - .getInterface(Ci.nsILoadContext); - if ( loadContext.topFrameElement ) { - return tabWatcher.tabIdFromTarget(loadContext.topFrameElement); - } - aWindow = loadContext.associatedWindow; - } catch (ex) { - //console.error(ex.toString()); - } + var ncbs = channel.notificationCallbacks; + if ( !ncbs && channel.loadGroup ) { + ncbs = channel.loadGroup.notificationCallbacks; } - var gBrowser; + if ( !ncbs ) { return vAPI.noTabId; } + var lc; try { - if ( !aWindow && channel.loadGroup && channel.loadGroup.notificationCallbacks ) { - aWindow = channel - .loadGroup - .notificationCallbacks - .getInterface(Ci.nsILoadContext) - .associatedWindow; - } - if ( aWindow ) { - gBrowser = aWindow - .getInterface(Ci.nsIWebNavigation) - .QueryInterface(Ci.nsIDocShell) - .rootTreeItem - .QueryInterface(Ci.nsIInterfaceRequestor) - .getInterface(Ci.nsIDOMWindow) - .gBrowser; - } - } catch (ex) { - //console.error(ex.toString()); + lc = ncbs.getInterface(Ci.nsILoadContext); + } catch (ex) { } + if ( !lc ) { return vAPI.noTabId; } + if ( lc.topFrameElement ) { + return tabWatcher.tabIdFromTarget(lc.topFrameElement); } - if ( !gBrowser || !gBrowser._getTabForContentWindow ) { - return vAPI.noTabId; + var win = lc.associatedWindow; + if ( !win ) { return vAPI.noTabId; } + if ( win.top ) { + win = win.top; } - // Using `_getTabForContentWindow` ensure older versions of Firefox - // work well. - return tabWatcher.tabIdFromTarget(gBrowser._getTabForContentWindow(aWindow)); + var tabBrowser; + try { + tabBrowser = getTabBrowser( + win.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIWebNavigation) + .QueryInterface(Ci.nsIDocShell).rootTreeItem + .QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindow) + ); + } catch (ex) { } + if ( !tabBrowser ) { return vAPI.noTabId; } + if ( tabBrowser.getBrowserForContentWindow ) { + return tabWatcher.tabIdFromTarget(tabBrowser.getBrowserForContentWindow(win)); + } + // Falling back onto _getTabForContentWindow to ensure older versions + // of Firefox work well. + return tabBrowser._getTabForContentWindow ? + tabWatcher.tabIdFromTarget(tabBrowser._getTabForContentWindow(win)) : + vAPI.noTabId; }, // https://github.com/gorhill/uBlock/issues/959 diff --git a/src/js/messaging.js b/src/js/messaging.js index 96b99a190..102a279cf 100644 --- a/src/js/messaging.js +++ b/src/js/messaging.js @@ -276,7 +276,7 @@ var getFirewallRules = function(srcHostname, desHostnames) { /******************************************************************************/ var popupDataFromTabId = function(tabId, tabTitle) { - var tabContext = µb.tabContextManager.lookup(tabId); + var tabContext = µb.tabContextManager.mustLookup(tabId); var r = { advancedUserEnabled: µb.userSettings.advancedUserEnabled, appName: vAPI.app.name, diff --git a/src/js/pagestore.js b/src/js/pagestore.js index abcd167ef..ed80b0298 100644 --- a/src/js/pagestore.js +++ b/src/js/pagestore.js @@ -292,7 +292,7 @@ PageStore.factory = function(tabId) { /******************************************************************************/ PageStore.prototype.init = function(tabId) { - var tabContext = µb.tabContextManager.lookup(tabId); + var tabContext = µb.tabContextManager.mustLookup(tabId); this.tabId = tabId; this.tabHostname = tabContext.rootHostname; this.title = tabContext.rawURL; @@ -335,7 +335,7 @@ PageStore.prototype.reuse = function(context) { // When force refreshing a page, the page store data needs to be reset. // If the hostname changes, we can't merely just update the context. - var tabContext = µb.tabContextManager.lookup(this.tabId); + var tabContext = µb.tabContextManager.mustLookup(this.tabId); if ( tabContext.rootHostname !== this.tabHostname ) { context = ''; } @@ -441,7 +441,7 @@ PageStore.prototype.createContextFromFrameHostname = function(frameHostname) { /******************************************************************************/ PageStore.prototype.getNetFilteringSwitch = function() { - return µb.tabContextManager.lookup(this.tabId).getNetFilteringSwitch(); + return µb.tabContextManager.mustLookup(this.tabId).getNetFilteringSwitch(); }; /******************************************************************************/ @@ -451,7 +451,7 @@ PageStore.prototype.getSpecificCosmeticFilteringSwitch = function() { return false; } - var tabContext = µb.tabContextManager.lookup(this.tabId); + var tabContext = µb.tabContextManager.mustLookup(this.tabId); if ( µb.hnSwitches.evaluateZ('no-cosmetic-filtering', tabContext.rootHostname) ) { return false; diff --git a/src/js/tab.js b/src/js/tab.js index 0a1194614..2ab5f9669 100644 --- a/src/js/tab.js +++ b/src/js/tab.js @@ -346,15 +346,15 @@ housekeep itself. return entry; }; + // Find a tab context for a specific tab. + var lookup = function(tabId) { + return tabContexts[tabId] || null; + }; + // Find a tab context for a specific tab. If none is found, attempt to // fix this. When all fail, the behind-the-scene context is returned. - var lookup = function(tabId, url) { - var entry; - if ( url !== undefined ) { - entry = push(tabId, url); - } else { - entry = tabContexts[tabId]; - } + var mustLookup = function(tabId) { + var entry = tabContexts[tabId]; if ( entry !== undefined ) { return entry; } @@ -426,6 +426,7 @@ housekeep itself. push: push, commit: commit, lookup: lookup, + mustLookup: mustLookup, exists: exists, createContext: createContext }; @@ -576,23 +577,15 @@ vAPI.tabs.onPopupUpdated = (function() { return function(targetTabId, openerTabId) { // Opener details. var tabContext = µb.tabContextManager.lookup(openerTabId); - var openerURL = ''; - if ( tabContext.tabId === openerTabId ) { - openerURL = tabContext.rawURL; - if ( openerURL === '' ) { - return; - } - } + if ( tabContext === null ) { return; } + var openerURL = tabContext.rawURL; + if ( openerURL === '' ) { return; } // Popup details. tabContext = µb.tabContextManager.lookup(targetTabId); - var targetURL = ''; - if ( tabContext.tabId === targetTabId ) { - targetURL = tabContext.rawURL; - if ( targetURL === '' ) { - return; - } - } + if ( tabContext === null ) { return; } + var targetURL = tabContext.rawURL; + if ( targetURL === '' ) { return; } // https://github.com/gorhill/uBlock/issues/341 // Allow popups if uBlock is turned off in opener's context. diff --git a/src/js/traffic.js b/src/js/traffic.js index bcd071b91..1335b23e5 100644 --- a/src/js/traffic.js +++ b/src/js/traffic.js @@ -60,7 +60,7 @@ var onBeforeRequest = function(details) { var µb = µBlock; var pageStore = µb.pageStoreFromTabId(tabId); if ( !pageStore ) { - var tabContext = µb.tabContextManager.lookup(tabId); + var tabContext = µb.tabContextManager.mustLookup(tabId); if ( vAPI.isBehindTheSceneTabId(tabContext.tabId) ) { return onBeforeBehindTheSceneRequest(details); }