Bug 295013 - installing the same theme a second time, or uninstalling a theme you just installed fails - patch by Robert Strong <moz_bugzilla@exchangecode.com> r=me a=shaver

chrome/public/nsIToolkitChromeRegistry.idl

@@ -40,7 +40,7 @@
 
 interface nsIURI;
 
-[scriptable, uuid(b1fe3107-fac6-432a-be34-fdfc26b0736c)]
+[scriptable, uuid(717FA4AB-FD62-412b-AEF5-54A50B340449)]
 interface nsIToolkitChromeRegistry : nsIXULChromeRegistry
 {
   /**
@@ -50,15 +50,22 @@ interface nsIToolkitChromeRegistry : nsIXULChromeRegistry
    *
    * @param aOldManifestURI  The URI of an old manifest to read, without
    *                         the trailing "contents.rdf", e.g.
-   *                         "jar:resource:///chrome/foo.jar!/content/foo/"
+   *                         "jar:resource:///chrome/foo.jar!/content/foo/" or
+   *                         "file://path/to/contents/rdf/"
    * @param aFile            The URI of a manifest file to write. It's a good
    *                         idea to use a resource: URI if possible.
+   * @param aBaseURI         The base URI for relative path creation
+   *                         "jar:resource:///chrome/foo.jar!/content/foo/"
+   *                         this is a separate param from aOldManifestURI so
+   *                         the "contents.rdf" can be read outside of the jar
+   *                         to keep the zipreader cache from holding it open.
    * @param aAppend          Whether we should append to an existing manifest
    *                         or truncate and start empty.
    * @param aSkinOnly        Only allow skin packages.
    */
   void processContentsManifest(in nsIURI aOldManifestURI, in nsIURI aFile,
-                               in boolean aAppend, in boolean aSkinOnly);
+                               in nsIURI aBaseURI, in boolean aAppend,
+                               in boolean aSkinOnly);
 
   /**
    * If the OS has a "high-visibility" or "disabled-friendly" theme set,

chrome/src/nsChromeRegistry.cpp

@@ -1299,7 +1299,8 @@ nsChromeRegistry::ProcessNewChromeBuffer(char *aBuffer, PRInt32 aLength,
         if (NS_FAILED(rv)) return rv;
       }
 
-      ProcessContentsManifest(baseURI, aManifest, PR_TRUE, strcmp(chromeType, "skin") == 0);
+      ProcessContentsManifest(baseURI, aManifest, baseURI, PR_TRUE,
+                              strcmp(chromeType, "skin") == 0);
     }
     
     while (aBuffer < bufferEnd && (*aBuffer == '\0' || *aBuffer == ' ' || *aBuffer == '\r' || *aBuffer == '\n'))
@@ -1459,12 +1460,13 @@ static const PRInt32 kNSPR_TRUNCATE_FLAGS = PR_WRONLY | PR_CREATE_FILE | PR_TRUN
 
 NS_IMETHODIMP
 nsChromeRegistry::ProcessContentsManifest(nsIURI* aOldManifest, nsIURI* aFile,
-                                          PRBool aAppend, PRBool aSkinOnly)
+                                          nsIURI* aBaseURI, PRBool aAppend,
+                                          PRBool aSkinOnly)
 {
   nsresult rv;
 
   nsCAutoString relativePath;
-  GetRelativePath(aFile, aOldManifest, relativePath);
+  GetRelativePath(aFile, aBaseURI, relativePath);
 
   nsCAutoString spec;
   aOldManifest->GetSpec(spec);

toolkit/mozapps/extensions/src/nsExtensionManager.js.in

@@ -1417,7 +1417,7 @@ Installer.prototype = {
       }
       catch (e) {
         zipReader.close();
-        LOG("installTheme: failed to open Theme jar file: " + jarFile.path);
+        LOG("extractThemeFiles: failed to open Theme jar file: " + jarFile.path);
         throw e; // let the safe-op clean up
       }
 
@@ -1456,22 +1456,30 @@ Installer.prototype = {
         zipReader.close();
       }
       else { // old theme structure requires only an install.rdf
+        try {
+          var entry = zipReader.getEntry(FILE_CONTENTS_MANIFEST);
+          var contentsManifestFile = installLocation.getItemFile(id, FILE_CONTENTS_MANIFEST);
+          zipReader.extract(FILE_CONTENTS_MANIFEST, contentsManifestFile);
+        }
+        catch (e) {
+          zipReader.close();
+          LOG("extractThemeFiles: failed to extract contents.rdf: " + target.path);
+          throw e; // let the safe-op clean up
+        }
         zipReader.close();
         var chromeDir = installLocation.getItemFile(id, DIR_CHROME);
         try {
           jarFile.copyTo(chromeDir, jarFile.fileName);
         }
         catch (e) {
-          LOG("installTheme: failed to copy theme JAR file to: " + chromeDir.path);
+          LOG("extractThemeFiles: failed to copy theme JAR file to: " + chromeDir.path);
           throw e; // let the safe-op clean up
         }
 
         if (!installer.metadataDS && installer._type == nsIUpdateItem.TYPE_THEME) {
-          var contentsManifestFile = extractRDFFileToTempDir(jarFile, FILE_CONTENTS_MANIFEST, false);
           if (contentsManifestFile && contentsManifestFile.exists()) {
             var contentsManifest = gRDF.GetDataSourceBlocking(getURLSpecFromFile(contentsManifestFile));
             showOldThemeError(contentsManifest);
-            contentsManifestFile.remove(false);
           }
           LOG("Theme JAR file: " + jarFile.leafName + " contains an Old-Style " + 
               "Theme that is not compatible with this version of the software.");
@@ -1506,16 +1514,18 @@ Installer.prototype = {
       var chromeDir = this._installLocation.getItemFile(this._id, DIR_CHROME);
       // We're relying on the fact that there is only one JAR file
       // in the "chrome" directory. This is a hack, but it works.
-
       var entries = chromeDir.directoryEntries.QueryInterface(nsIDirectoryEnumerator);
       var jarFile = entries.nextFile;
       if (jarFile) {
         var jarFileURI = getURIFromFile(jarFile);
         var contentsURI = newURI("jar:" + jarFileURI.spec + "!/");
+        var contentsFile = this._installLocation.getItemFile(this._id, FILE_CONTENTS_MANIFEST);
+        var contentsFileURI = getURIFromFile(contentsFile.parent);
 
-        cr.processContentsManifest(contentsURI, manifestURI, false, true);
+        cr.processContentsManifest(contentsFileURI, manifestURI, contentsURI, false, true);
       }
       entries.close();
+      contentsFile.remove(false);
     }
     catch (e) {
       // Failed to register chrome, for any number of reasons - non-existent 
@@ -1612,7 +1622,7 @@ Installer.prototype = {
     var fileURI = newURI(fileURLSpec);
     var cr = Components.classes["@mozilla.org/chrome/chrome-registry;1"]
                         .getService(Components.interfaces.nsIToolkitChromeRegistry);
-    cr.processContentsManifest(fileURI, manifest, true, false);
+    cr.processContentsManifest(fileURI, manifest, fileURI, true, false);
   }  
 };
 
@@ -2796,7 +2806,7 @@ ExtensionManager.prototype = {
             // Use the Chrome Registry API to install the theme there
             var cr = Components.classes["@mozilla.org/chrome/chrome-registry;1"]
                               .getService(Components.interfaces.nsIToolkitChromeRegistry);
-            cr.processContentsManifest(contentsURI, manifestURI, false, true);
+            cr.processContentsManifest(contentsURI, manifestURI, contentsURI, false, true);
           }
           entries.close();
         }

xpinstall/src/nsRegisterItem.cpp

@@ -297,7 +297,7 @@ PRInt32 nsRegisterItem::Complete()
             else {
                 PRBool skinOnly = (mChromeType & CHROME_ALL) == CHROME_SKIN;
                 rv = reg->ProcessContentsManifest(baseuri, manifesturi,
-                                                  PR_TRUE,
+                                                  baseuri, PR_TRUE,
                                                   skinOnly);
                 if (NS_FAILED(rv)) {
                     LogError(NS_LITERAL_STRING("ProcessContentsManifest failed."), rv);