From 950d7c12f4f917656e0bd4dd981778329ab6b823 Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Mon, 18 Feb 2013 13:05:07 -0800 Subject: [PATCH] Bug 842377 - Rename "constant only" to "pull only"; r=rnewman The new name better reflects the lazy-init behavior of providers. --- services/healthreport/healthreporter.jsm | 50 +++++++++---------- services/healthreport/profile.jsm | 2 +- services/healthreport/providers.jsm | 8 +-- .../tests/xpcshell/test_healthreporter.js | 9 ++-- services/metrics/dataprovider.jsm | 18 ++++--- services/metrics/modules-testing/mocks.jsm | 2 +- 6 files changed, 45 insertions(+), 44 deletions(-) diff --git a/services/healthreport/healthreporter.jsm b/services/healthreport/healthreporter.jsm index 90ed544d158d..58129ca98903 100644 --- a/services/healthreport/healthreporter.jsm +++ b/services/healthreport/healthreporter.jsm @@ -88,8 +88,8 @@ function AbstractHealthReporter(branch, policy, sessionRecorder) { this._shutdownComplete = false; this._shutdownCompleteCallback = null; - this._constantOnlyProviders = {}; - this._constantOnlyProvidersRegistered = false; + this._pullOnlyProviders = {}; + this._pullOnlyProvidersRegistered = false; this._lastDailyDate = null; // Yes, this will probably run concurrently with remaining constructor work. @@ -387,7 +387,7 @@ AbstractHealthReporter.prototype = Object.freeze({ * Obtain a provider from its name. * * This will only return providers that are currently initialized. If - * a provider is lazy initialized (like constant-only providers) this + * a provider is lazy initialized (like pull-only providers) this * will likely not return anything. */ getProvider: function (name) { @@ -410,19 +410,19 @@ AbstractHealthReporter.prototype = Object.freeze({ /** * Registers a provider from its constructor function. * - * If the provider is constant-only, it will be stashed away and + * If the provider is pull-only, it will be stashed away and * initialized later. Null will be returned. * - * If it is not constant-only, it will be initialized immediately and a + * If it is not pull-only, it will be initialized immediately and a * promise will be returned. The promise will be resolved when the * provider has finished initializing. */ registerProviderFromType: function (type) { let proto = type.prototype; - if (proto.constantOnly) { - this._log.info("Provider is constant-only. Deferring initialization: " + + if (proto.pullOnly) { + this._log.info("Provider is pull-only. Deferring initialization: " + proto.name); - this._constantOnlyProviders[proto.name] = type; + this._pullOnlyProviders[proto.name] = type; return null; } @@ -506,50 +506,50 @@ AbstractHealthReporter.prototype = Object.freeze({ }, /** - * Ensure that constant-only providers are registered. + * Ensure that pull-only providers are registered. */ - ensureConstantOnlyProvidersRegistered: function () { - if (this._constantOnlyProvidersRegistered) { + ensurePullOnlyProvidersRegistered: function () { + if (this._pullOnlyProvidersRegistered) { return Promise.resolve(); } let onFinished = function () { - this._constantOnlyProvidersRegistered = true; + this._pullOnlyProvidersRegistered = true; return Promise.resolve(); }.bind(this); - return Task.spawn(function registerConstantProviders() { - for each (let providerType in this._constantOnlyProviders) { + return Task.spawn(function registerPullProviders() { + for each (let providerType in this._pullOnlyProviders) { try { let provider = this.initProviderFromType(providerType); yield this.registerProvider(provider); } catch (ex) { - this._log.warn("Error registering constant-only provider: " + + this._log.warn("Error registering pull-only provider: " + CommonUtils.exceptionStr(ex)); } } }.bind(this)).then(onFinished, onFinished); }, - ensureConstantOnlyProvidersUnregistered: function () { - if (!this._constantOnlyProvidersRegistered) { + ensurePullOnlyProvidersUnregistered: function () { + if (!this._pullOnlyProvidersRegistered) { return Promise.resolve(); } let onFinished = function () { - this._constantOnlyProvidersRegistered = false; + this._pullOnlyProvidersRegistered = false; return Promise.resolve(); }.bind(this); - return Task.spawn(function unregisterConstantProviders() { + return Task.spawn(function unregisterPullProviders() { for (let provider of this._collector.providers) { - if (!provider.constantOnly) { + if (!provider.pullOnly) { continue; } - this._log.info("Shutting down constant-only provider: " + + this._log.info("Shutting down pull-only provider: " + provider.name); try { @@ -618,7 +618,7 @@ AbstractHealthReporter.prototype = Object.freeze({ */ collectAndObtainJSONPayload: function (asObject=false) { return Task.spawn(function collectAndObtain() { - yield this.ensureConstantOnlyProvidersRegistered(); + yield this.ensurePullOnlyProvidersRegistered(); let payload; let error; @@ -631,7 +631,7 @@ AbstractHealthReporter.prototype = Object.freeze({ this._log.warn("Error collecting and/or retrieving JSON payload: " + CommonUtils.exceptionStr(ex)); } finally { - yield this.ensureConstantOnlyProvidersUnregistered(); + yield this.ensurePullOnlyProvidersUnregistered(); if (error) { throw error; @@ -1044,7 +1044,7 @@ HealthReporter.prototype = Object.freeze({ */ requestDataUpload: function (request) { return Task.spawn(function doUpload() { - yield this.ensureConstantOnlyProvidersRegistered(); + yield this.ensurePullOnlyProvidersRegistered(); try { yield this.collectMeasurements(); try { @@ -1053,7 +1053,7 @@ HealthReporter.prototype = Object.freeze({ this._onSubmitDataRequestFailure(ex); } } finally { - yield this.ensureConstantOnlyProvidersUnregistered(); + yield this.ensurePullOnlyProvidersUnregistered(); } }.bind(this)); }, diff --git a/services/healthreport/profile.jsm b/services/healthreport/profile.jsm index 5b94d75ebd77..cf5d40bfd41a 100644 --- a/services/healthreport/profile.jsm +++ b/services/healthreport/profile.jsm @@ -214,7 +214,7 @@ ProfileMetadataProvider.prototype = { measurementTypes: [ProfileMetadataMeasurement], - constantOnly: true, + pullOnly: true, getProfileCreationDays: function () { let accessor = new ProfileCreationTimeAccessor(null, this._log); diff --git a/services/healthreport/providers.jsm b/services/healthreport/providers.jsm index c7b9b3cf89cf..34a12e513f0c 100644 --- a/services/healthreport/providers.jsm +++ b/services/healthreport/providers.jsm @@ -126,7 +126,7 @@ AppInfoProvider.prototype = Object.freeze({ measurementTypes: [AppInfoMeasurement, AppVersionMeasurement], - constantOnly: true, + pullOnly: true, appInfoFields: { // From nsIXULAppInfo. @@ -303,7 +303,7 @@ SysInfoProvider.prototype = Object.freeze({ measurementTypes: [SysInfoMeasurement], - constantOnly: true, + pullOnly: true, sysInfoFields: { cpucount: "cpuCount", @@ -486,7 +486,7 @@ SessionsProvider.prototype = Object.freeze({ measurementTypes: [CurrentSessionMeasurement, PreviousSessionsMeasurement], - constantOnly: true, + pullOnly: true, collectConstantData: function () { let previous = this.getMeasurement("previous", 3); @@ -757,7 +757,7 @@ CrashesProvider.prototype = Object.freeze({ measurementTypes: [DailyCrashesMeasurement], - constantOnly: true, + pullOnly: true, collectConstantData: function () { return Task.spawn(this._populateCrashCounts.bind(this)); diff --git a/services/healthreport/tests/xpcshell/test_healthreporter.js b/services/healthreport/tests/xpcshell/test_healthreporter.js index 2653eb1fa45c..024d0a04f353 100644 --- a/services/healthreport/tests/xpcshell/test_healthreporter.js +++ b/services/healthreport/tests/xpcshell/test_healthreporter.js @@ -171,9 +171,8 @@ add_task(function test_register_providers_from_category_manager() { reporter._shutdown(); }); -// Constant only providers are only initialized at constant collect -// time. -add_task(function test_constant_only_providers() { +// Pull-only providers are only initialized at collect time. +add_task(function test_pull_only_providers() { const category = "healthreporter-constant-only"; let cm = Cc["@mozilla.org/categorymanager;1"] @@ -194,9 +193,9 @@ add_task(function test_constant_only_providers() { do_check_neq(reporter.getProvider("DummyProvider"), null); do_check_null(reporter.getProvider("DummyConstantProvider")); - yield reporter.ensureConstantOnlyProvidersRegistered(); + yield reporter.ensurePullOnlyProvidersRegistered(); yield reporter.collectMeasurements(); - yield reporter.ensureConstantOnlyProvidersUnregistered(); + yield reporter.ensurePullOnlyProvidersUnregistered(); do_check_eq(reporter._collector._providers.size, 1); do_check_true(reporter._storage.hasProvider("DummyConstantProvider")); diff --git a/services/metrics/dataprovider.jsm b/services/metrics/dataprovider.jsm index fcc96ccd80b2..e319b5976ba9 100644 --- a/services/metrics/dataprovider.jsm +++ b/services/metrics/dataprovider.jsm @@ -392,18 +392,20 @@ this.Provider = function () { Provider.prototype = Object.freeze({ /** - * Whether the provider provides only constant data. + * Whether the provider only pulls data from other sources. * - * If this is true, the provider likely isn't instantiated until - * `collectConstantData` is called and the provider may be torn down after - * this function has finished. + * If this is true, the provider pulls data from other sources. By contrast, + * "push-based" providers subscribe to foreign sources and record/react to + * external events as they happen. * - * This is an optimization so provider instances aren't dead weight while the - * application is running. + * Pull-only providers likely aren't instantiated until a data collection + * is performed. Thus, implementations cannot rely on a provider instance + * always being alive. This is an optimization so provider instances aren't + * dead weight while the application is running. * - * This must be set on the prototype for the optimization to be realized. + * This must be set on the prototype to have an effect. */ - constantOnly: false, + pullOnly: false, /** * Obtain a `Measurement` from its name and version. diff --git a/services/metrics/modules-testing/mocks.jsm b/services/metrics/modules-testing/mocks.jsm index 9e9885470b14..3508b5986457 100644 --- a/services/metrics/modules-testing/mocks.jsm +++ b/services/metrics/modules-testing/mocks.jsm @@ -103,6 +103,6 @@ this.DummyConstantProvider = function () { DummyConstantProvider.prototype = { __proto__: DummyProvider.prototype, - constantOnly: true, + pullOnly: true, };