Skip to content

Loading…

WIP: installUrl; addon listeners #525

Closed
wants to merge 1 commit into from

4 participants

@gregglind
Mozilla member
  1. stubs for listerner for all addon install events
  2. method for installUrl

TODO:

  1. should these promises resolve with the aAddon, or aAddon.id?
  2. can we trigger the doorhanger ui?
@gregglind gregglind WIP: installUrl; addon listeners
1.  stubs for listerner for all addon install events
2.  method for installUrl

TODO:

1.  should these promises resolve with the aAddon, or aAddon.id?
2.  can we trigger the doorhanger ui?
50c419a
@ochameau ochameau commented on the diff
packages/api-utils/lib/addon/installer.js
@@ -18,6 +30,57 @@ exports.ERROR_CORRUPT_FILE = AddonManager.ERROR_CORRUPT_FILE;
exports.ERROR_FILE_ACCESS = AddonManager.ERROR_FILE_ACCESS;
/**
+ * `install` listener events
+ *
+ * https://developer.mozilla.org/en-US/docs/Addons/Add-on_Manager/InstallListener
+ */
+
+// Listen for various install events. Using a class here is possibly overkill.
+// see git history for when this was a simpler object.
@ochameau Mozilla member

Yes, looks overkill to me.

@gregglind Mozilla member

I am not sure how else to the Listener here without making a class for it. Ideas welcome.

@ochameau Mozilla member

Any standard JS way of doing, like this:

function Listener(defer) {

  return {
    onInstallEnded: function () {
      ... defer.resolve  ...
    },
    ...
  };
}

