From b4c569d8b8064b3f355a7d1b72386fdda9a0c2e8 Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Mon, 8 Aug 2016 10:41:38 +1200 Subject: [PATCH 1/8] Bug 1293101 - Refactor CDMCaps.cpp to treat keys marked as output-restricted as usable in all cases. r=gerald This is a follow up from bug 1289623, as I missed a few cases in that bug. MozReview-Commit-ID: DM7FtVSZUo3 --HG-- extra : rebase_source : 4fd7ae93b1418ded54f0c21a50a654fc962d4892 --- dom/media/eme/CDMCaps.cpp | 52 ++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/dom/media/eme/CDMCaps.cpp b/dom/media/eme/CDMCaps.cpp index f10c80121e89..474ae70fc466 100644 --- a/dom/media/eme/CDMCaps.cpp +++ b/dom/media/eme/CDMCaps.cpp @@ -43,19 +43,23 @@ CDMCaps::AutoLock::~AutoLock() mData.Unlock(); } +// Keys with kGMPUsable, kGMPOutputDownscaled, or kGMPOutputRestricted status +// can be used by the CDM to decrypt or decrypt-and-decode samples. +static bool +IsUsableStatus(GMPMediaKeyStatus aStatus) +{ + return aStatus == kGMPUsable || + aStatus == kGMPOutputRestricted || + aStatus == kGMPOutputDownscaled; +} + bool CDMCaps::AutoLock::IsKeyUsable(const CencKeyId& aKeyId) { mData.mMonitor.AssertCurrentThreadOwns(); - const auto& keys = mData.mKeyStatuses; - for (size_t i = 0; i < keys.Length(); i++) { - if (keys[i].mId != aKeyId) { - continue; - } - if (keys[i].mStatus == kGMPUsable || - keys[i].mStatus == kGMPOutputRestricted || - keys[i].mStatus == kGMPOutputDownscaled) { - return true; + for (const KeyStatus& keyStatus : mData.mKeyStatuses) { + if (keyStatus.mId == aKeyId) { + return IsUsableStatus(keyStatus.mStatus); } } return false; @@ -68,31 +72,34 @@ CDMCaps::AutoLock::SetKeyStatus(const CencKeyId& aKeyId, { mData.mMonitor.AssertCurrentThreadOwns(); KeyStatus key(aKeyId, aSessionId, aStatus); - auto index = mData.mKeyStatuses.IndexOf(key); if (aStatus == kGMPUnknown) { // Return true if the element is found to notify key changes. return mData.mKeyStatuses.RemoveElement(key); } + auto index = mData.mKeyStatuses.IndexOf(key); if (index != mData.mKeyStatuses.NoIndex) { if (mData.mKeyStatuses[index].mStatus == aStatus) { + // No change. return false; } auto oldStatus = mData.mKeyStatuses[index].mStatus; mData.mKeyStatuses[index].mStatus = aStatus; - if (oldStatus == kGMPUsable || oldStatus == kGMPOutputDownscaled) { + // The old key status was one for which we can decrypt media. We don't + // need to do the "notify usable" step below, as it should be impossible + // for us to have anything waiting on this key to become usable, since it + // was already usable. + if (IsUsableStatus(oldStatus)) { return true; } } else { mData.mKeyStatuses.AppendElement(key); } - // Both kGMPUsable and kGMPOutputDownscaled are treated able to decrypt. - // We don't need to notify when transition happens between kGMPUsable and - // kGMPOutputDownscaled. Only call NotifyUsable() when we are going from - // ![kGMPUsable|kGMPOutputDownscaled] to [kGMPUsable|kGMPOutputDownscaled] - if (aStatus != kGMPUsable && aStatus != kGMPOutputDownscaled) { + // Only call NotifyUsable() for a key when we are going from non-usable + // to usable state. + if (!IsUsableStatus(aStatus)) { return true; } @@ -124,10 +131,9 @@ void CDMCaps::AutoLock::GetKeyStatusesForSession(const nsAString& aSessionId, nsTArray& aOutKeyStatuses) { - for (size_t i = 0; i < mData.mKeyStatuses.Length(); i++) { - const auto& key = mData.mKeyStatuses[i]; - if (key.mSessionId.Equals(aSessionId)) { - aOutKeyStatuses.AppendElement(key); + for (const KeyStatus& keyStatus : mData.mKeyStatuses) { + if (keyStatus.mSessionId.Equals(aSessionId)) { + aOutKeyStatuses.AppendElement(keyStatus); } } } @@ -136,7 +142,7 @@ void CDMCaps::AutoLock::GetSessionIdsForKeyId(const CencKeyId& aKeyId, nsTArray& aOutSessionIds) { - for (const auto& keyStatus : mData.mKeyStatuses) { + for (const KeyStatus& keyStatus : mData.mKeyStatuses) { if (keyStatus.mId == aKeyId) { aOutSessionIds.AppendElement(NS_ConvertUTF16toUTF8(keyStatus.mSessionId)); } @@ -149,8 +155,8 @@ CDMCaps::AutoLock::RemoveKeysForSession(const nsString& aSessionId) bool changed = false; nsTArray statuses; GetKeyStatusesForSession(aSessionId, statuses); - for (const KeyStatus& status : statuses) { - changed |= SetKeyStatus(status.mId, aSessionId, kGMPUnknown); + for (const KeyStatus& keyStatus : statuses) { + changed |= SetKeyStatus(keyStatus.mId, aSessionId, kGMPUnknown); } return changed; } From 956647e2c9776385b1578b8b79e52cbd6677566f Mon Sep 17 00:00:00 2001 From: Andrew Swan Date: Fri, 22 Jul 2016 10:16:55 -0700 Subject: [PATCH 2/8] Bug 1213990 Do immediate uninstall for test webextensions r=kmag MozReview-Commit-ID: 9vcAfPhRYFi --HG-- extra : rebase_source : c10b7e927eb0d0e12f70d9e06f77d1b56fa405fc --- toolkit/components/extensions/Extension.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 147ca1f3b897..0d60c15b1e98 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -1173,7 +1173,7 @@ class MockExtension { } shutdown() { - this.addon.uninstall(true); + this.addon.uninstall(); return this.cleanupGeneratedFile(); } From d7af10177b64ad8290df6f0d72a01791db535d8d Mon Sep 17 00:00:00 2001 From: Andrew Swan Date: Sun, 7 Aug 2016 09:53:36 -0700 Subject: [PATCH 3/8] Bug 1213990 Convert getExtensionUUID to UUIDMap r=kmag MozReview-Commit-ID: 9VVNa0pjx8g --HG-- extra : rebase_source : d62e8289c127afe762a6af71fecb4e15c08272ca --- toolkit/components/extensions/Extension.jsm | 89 +++++++++++++-------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 0d60c15b1e98..0cd829a9d492 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -88,6 +88,7 @@ var { } = ExtensionUtils; const LOGGER_ID_BASE = "addons.webextension."; +const UUID_MAP_PREF = "extensions.webextensions.uuids"; const COMMENT_REGEXP = new RegExp(String.raw` ^ @@ -475,6 +476,60 @@ let ParentAPIManager = { ParentAPIManager.init(); +// All moz-extension URIs use a machine-specific UUID rather than the +// extension's own ID in the host component. This makes it more +// difficult for web pages to detect whether a user has a given add-on +// installed (by trying to load a moz-extension URI referring to a +// web_accessible_resource from the extension). UUIDMap.get() +// returns the UUID for a given add-on ID. +var UUIDMap = { + _read() { + let pref = Preferences.get(UUID_MAP_PREF, "{}"); + try { + return JSON.parse(pref); + } catch (e) { + Cu.reportError(`Error parsing ${UUID_MAP_PREF}.`); + return {}; + } + }, + + _write(map) { + Preferences.set(UUID_MAP_PREF, JSON.stringify(map)); + }, + + get(id, create = true) { + let map = this._read(); + + if (id in map) { + return map[id]; + } + + let uuid = null; + if (create) { + let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator); + uuid = uuidGenerator.generateUUID().number; + uuid = uuid.slice(1, -1); // Strip { and } off the UUID. + + map[id] = uuid; + this._write(map); + } + return uuid; + }, + + remove(id) { + let map = this._read(); + delete map[id]; + this._write(map); + }, +}; + +// This is the old interface that UUIDMap replaced, to be removed when +// the references listed in bug 1291399 are updated. +/* exported getExtensionUUID */ +function getExtensionUUID(id) { + return UUIDMap.get(id, true); +} + // For extensions that have called setUninstallURL(), send an event // so the browser can display the URL. var UninstallObserver = { @@ -708,36 +763,6 @@ GlobalManager = { }, }; -// All moz-extension URIs use a machine-specific UUID rather than the -// extension's own ID in the host component. This makes it more -// difficult for web pages to detect whether a user has a given add-on -// installed (by trying to load a moz-extension URI referring to a -// web_accessible_resource from the extension). getExtensionUUID -// returns the UUID for a given add-on ID. -function getExtensionUUID(id) { - const PREF_NAME = "extensions.webextensions.uuids"; - - let pref = Preferences.get(PREF_NAME, "{}"); - let map = {}; - try { - map = JSON.parse(pref); - } catch (e) { - Cu.reportError(`Error parsing ${PREF_NAME}.`); - } - - if (id in map) { - return map[id]; - } - - let uuidGenerator = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator); - let uuid = uuidGenerator.generateUUID().number; - uuid = uuid.slice(1, -1); // Strip { and } off the UUID. - - map[id] = uuid; - Preferences.set(PREF_NAME, JSON.stringify(map)); - return uuid; -} - // Represents the data contained in an extension, contained either // in a directory or a zip file, which may or may not be installed. // This class implements the functionality of the Extension class, @@ -798,7 +823,7 @@ this.ExtensionData = class { throw new Error("getURL may not be called before an `id` or `uuid` has been set"); } if (!this.uuid) { - this.uuid = getExtensionUUID(this.id); + this.uuid = UUIDMap.get(this.id); } return `moz-extension://${this.uuid}/${path}`; } @@ -1189,7 +1214,7 @@ this.Extension = class extends ExtensionData { constructor(addonData) { super(addonData.resourceURI); - this.uuid = getExtensionUUID(addonData.id); + this.uuid = UUIDMap.get(addonData.id); if (addonData.cleanupFile) { Services.obs.addObserver(this, "xpcom-shutdown", false); From fa3c0fac00a8be2ebffacfaaf09247d054c820ce Mon Sep 17 00:00:00 2001 From: Andrew Swan Date: Mon, 1 Aug 2016 16:30:18 -0700 Subject: [PATCH 4/8] Bug 1213990 Clear storage when webextension is uninstalled r=kmag MozReview-Commit-ID: BeMOxOCSeru --HG-- extra : rebase_source : 3eb7fc4e61fe1ce698e22e25bd05fc779a6cf7a8 --- modules/libpref/init/all.js | 4 + toolkit/components/extensions/Extension.jsm | 48 ++++-- .../extensions/test/mochitest/chrome.ini | 1 + .../test_chrome_ext_shutdown_cleanup.html | 11 +- .../test_chrome_ext_storage_cleanup.html | 161 ++++++++++++++++++ 5 files changed, 205 insertions(+), 20 deletions(-) create mode 100644 toolkit/components/extensions/test/mochitest/test_chrome_ext_storage_cleanup.html diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 2195d50f1cc6..fb02d3b4da51 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4639,6 +4639,10 @@ pref("extensions.alwaysUnpack", false); pref("extensions.minCompatiblePlatformVersion", "2.0"); pref("extensions.webExtensionsMinPlatformVersion", "42.0a1"); +// Other webextensions prefs +pref("extensions.webextensions.keepStorageOnUninstall", false); +pref("extensions.webextensions.keepUuidOnUninstall", false); + pref("network.buffer.cache.count", 24); pref("network.buffer.cache.size", 32768); diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 0cd829a9d492..af3ba30f2bb0 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -32,6 +32,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "AppConstants", "resource://gre/modules/AppConstants.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "ExtensionAPIs", "resource://gre/modules/ExtensionAPI.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage", + "resource://gre/modules/ExtensionStorage.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "FileUtils", "resource://gre/modules/FileUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "Locale", @@ -89,6 +91,8 @@ var { const LOGGER_ID_BASE = "addons.webextension."; const UUID_MAP_PREF = "extensions.webextensions.uuids"; +const LEAVE_STORAGE_PREF = "extensions.webextensions.keepStorageOnUninstall"; +const LEAVE_UUID_PREF = "extensions.webextensions.keepUuidOnUninstall"; const COMMENT_REGEXP = new RegExp(String.raw` ^ @@ -535,24 +539,49 @@ function getExtensionUUID(id) { var UninstallObserver = { initialized: false, - init: function() { + init() { if (!this.initialized) { AddonManager.addAddonListener(this); + XPCOMUtils.defineLazyPreferenceGetter(this, "leaveStorage", LEAVE_STORAGE_PREF, false); + XPCOMUtils.defineLazyPreferenceGetter(this, "leaveUuid", LEAVE_UUID_PREF, false); this.initialized = true; } }, - uninit: function() { - if (this.initialized) { - AddonManager.removeAddonListener(this); - this.initialized = false; + onUninstalling(addon) { + let extension = GlobalManager.extensionMap.get(addon.id); + if (extension) { + // Let any other interested listeners respond + // (e.g., display the uninstall URL) + Management.emit("uninstall", extension); } }, - onUninstalling: function(addon) { - let extension = GlobalManager.extensionMap.get(addon.id); - if (extension) { - Management.emit("uninstall", extension); + onUninstalled(addon) { + let uuid = UUIDMap.get(addon.id, false); + if (!uuid) { + return; + } + + if (!this.leaveStorage) { + // Clear browser.local.storage + ExtensionStorage.clear(addon.id); + + // Clear any IndexedDB storage created by the extension + let baseURI = NetUtil.newURI(`moz-extension://${uuid}/`); + let principal = Services.scriptSecurityManager.createCodebasePrincipal( + baseURI, {addonId: addon.id} + ); + Services.qms.clearStoragesForPrincipal(principal); + + // Clear localStorage created by the extension + let attrs = JSON.stringify({addonId: addon.id}); + Services.obs.notifyObservers(null, "clear-origin-data", attrs); + } + + if (!this.leaveUuid) { + // Clear the entry in the UUID map + UUIDMap.remove(addon.id); } }, }; @@ -579,7 +608,6 @@ GlobalManager = { if (this.extensionMap.size == 0 && this.initialized) { Services.obs.removeObserver(this, "content-document-global-created"); - UninstallObserver.uninit(); this.initialized = false; } }, diff --git a/toolkit/components/extensions/test/mochitest/chrome.ini b/toolkit/components/extensions/test/mochitest/chrome.ini index 2a45b61c9d69..c8a1cc36a47a 100644 --- a/toolkit/components/extensions/test/mochitest/chrome.ini +++ b/toolkit/components/extensions/test/mochitest/chrome.ini @@ -24,3 +24,4 @@ skip-if = buildapp == 'b2g' [test_ext_jsversion.html] skip-if = buildapp == 'b2g' [test_ext_schema.html] +[test_chrome_ext_storage_cleanup.html] diff --git a/toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html b/toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html index 590c60af50bb..48e973f4e299 100644 --- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html +++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_shutdown_cleanup.html @@ -18,18 +18,13 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components; Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://testing-common/TestUtils.jsm"); -const { - GlobalManager, - UninstallObserver, -} = Cu.import("resource://gre/modules/Extension.jsm"); +const {GlobalManager} = Cu.import("resource://gre/modules/Extension.jsm"); /* eslint-disable mozilla/balanced-listeners */ add_task(function* testShutdownCleanup() { is(GlobalManager.initialized, false, "GlobalManager start as not initialized"); - is(UninstallObserver.initialized, false, - "UninstallObserver start as not initialized"); let extension = ExtensionTestUtils.loadExtension({ background: "new " + function() { @@ -43,15 +38,11 @@ add_task(function* testShutdownCleanup() { is(GlobalManager.initialized, true, "GlobalManager has been initialized once an extension is started"); - is(UninstallObserver.initialized, true, - "UninstallObserver has been initialized once an extension is started"); yield extension.unload(); is(GlobalManager.initialized, false, "GlobalManager has been uninitialized once all the webextensions have been stopped"); - is(UninstallObserver.initialized, false, - "UninstallObserver has been uninitialized once all the webextensions have been stopped"); }); diff --git a/toolkit/components/extensions/test/mochitest/test_chrome_ext_storage_cleanup.html b/toolkit/components/extensions/test/mochitest/test_chrome_ext_storage_cleanup.html new file mode 100644 index 000000000000..da025f028b7e --- /dev/null +++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_storage_cleanup.html @@ -0,0 +1,161 @@ + + + + WebExtension test + + + + + + + + + + + + From 6168f0df5bb5b7801d5e1f68b29e61f34523508d Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 4 Aug 2016 11:17:41 +0900 Subject: [PATCH 5/8] Bug 1282256 - Avoid loading mozconfig in MozbuildObject.from_environment. r=gps We've been reading the mozconfig in MozbuildObject.from_environment to check whether the mozconfig topobjdir matches the detected topobjdir. Since bug 1278415, everything using the buildconfig python module now calls MozbuildObject.from_environment, which reads the mozconfig. A lot of things to that during the build. But none of them actually need the data from the mozconfig, and the topobjdir match test has been breaking things randomly on multiple occasions. The topobjdir match test, however, really only needs to happen once: when a mach command starts. So we can move the test to MachCommandBase, where it belongs, and anything actively using MozbuildObject.mozconfig will have the mozconfig read, but everything else won't. On a Linux64 opt build, this brings down the number of times the mozconfig is read during `mach build` from 979 to 9. --HG-- extra : rebase_source : 6b340f1fcf73a3c3987033c37f8f14ef06a44f04 --- python/mozbuild/mozbuild/base.py | 42 ++++++++-------------- python/mozbuild/mozbuild/test/test_base.py | 23 ++++++++++-- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/python/mozbuild/mozbuild/base.py b/python/mozbuild/mozbuild/base.py index e09abe5ce887..c991626acefb 100644 --- a/python/mozbuild/mozbuild/base.py +++ b/python/mozbuild/mozbuild/base.py @@ -159,32 +159,7 @@ class MozbuildObject(ProcessExecutionMixin): raise BuildEnvironmentNotFoundException( 'Could not find Mozilla source tree or build environment.') - # Now we try to load the config for this environment. If mozconfig is - # None, read_mozconfig() will attempt to find one in the existing - # environment. If no mozconfig is present, the config will not have - # much defined. - loader = MozconfigLoader(topsrcdir) - current_project = os.environ.get('MOZ_CURRENT_PROJECT') - config = loader.read_mozconfig(mozconfig, moz_build_app=current_project) - - config_topobjdir = MozbuildObject.resolve_mozconfig_topobjdir( - topsrcdir, config) - - # If we're inside a objdir and the found mozconfig resolves to - # another objdir, we abort. The reasoning here is that if you are - # inside an objdir you probably want to perform actions on that objdir, - # not another one. This prevents accidental usage of the wrong objdir - # when the current objdir is ambiguous. - if topobjdir and config_topobjdir: - if current_project: - config_topobjdir = os.path.join(config_topobjdir, current_project) - - _config_topobjdir = config_topobjdir - if not samepath(topobjdir, _config_topobjdir): - raise ObjdirMismatchException(topobjdir, _config_topobjdir) - topsrcdir = mozpath.normsep(topsrcdir) - topobjdir = topobjdir or config_topobjdir if topobjdir: topobjdir = mozpath.normsep(os.path.normpath(topobjdir)) @@ -193,8 +168,8 @@ class MozbuildObject(ProcessExecutionMixin): 'to be the same as your source directory (%s). This build ' 'configuration is not supported.' % topsrcdir) - # If we can't resolve topobjdir, oh well. The constructor will figure - # it out via config.guess. + # If we can't resolve topobjdir, oh well. We'll figure out when we need + # one. return cls(topsrcdir, None, None, topobjdir=topobjdir, mozconfig=mozconfig) @@ -237,7 +212,7 @@ class MozbuildObject(ProcessExecutionMixin): This a dict as returned by MozconfigLoader.read_mozconfig() """ - if self._mozconfig is MozconfigLoader.AUTODETECT: + if not isinstance(self._mozconfig, dict): loader = MozconfigLoader(self.topsrcdir) self._mozconfig = loader.read_mozconfig(path=self._mozconfig, moz_build_app=os.environ.get('MOZ_CURRENT_PROJECT')) @@ -699,6 +674,17 @@ class MachCommandBase(MozbuildObject): detect_virtualenv_mozinfo=detect_virtualenv_mozinfo) topsrcdir = dummy.topsrcdir topobjdir = dummy._topobjdir + if topobjdir: + # If we're inside a objdir and the found mozconfig resolves to + # another objdir, we abort. The reasoning here is that if you + # are inside an objdir you probably want to perform actions on + # that objdir, not another one. This prevents accidental usage + # of the wrong objdir when the current objdir is ambiguous. + config_topobjdir = MozbuildObject.resolve_mozconfig_topobjdir( + topsrcdir, dummy.mozconfig) + if config_topobjdir and not samepath(topobjdir, + config_topobjdir): + raise ObjdirMismatchException(topobjdir, config_topobjdir) except BuildEnvironmentNotFoundException: pass except ObjdirMismatchException as e: diff --git a/python/mozbuild/mozbuild/test/test_base.py b/python/mozbuild/mozbuild/test/test_base.py index ffcc26c2dd9a..9c4a1c623e0a 100644 --- a/python/mozbuild/mozbuild/test/test_base.py +++ b/python/mozbuild/mozbuild/test/test_base.py @@ -12,6 +12,7 @@ import sys import tempfile import unittest +from cStringIO import StringIO from mozfile.mozfile import NamedTemporaryFile from mozunit import main @@ -272,8 +273,26 @@ class TestMozbuildObject(unittest.TestCase): os.chdir(topobjdir) - with self.assertRaises(ObjdirMismatchException): - MozbuildObject.from_environment(detect_virtualenv_mozinfo=False) + class MockMachContext(object): + pass + + context = MockMachContext() + context.cwd = topobjdir + context.topdir = topsrcdir + context.settings = None + context.log_manager = None + context.detect_virtualenv_mozinfo=False + + stdout = sys.stdout + sys.stdout = StringIO() + try: + with self.assertRaises(SystemExit): + MachCommandBase(context) + + self.assertTrue(sys.stdout.getvalue().startswith( + 'Ambiguous object directory detected.')) + finally: + sys.stdout = stdout finally: os.chdir(self._old_cwd) From ad5e3fa4c45148d830f1487f2a4ffcf92147eaeb Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 4 Aug 2016 10:07:56 +0900 Subject: [PATCH 6/8] Bug 1282256 - Remove MozbuildObject._config_guess. r=gps Back when it was added, it was used, but it is not anymore, outside test_base.py. --HG-- extra : rebase_source : f0b9a4dab2985e89e9950eda774ae853c7de764c --- python/mozbuild/mozbuild/base.py | 9 --------- python/mozbuild/mozbuild/test/test_base.py | 13 +++---------- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/python/mozbuild/mozbuild/base.py b/python/mozbuild/mozbuild/base.py index c991626acefb..5eaebdca07ba 100644 --- a/python/mozbuild/mozbuild/base.py +++ b/python/mozbuild/mozbuild/base.py @@ -90,7 +90,6 @@ class MozbuildObject(ProcessExecutionMixin): self._make = None self._topobjdir = mozpath.normsep(topobjdir) if topobjdir else topobjdir self._mozconfig = mozconfig - self._config_guess_output = None self._config_environment = None self._virtualenv_manager = None @@ -443,14 +442,6 @@ class MozbuildObject(ProcessExecutionMixin): self.log(logging.WARNING, 'notifier-failed', {'error': e.message}, 'Notification center failed: {error}') - @property - def _config_guess(self): - if self._config_guess_output is None: - self._config_guess_output = MozbuildObject.resolve_config_guess( - self.mozconfig, self.topsrcdir) - - return self._config_guess_output - def _ensure_objdir_exists(self): if os.path.isdir(self.statedir): return diff --git a/python/mozbuild/mozbuild/test/test_base.py b/python/mozbuild/mozbuild/test/test_base.py index 9c4a1c623e0a..590b679d4c25 100644 --- a/python/mozbuild/mozbuild/test/test_base.py +++ b/python/mozbuild/mozbuild/test/test_base.py @@ -60,7 +60,9 @@ class TestMozbuildObject(unittest.TestCase): self.assertIsNotNone(base.topobjdir) self.assertEqual(len(base.topobjdir.split()), 1) - self.assertTrue(base.topobjdir.endswith(base._config_guess)) + config_guess = MozbuildObject.resolve_config_guess(base.mozconfig, + base.topsrcdir) + self.assertTrue(base.topobjdir.endswith(config_guess)) self.assertTrue(os.path.isabs(base.topobjdir)) self.assertTrue(base.topobjdir.startswith(base.topsrcdir)) @@ -298,15 +300,6 @@ class TestMozbuildObject(unittest.TestCase): os.chdir(self._old_cwd) shutil.rmtree(d) - def test_config_guess(self): - # It's difficult to test for exact values from the output of - # config.guess because they vary depending on platform. - base = self.get_base() - result = base._config_guess - - self.assertIsNotNone(result) - self.assertGreater(len(result), 0) - def test_config_environment(self): base = self.get_base(topobjdir=topobjdir) From e6fb85ebb39e9f1f4e86607a632c7369da65c118 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 4 Aug 2016 13:41:57 +0900 Subject: [PATCH 7/8] Bug 1282256 - Make MozbuildObject.resolve_mozconfig_topobjdir an instance method. r=gps The only use that didn't have an existing instance was just removed. --HG-- extra : rebase_source : 47891a9bd5c6ecef31d8a2c7053c1ae8efe36fa5 --- python/mozbuild/mozbuild/base.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/python/mozbuild/mozbuild/base.py b/python/mozbuild/mozbuild/base.py index 5eaebdca07ba..c8295cce133a 100644 --- a/python/mozbuild/mozbuild/base.py +++ b/python/mozbuild/mozbuild/base.py @@ -172,26 +172,26 @@ class MozbuildObject(ProcessExecutionMixin): return cls(topsrcdir, None, None, topobjdir=topobjdir, mozconfig=mozconfig) - @staticmethod - def resolve_mozconfig_topobjdir(topsrcdir, mozconfig, default=None): - topobjdir = mozconfig['topobjdir'] or default + def resolve_mozconfig_topobjdir(self, default=None): + topobjdir = self.mozconfig['topobjdir'] or default if not topobjdir: return None if '@CONFIG_GUESS@' in topobjdir: topobjdir = topobjdir.replace('@CONFIG_GUESS@', - MozbuildObject.resolve_config_guess(mozconfig, topsrcdir)) + MozbuildObject.resolve_config_guess(self.mozconfig, + self.topsrcdir)) if not os.path.isabs(topobjdir): - topobjdir = os.path.abspath(os.path.join(topsrcdir, topobjdir)) + topobjdir = os.path.abspath(os.path.join(self.topsrcdir, topobjdir)) return mozpath.normsep(os.path.normpath(topobjdir)) @property def topobjdir(self): if self._topobjdir is None: - self._topobjdir = MozbuildObject.resolve_mozconfig_topobjdir( - self.topsrcdir, self.mozconfig, default='obj-@CONFIG_GUESS@') + self._topobjdir = self.resolve_mozconfig_topobjdir( + default='obj-@CONFIG_GUESS@') return self._topobjdir @@ -671,8 +671,7 @@ class MachCommandBase(MozbuildObject): # are inside an objdir you probably want to perform actions on # that objdir, not another one. This prevents accidental usage # of the wrong objdir when the current objdir is ambiguous. - config_topobjdir = MozbuildObject.resolve_mozconfig_topobjdir( - topsrcdir, dummy.mozconfig) + config_topobjdir = dummy.resolve_mozconfig_topobjdir() if config_topobjdir and not samepath(topobjdir, config_topobjdir): raise ObjdirMismatchException(topobjdir, config_topobjdir) From 50a4467ec35c4bad7c818fb9299ff90878e36a70 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 4 Aug 2016 13:43:47 +0900 Subject: [PATCH 8/8] Bug 1282256 - Make MozbuildObject.resolve_config_guess an instance method. r=gps The last use that didn't have an existing instance was just removed. --HG-- extra : rebase_source : 81f99e8a8d8046c9741e8a27072a6c4773fa7292 --- python/mozbuild/mozbuild/base.py | 14 ++++++-------- python/mozbuild/mozbuild/test/test_base.py | 3 +-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/python/mozbuild/mozbuild/base.py b/python/mozbuild/mozbuild/base.py index c8295cce133a..e030ece0301e 100644 --- a/python/mozbuild/mozbuild/base.py +++ b/python/mozbuild/mozbuild/base.py @@ -179,8 +179,7 @@ class MozbuildObject(ProcessExecutionMixin): if '@CONFIG_GUESS@' in topobjdir: topobjdir = topobjdir.replace('@CONFIG_GUESS@', - MozbuildObject.resolve_config_guess(self.mozconfig, - self.topsrcdir)) + self.resolve_config_guess()) if not os.path.isabs(topobjdir): topobjdir = os.path.abspath(os.path.join(self.topsrcdir, topobjdir)) @@ -353,9 +352,8 @@ class MozbuildObject(ProcessExecutionMixin): return path - @staticmethod - def resolve_config_guess(mozconfig, topsrcdir): - make_extra = mozconfig['make_extra'] or [] + def resolve_config_guess(self): + make_extra = self.mozconfig['make_extra'] or [] make_extra = dict(m.split('=', 1) for m in make_extra) config_guess = make_extra.get('CONFIG_GUESS', None) @@ -368,17 +366,17 @@ class MozbuildObject(ProcessExecutionMixin): if _config_guess_output: return _config_guess_output[0] - p = os.path.join(topsrcdir, 'build', 'autoconf', 'config.guess') + p = os.path.join(self.topsrcdir, 'build', 'autoconf', 'config.guess') # This is a little kludgy. We need access to the normalize_command # function. However, that's a method of a mach mixin, so we need a # class instance. Ideally the function should be accessible as a # standalone function. - o = MozbuildObject(topsrcdir, None, None, None) + o = MozbuildObject(self.topsrcdir, None, None, None) args = o._normalize_command([p], True) _config_guess_output.append( - subprocess.check_output(args, cwd=topsrcdir).strip()) + subprocess.check_output(args, cwd=self.topsrcdir).strip()) return _config_guess_output[0] def notify(self, msg): diff --git a/python/mozbuild/mozbuild/test/test_base.py b/python/mozbuild/mozbuild/test/test_base.py index 590b679d4c25..87f0db85bfba 100644 --- a/python/mozbuild/mozbuild/test/test_base.py +++ b/python/mozbuild/mozbuild/test/test_base.py @@ -60,8 +60,7 @@ class TestMozbuildObject(unittest.TestCase): self.assertIsNotNone(base.topobjdir) self.assertEqual(len(base.topobjdir.split()), 1) - config_guess = MozbuildObject.resolve_config_guess(base.mozconfig, - base.topsrcdir) + config_guess = base.resolve_config_guess() self.assertTrue(base.topobjdir.endswith(config_guess)) self.assertTrue(os.path.isabs(base.topobjdir)) self.assertTrue(base.topobjdir.startswith(base.topsrcdir))