384042 - Extension manager uses installLocation.stageFile method (not defined in the interface)

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Wladimir Palant]

Attachment: Proposed patch

I am trying to write an nsIInstallLocation implementation and to register it in the extension-install-locations category. Unfortunately it seems that this feature of the extension manager doesn't work (and never worked). The problem is that the extension manager calls methods of installLocation like stageFile (see for example http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in&rev=1.217#4623) - but those are not part of the IDL and only exist in the local JavaScript implementations. With implementations accessed via XPCOM errors like this one occur:

Error: installLocation.stageFile is not a function
Source File: file:///.../components/nsExtensionManager.js
Line: 4697

It doesn't seem to make sense to remove these calls for install locations that don't support them, upgrading will no longer work without staged XPIs. So I would rather suggest adding stageFile, getStageFile and removeFile to the IDL.

Comment 1

[Benjamin Smedberg]

Comment on attachment 267990 [details] [diff] [review]
Proposed patch

Robstrong should approve this, but it looks pretty straightforward.

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 267990 [details] [diff] [review]
Proposed patch

>Index: toolkit/mozapps/extensions/public/nsIExtensionManager.idl
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl,v
>retrieving revision 1.45
>diff -p -u -8 -r1.45 nsIExtensionManager.idl
>--- toolkit/mozapps/extensions/public/nsIExtensionManager.idl	27 Apr 2006 03:04:31 -0000	1.45
>+++ toolkit/mozapps/extensions/public/nsIExtensionManager.idl	11 Jun 2007 18:14:41 -0000
>@@ -47,17 +47,17 @@ interface nsIAddonUpdateCheckListener;
> interface nsICommandLine;
> interface nsISimpleEnumerator;
> interface nsIDirectoryEnumerator;
> 
> /**
>  * Interface representing a location where extensions, themes etc are 
>  * installed.
>  */
>-[scriptable, uuid(D3D4DADA-C6EB-11D9-A68F-001124787B2E)]
>+[scriptable, uuid(32a74707-ec7c-af19-f4d8-d0cd8cb6a948)]
> interface nsIInstallLocation : nsISupports
> {
>   /**
>    * The string identifier of this Install Location
>    */
>   readonly attribute AString name;
>   
>   /**
>@@ -152,16 +152,47 @@ interface nsIInstallLocation : nsISuppor
>    * Install Locations are usually managed by other applications or installers.
>    *
>    * @param   id
>    *          The GUID of the item.
>    * @returns true if the item's resources are managed independently of the
>    *          Extension System, false otherwise.
>    */
>   boolean itemIsManagedIndependently(in AString id);
>+
>+  /**
>+   * Stages the specified file by copying it to some location from where it
>+   * can be retrieved later to complete installation.
>+   *
>+   * @param   file
>+   *          The file to stage
>+   * @param   id
>+   *          The GUID of the item the file represents
>+   * @returns The staged file
>+   */
>+  nsIFile stageFile(in nsIFile file, in AString id);
>+
>+  /**
>+   * Returns the most recently staged package (e.g. the last XPI or JAR in a
>+   * directory) for an item and removes items that do not qualify.
>+   *
>+   * @param   id
>+   *          The ID of the staged package
>+   * @returns an nsIFile if the package exists otherwise null.
>+   */
>+  nsIFile getStageFile(in AString id);
>+
>+  /**
>+   * Removes a file from the stage. This cleans up the stage if there is nothing
>+   * else left after the remove operation.
>+   *
>+   * @param   file
>+   *          The file to remove.
>+   */
>+  void removeFile(in nsIFile file);
> };
> 
> /**
>  * Interface representing a system for the installation and management of
>  * Extensions, Themes etc. 
>  *
>  * XXXben - Some of this stuff should go into a management-ey interface, 
>  *          some into an app-startup-ey interface.

Comment 3

[Wladimir Palant]

Does this need a testcase, and if yes - what can we test here?

Comment 4

[Robert Strong (they/them - no direct email)]

I don't see much value in testing in this instance. I do see value in testing that these methods do what they are suppose to do but that would be another bug.

I'll get this checked in sometime in the next day or two unless someone beats me to it.

Comment 5

[Ryan Flint [:rflint] (ping via IRC for reviews)]

mozilla/toolkit/mozapps/extensions/public/nsIExtensionManager.idl 1.48