Or with prototype.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ochameau ochameau commented on the diff
packages/api-utils/lib/addon/installer.js
@@ -68,6 +118,30 @@ exports.install = function install(xpiPath) {
return promise;
};
+
+exports.install = exports.installFile;
+
+exports.installUrl = function installUrl(url,hash,mimetype){
+ let { promise, resolve, reject } = defer();
+
+ if (mimetype === undefined) {mimetype = "application/x-xpinstall"};
+ if (hash === undefined) {hash = ""};
+
+ // TODO, should we download the file ourselves, then go with file?
@ochameau Mozilla member

It seems fine to use AddonManager.getInstallForURL here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ochameau
Mozilla member

It is ok to change the resolution from id to addon if it can help your usage.
But please modify unit test. And it would be really cool to have your new method tested!

@Gozala Gozala commented on the diff
packages/api-utils/lib/addon/installer.js
((15 lines not shown))
const { Cc, Ci, Cu } = require("chrome");
const { AddonManager } = Cu.import("resource://gre/modules/AddonManager.jsm");
const { defer } = require("api-utils/promise");
const { setTimeout } = require("api-utils/timer");
+const { Class, mix } = require('api-utils/heritage');
@Gozala Mozilla member
Gozala added a note

mix is not used, no need to import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Gozala
Mozilla member

@ochameau @gregglind what's a plan for this change. Do you still intend on implementing such a change ? If not please close a pull request.

@KWierso KWierso closed this
@Gozala
Mozilla member

Feel free to reopen if you still find it to be relevant

@gregglind gregglind referenced this pull request in mozilla/testpilot2
Closed

Use the jetpack addon/installer.js once it lands #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 14, 2012
  1. @gregglind

    WIP: installUrl; addon listeners

    gregglind committed
    1.  stubs for listerner for all addon install events
    2.  method for installUrl
    
    TODO:
    
    1.  should these promises resolve with the aAddon, or aAddon.id?
    2.  can we trigger the doorhanger ui?
Showing with 99 additions and 25 deletions.
  1. +99 −25 packages/api-utils/lib/addon/installer.js
View
124 packages/api-utils/lib/addon/installer.js
@@ -2,10 +2,22 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+/* Promise-style installer for a addons */
+
+
+/* references:
+
+aInstall: https://developer.mozilla.org/en-US/docs/Addons/Add-on_Manager/AddonInstall
+aAddon: https://developer.mozilla.org/en-US/docs/Addons/Add-on_Manager/Addon
+Listener: https://developer.mozilla.org/en-US/docs/Addons/Add-on_Manager/InstallListener
+
+*/
+
const { Cc, Ci, Cu } = require("chrome");
const { AddonManager } = Cu.import("resource://gre/modules/AddonManager.jsm");
const { defer } = require("api-utils/promise");
const { setTimeout } = require("api-utils/timer");
+const { Class, mix } = require('api-utils/heritage');
@Gozala Mozilla member
Gozala added a note

mix is not used, no need to import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
/**
* `install` method error codes:
@@ -18,6 +30,57 @@ exports.ERROR_CORRUPT_FILE = AddonManager.ERROR_CORRUPT_FILE;
exports.ERROR_FILE_ACCESS = AddonManager.ERROR_FILE_ACCESS;
/**
+ * `install` listener events
+ *
+ * https://developer.mozilla.org/en-US/docs/Addons/Add-on_Manager/InstallListener
+ */
+
+// Listen for various install events. Using a class here is possibly overkill.
+// see git history for when this was a simpler object.
@ochameau Mozilla member

Yes, looks overkill to me.

@gregglind Mozilla member

I am not sure how else to the Listener here without making a class for it. Ideas welcome.

@ochameau Mozilla member

Any standard JS way of doing, like this:

function Listener(defer) {

  return {
    onInstallEnded: function () {
      ... defer.resolve  ...
    },
    ...
  };
}

Or with prototype.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+let Listener = Class({
+ initialize: function initialize(promise,resolve,reject) {
+ // TODO, what should the args here be? we need to bind to the promise.
+ this.promise = promise;
+ this.reject = reject;
+ this.resolve = resolve;
+ },
+ //extends: EventTarget, // ?
+ type: 'InstallEventListener',
+ myself: this, // are we dancing too much here?
+ onInstallEnded: function(aInstall, aAddon) {
+ console.log('hooray, we finished the install!');
+ aInstall.removeListener(this.myself);
+ // Bug 749745: on FF14+, onInstallEnded is called just before `startup()`
+ // is called, but we expect to resolve the promise only after it.
+ // As startup is called synchronously just after onInstallEnded,
+ // a simple setTimeout(0) is enough
+ setTimeout(this.resolve, 0, aAddon); // return the whole addon
+ //
+ },
+ onInstallFailed: function (aInstall) {
+ console.log("failed");
+ aInstall.removeListener(this.myself);
+ console.log('I really truly failed');
+ this.reject(aInstall.error);
+ },
+ onDownloadFailed: function(aInstall) {
+ this.onInstallFailed(aInstall);
+ },
+ onNewInstall: function(aInstall){ console.log('NewInstall')},
+ onDownloadStarted: function(aInstall){},
+ onDownloadProgress: function(aInstall){},
+ onDownloadEnded: function(aInstall){},
+ onDownloadCancelled: function(aInstall){},
+ onInstallStarted: function(aInstall){console.log('Install starting in listener')},
+ onInstallCancelled: function(aInstall){},
+ onExternalInstall: function(aInstall,aAddon,needsRestart){}
+
+ // TODO, add 'hash failure?'
+});
+
+
+/**
* Immediatly install an addon.
*
* @param {String} xpiPath
@@ -26,7 +89,9 @@ exports.ERROR_FILE_ACCESS = AddonManager.ERROR_FILE_ACCESS;
* A promise resolved when the addon is finally installed.
* Resolved with addon id as value or rejected with an error code.
*/
-exports.install = function install(xpiPath) {
+
+exports.installFile = function installFile(xpiPath) {
+ console.log('into the install');
let { promise, resolve, reject } = defer();
// Create nsIFile for the xpi file
@@ -40,24 +105,9 @@ exports.install = function install(xpiPath) {
}
// Listen for installation end
- let listener = {
- onInstallEnded: function(aInstall, aAddon) {
- aInstall.removeListener(listener);
- // Bug 749745: on FF14+, onInstallEnded is called just before `startup()`
- // is called, but we expect to resolve the promise only after it.
- // As startup is called synchronously just after onInstallEnded,
- // a simple setTimeout(0) is enough
- setTimeout(resolve, 0, aAddon.id);
- },
- onInstallFailed: function (aInstall) {
- console.log("failed");
- aInstall.removeListener(listener);
- reject(aInstall.error);
- },
- onDownloadFailed: function(aInstall) {
- this.onInstallFailed(aInstall);
- }
- };
+ let listener = new Listener(promise,resolve,reject);
+ console.log("made a listener");
+ listener.onNewInstall(); // test some pipes
// Order AddonManager to install the addon
AddonManager.getInstallForFile(file, function(install) {
@@ -68,6 +118,30 @@ exports.install = function install(xpiPath) {
return promise;
};
+
+exports.install = exports.installFile;
+
+exports.installUrl = function installUrl(url,hash,mimetype){
+ let { promise, resolve, reject } = defer();
+
+ if (mimetype === undefined) {mimetype = "application/x-xpinstall"};
+ if (hash === undefined) {hash = ""};
+
+ // TODO, should we download the file ourselves, then go with file?
@ochameau Mozilla member

It seems fine to use AddonManager.getInstallForURL here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ let listener = new Listener(promise,resolve,reject);
+ console.log("made a listener");
+ listener.onNewInstall();
+
+ AddonManager.getInstallForURL(url, function(install) {
+ install.addListener(listener);
+ install.install();
+ },
+ mimetype,
+ hash
+ );
+ return promise;
+};
+
exports.uninstall = function uninstall(addonId) {
let { promise, resolve, reject } = defer();
@@ -77,14 +151,14 @@ exports.uninstall = function uninstall(addonId) {
if (aAddon.id != addonId)
return;
AddonManager.removeAddonListener(listener);
- resolve();
+ resolve(aAddon);
}
};
AddonManager.addAddonListener(listener);
// Order Addonmanager to uninstall the addon
- AddonManager.getAddonByID(addonId, function (addon) {
- addon.uninstall();
+ AddonManager.getAddonByID(addonId, function (aAddon) {
+ aAddon.uninstall();
});
return promise;
@@ -93,9 +167,9 @@ exports.uninstall = function uninstall(addonId) {
exports.disable = function disable(addonId) {
let { promise, resolve, reject } = defer();
- AddonManager.getAddonByID(addonId, function (addon) {
- addon.userDisabled = true;
- resolve();
+ AddonManager.getAddonByID(addonId, function (aAddon) {
+ aAddon.userDisabled = true;
+ resolve(aAddon);
});
return promise;
Something went wrong with that request. Please try again.