Bug 579178 part A - Code changes to add a "manifest" directive to read sub-manifests and read only the root chrome.manifest file, instead of enumerating components/*.manifest and chrome/*.manifest. Review comments by Mossop to be addressed in a followup commit.

modules/libjar/nsManifestZIPLoader.cpp

@@ -40,79 +40,26 @@
 
 #include "nsManifestZIPLoader.h"
 #include "nsJAR.h"
-#include "nsString.h"
+#include "mozilla/Omnijar.h"
-#include "nsStringEnumerator.h"
 
-nsManifestZIPLoader::nsManifestZIPLoader() {
+nsManifestZIPLoader::nsManifestZIPLoader() 
+    : mZipReader(new nsJAR())
+{
+    nsresult rv = reader->Open(mozilla::OmnijarPath());
+    if (NS_FAILED(rv))
+        mZipReader = NULL;
 }
 
-NS_IMPL_ISUPPORTS1(nsManifestZIPLoader, nsIManifestLoader)
+already_AddRefed<nsIInputStream>
-
+nsManifestZIPLoader::LoadEntry(const char* aName)
-nsresult
-nsManifestZIPLoader::LoadEntry(nsILocalFile* aFile,
-                               const char* aName,
-                               nsIInputStream** aResult)
 {
-    nsCOMPtr<nsIZipReader> zip = dont_AddRef(GetZipReader(aFile));
+    if (!mZipReader)
+        return NS_ERROR_NOT_INITIALIZED;
 
-    if (!zip)
+    nsCOMPtr<nsIInputStream> is;
-        return NS_OK;
+    nsresult rv = zip->GetInputStream(aName, getter_AddRefs(is));
-
-    return zip->GetInputStream(aName, aResult);
-}
-
-static void
-EnumerateEntriesForPattern(nsIZipReader* zip, const char* pattern,
-                           nsIManifestLoaderSink* aSink)
-{
-    nsCOMPtr<nsIUTF8StringEnumerator> entries;
-    if (NS_FAILED(zip->FindEntries(pattern, getter_AddRefs(entries))) ||
-        !entries) {
-        return;
-    }
-
-    PRBool hasMore;
-    int index = 0;
-    while (NS_SUCCEEDED(entries->HasMore(&hasMore)) && hasMore) {
-        nsCAutoString itemName;
-        if (NS_FAILED(entries->GetNext(itemName)))
-            return;
-
-        nsCOMPtr<nsIInputStream> stream;
-        if (NS_FAILED(zip->GetInputStream(itemName.get(), getter_AddRefs(stream))))
-            continue;
-
-        // ignore the result
-        aSink->FoundEntry(itemName.get(), index++, stream);
-    }
-}
-    
-nsresult
-nsManifestZIPLoader::EnumerateEntries(nsILocalFile* aFile,
-                                      nsIManifestLoaderSink* aSink)
-{
-    nsCOMPtr<nsIZipReader> zip = dont_AddRef(GetZipReader(aFile));
-
-    if (!zip) {
-        NS_WARNING("Could not get Zip Reader");
-        return NS_OK;
-    }
-
-    EnumerateEntriesForPattern(zip, "components/*.manifest$", aSink);
-    EnumerateEntriesForPattern(zip, "chrome/*.manifest$", aSink);
-
-    return NS_OK;
-}
-
-already_AddRefed<nsIZipReader>
-nsManifestZIPLoader::GetZipReader(nsILocalFile* file)
-{
-    NS_ASSERTION(file, "bad file");
-    
-    nsCOMPtr<nsIZipReader> reader = new nsJAR();
-    nsresult rv = reader->Open(file);
     if (NS_FAILED(rv))
         return NULL;
 
-    return reader.forget();
+    return is.forget();
 }

modules/libjar/nsManifestZIPLoader.h

@@ -42,16 +42,15 @@
 
 #include "nsIZipReader.h"
 
-class nsManifestZIPLoader : public nsIManifestLoader
+class nsManifestZIPLoader
 {
  public:
     nsManifestZIPLoader();
-    virtual ~nsManifestZIPLoader() {}
+    ~nsManifestZIPLoader();
-    
+
-    NS_DECL_ISUPPORTS
+    already_AddRefed<nsIInputStream> LoadEntry(const char* name);
-    NS_DECL_NSIMANIFESTLOADER
 
  private:
-    already_AddRefed<nsIZipReader> GetZipReader(nsILocalFile* aFile);
+    nsCOMPtr<nsIZipReader> mZipReader;
 };
 

modules/libjar/objs.mk

@@ -40,16 +40,18 @@ MODULES_LIBJAR_LCPPSRCS = \
 		nsJARInputStream.cpp \
 		nsJAR.cpp \
 		nsJARFactory.cpp \
-		nsManifestZIPLoader.cpp \
 		nsJARProtocolHandler.cpp \
 		nsJARChannel.cpp  \
 		nsJARURI.cpp  \
 		$(NULL)
 
+ifdef MOZ_OMNIJAR
+MODULES_LIBJAR_LCPPSRCS += nsManifestZIPLoader.cpp
+endif
+
 MODULES_LIBJAR_LEXPORTS = \
 		zipstruct.h \
 		nsZipArchive.h \
-		nsManifestZIPLoader.h \
 		$(NULL)
 
 MODULES_LIBJAR_LXPIDLSRCS = \

xpcom/components/Makefile.in

@@ -76,7 +76,6 @@ SDK_XPIDLSRCS	= \
 		nsIServiceManager.idl	      \
 		nsIComponentManager.idl       \
 		nsICategoryManager.idl        \
-		nsIManifestLoader.idl         \
 		$(NULL)
 
 LOCAL_INCLUDES	= \
@@ -87,6 +86,7 @@ LOCAL_INCLUDES	= \
 	-I$(srcdir)/../build \
 	-I.. \
 	-I$(topsrcdir)/chrome/src \
+	-I$(topsrcdir)/modules/libjar \
 	$(NULL)
 
 # we don't want the shared lib, but we want to force the creation of a static lib.

xpcom/components/ManifestParser.cpp

@@ -88,6 +88,8 @@ struct ManifestDirective
   bool isContract;
 };
 static const ManifestDirective kParsingTable[] = {
+  { "manifest", 1, false, true, false,
+    &nsComponentManagerImpl::ManifestManifest, NULL },
   { "binary-component", 1, true, false, false,
     &nsComponentManagerImpl::ManifestBinaryComponent, NULL },
   { "interfaces",       1, true, false, false,
@@ -600,7 +602,7 @@ ParseManifestCommon(NSLocationType aType, nsILocalFile* aFile,
         stABI == eBad)
       continue;
 
-    if (directive->ischrome) {
+    if (directive->regfunc) {
 #ifdef MOZ_IPC
       if (GeckoProcessType_Default != XRE_GetProcessType())
         continue;
@@ -619,7 +621,7 @@ ParseManifestCommon(NSLocationType aType, nsILocalFile* aFile,
       (nsChromeRegistry::gChromeRegistry->*(directive->regfunc))
 	(chromecx, line, argv, platform, contentAccessible);
     }
-    else if (!aChromeOnly) {
+    else if (directive->ischrome || !aChromeOnly) {
       if (directive->isContract) {
         CachedDirective* cd = contracts.AppendElement();
         cd->lineno = line;
@@ -643,7 +645,7 @@ void
 ParseManifest(NSLocationType type, nsILocalFile* file,
               char* buf, bool aChromeOnly)
 {
-  nsComponentManagerImpl::ManifestProcessingContext mgrcx(type, file);
+  nsComponentManagerImpl::ManifestProcessingContext mgrcx(type, file, aChromeOnly);
   nsChromeRegistry::ManifestProcessingContext chromecx(type, file);
   ParseManifestCommon(type, file, mgrcx, chromecx, NULL, buf, aChromeOnly);
 }
@@ -653,7 +655,7 @@ void
 ParseManifest(NSLocationType type, const char* jarPath,
               char* buf, bool aChromeOnly)
 {
-  nsComponentManagerImpl::ManifestProcessingContext mgrcx(type, jarPath);
+  nsComponentManagerImpl::ManifestProcessingContext mgrcx(type, jarPath, aChromeOnly);
   nsChromeRegistry::ManifestProcessingContext chromecx(type, jarPath);
   ParseManifestCommon(type, mozilla::OmnijarPath(), mgrcx, chromecx, jarPath,
                       buf, aChromeOnly);

xpcom/components/nsComponentManager.cpp

@@ -107,7 +107,6 @@
 #endif
 
 #ifdef MOZ_OMNIJAR
-#include "nsManifestZIPLoader.h"
 #include "mozilla/Omnijar.h"
 static NS_DEFINE_CID(kZipReaderCID, NS_ZIPREADER_CID);
 #endif
@@ -370,25 +369,16 @@ nsresult nsComponentManagerImpl::Init()
     InitializeStaticModules();
     InitializeModuleLocations();
 
-    NS_NAMED_LITERAL_CSTRING(strComponents, "components");
+    ComponentLocation* cl = sModuleLocations->InsertElementAt(0);
-    NS_NAMED_LITERAL_CSTRING(strChrome, "chrome");
+    cl->type = NS_COMPONENT_LOCATION;
-
+    cl->location = CloneAndAppend(appDir, NS_LITERAL_CSTRING("chrome.manifest"));
-    ComponentLocation appLocations[2] = {
-        { NS_COMPONENT_LOCATION, CloneAndAppend(appDir, strComponents) },
-        { NS_COMPONENT_LOCATION, CloneAndAppend(appDir, strChrome) },
-    };
-    sModuleLocations->
-        InsertElementsAt(0, appLocations, NS_ARRAY_LENGTH(appLocations));
 
     PRBool equals = PR_FALSE;
     appDir->Equals(greDir, &equals);
     if (!equals) {
-        ComponentLocation greLocations[2] = {
+        cl = sModuleLocations->InsertElementAt(0);
-            { NS_COMPONENT_LOCATION, CloneAndAppend(greDir, strComponents) },
+        cl->type = NS_COMPONENT_LOCATION;
-            { NS_COMPONENT_LOCATION, CloneAndAppend(greDir, strChrome) },
+        cl->location = CloneAndAppend(greDir, NS_LITERAL_CSTRING("chrome.manifest"));
-        };
-        sModuleLocations->
-            InsertElementsAt(0, greLocations, NS_ARRAY_LENGTH(greLocations));
     }
 
     PR_LOG(nsComponentManagerLog, PR_LOG_DEBUG,
@@ -407,12 +397,14 @@ nsresult nsComponentManagerImpl::Init()
         RegisterModule((*sStaticModules)[i], NULL);
 
 #ifdef MOZ_OMNIJAR
-    RegisterOmnijar(false);
+    mManifestLoader = new nsManifestZIPLoader();
+
+    RegisterOmnijar("chrome.manifest", false);
 #endif
 
     for (PRUint32 i = 0; i < sModuleLocations->Length(); ++i) {
         ComponentLocation& l = sModuleLocations->ElementAt(i);
-        RegisterLocation(l.type, l.location, false);
+        RegisterManifestFile(l.type, l.location, false);
     }
 
     nsCategoryManager::GetSingleton()->SuppressNotifications(false);
@@ -530,47 +522,14 @@ GetExtension(nsILocalFile* file)
     return extension;
 }
 
-void
-nsComponentManagerImpl::RegisterLocation(NSLocationType aType,
-                                         nsILocalFile* aLocation,
-                                         bool aChromeOnly)
-{
-    nsCOMArray<nsILocalFile> manifests;
-
-    PRBool directory = PR_FALSE;
-    aLocation->IsDirectory(&directory);
-    if (directory)
-        GetManifestsInDirectory(aLocation, manifests);
-    else if (GetExtension(aLocation).LowerCaseEqualsLiteral("manifest"))
-        manifests.AppendObject(aLocation);
-
-    for (PRInt32 i = 0; i < manifests.Count(); ++i)
-        RegisterManifestFile(aType, manifests[i], aChromeOnly);
-}
-
 #ifdef MOZ_OMNIJAR
 void
-nsComponentManagerImpl::RegisterOmnijar(bool aChromeOnly)
+nsComponentManagerImpl::RegisterOmnijar(const char* aPath, bool aChromeOnly)
 {
-    nsCOMPtr<nsIManifestLoader> loader = new nsManifestZIPLoader();
+    nsCOMPtr<nsIInputStream> is = mManifestLoader->LoadEntry(aPath);
-
-    mManifestLoader = loader;
-    mRegisterJARChromeOnly = aChromeOnly;
-
-    loader->EnumerateEntries(mozilla::OmnijarPath(), this);
-
-    mManifestLoader = NULL;
-}
-
-NS_IMETHODIMP
-nsComponentManagerImpl::FoundEntry(const char* aPath,
-                                   PRInt32 aIndex,
-                                   nsIInputStream* aStream)
-{
-    NS_ASSERTION(mManifestLoader, "Not registering a JAR.");
 
     PRUint32 flen;
-    aStream->Available(&flen);
+    is->Available(&flen);
 
     nsAutoArrayPtr<char> whole(new char[flen + 1]);
     if (!whole)
@@ -580,13 +539,13 @@ nsComponentManagerImpl::FoundEntry(const char* aPath,
         PRUint32 avail;
         PRUint32 read;
 
-        if (NS_FAILED(aStream->Available(&avail)))
+        if (NS_FAILED(is->Available(&avail)))
             return NS_ERROR_FAILURE;
 
         if (avail > flen)
             return NS_ERROR_FAILURE;
 
-        if (NS_FAILED(aStream->Read(whole + totalRead, avail, &read)))
+        if (NS_FAILED(is->Read(whole + totalRead, avail, &read)))
             return NS_ERROR_FAILURE;
 
         totalRead += read;
@@ -594,33 +553,11 @@ nsComponentManagerImpl::FoundEntry(const char* aPath,
 
     whole[flen] = '\0';
 
-    ParseManifest(NS_COMPONENT_LOCATION, aPath, whole, mRegisterJARChromeOnly);
+    ParseManifest(NS_COMPONENT_LOCATION, aPath, whole, aChromeOnly);
     return NS_OK;
 }
 #endif // MOZ_OMNIJAR
 
-void
-nsComponentManagerImpl::GetManifestsInDirectory(nsILocalFile* aDirectory,
-                                                nsCOMArray<nsILocalFile>& aManifests)
-{
-    nsCOMPtr<nsISimpleEnumerator> entries;
-    aDirectory->GetDirectoryEntries(getter_AddRefs(entries));
-    if (!entries)
-        return;
-
-    PRBool more;
-    while (NS_SUCCEEDED(entries->HasMoreElements(&more)) && more) {
-        nsCOMPtr<nsISupports> supp;
-        entries->GetNext(getter_AddRefs(supp));
-        nsCOMPtr<nsILocalFile> f = do_QueryInterface(supp);
-        if (!f)
-            continue;
-
-        if (GetExtension(f).LowerCaseEqualsLiteral("manifest"))
-            aManifests.AppendObject(f);
-    }
-}
-
 namespace {
 struct AutoCloseFD
 {
@@ -654,8 +591,12 @@ nsComponentManagerImpl::RegisterManifestFile(NSLocationType aType,
 
     AutoCloseFD fd;
     rv = aFile->OpenNSPRFileDesc(PR_RDONLY, 0444, &fd);
-    if (NS_FAILED(rv))
+    if (NS_FAILED(rv)) {
+        nsCAutoString path;
+        aFile->GetNativePath(path);
+        LogMessage("Could not read chrome manifest file '%s'.", path.get());
         return;
+    }
 
     PRFileInfo64 fileInfo;
     if (PR_SUCCESS != PR_GetOpenFileInfo64(fd, &fileInfo))
@@ -689,6 +630,39 @@ TranslateSlashes(char* path)
 }
 #endif
 
+void
+nsComponentManagerImpl::ManifestManifest(ManifestProcessingContext& cx, int lineno, char *const * argv)
+{
+    char* file = argv[0];
+
+#ifdef TRANSLATE_SLASHES
+    TranslateSlashes(file);
+#endif
+
+#ifdef MOZ_OMNIJAR
+    if (cx.mPath) {
+        nsCAutoString manifest(cx.mPath);
+        AppendFileToManifestPath(manifest, file);
+
+        RegisterOmnijar(manifest.get(), cx.mChromeOnly);
+    }
+    else
+#endif
+    {
+        nsCOMPtr<nsIFile> cfile;
+        cx.mFile->GetParent(getter_AddRefs(cfile));
+        nsCOMPtr<nsILocalFile> clfile = do_QueryInterface(cfile);
+
+        nsresult rv = clfile->AppendRelativeNativePath(nsDependentCString(file));
+        if (NS_FAILED(rv)) {
+            NS_WARNING("Couldn't append relative path?");
+            return;
+        }
+
+        RegisterManifestFile(cx.mType, clfile, cx.mChromeOnly);
+    }
+}
+
 void
 nsComponentManagerImpl::ManifestBinaryComponent(ManifestProcessingContext& cx, int lineno, char *const * argv)
 {
@@ -906,12 +880,12 @@ void
 nsComponentManagerImpl::RereadChromeManifests()
 {
 #ifdef MOZ_OMNIJAR
-    RegisterOmnijar(true);
+    RegisterOmnijar("chrome.manifest", true);
 #endif
 
     for (PRUint32 i = 0; i < sModuleLocations->Length(); ++i) {
         ComponentLocation& l = sModuleLocations->ElementAt(i);
-        RegisterLocation(l.type, l.location, true);
+        RegisterManifestFile(l.type, l.location, true);
     }
 }
 
@@ -2019,7 +1993,7 @@ XRE_AddManifestLocation(NSLocationType aType, nsILocalFile* aLocation)
 
     if (nsComponentManagerImpl::gComponentManager &&
         nsComponentManagerImpl::NORMAL == nsComponentManagerImpl::gComponentManager->mStatus)
-        nsComponentManagerImpl::gComponentManager->RegisterLocation(aType, aLocation, false);
+        nsComponentManagerImpl::gComponentManager->RegisterManifestFile(aType, aLocation, false);
 
     return NS_OK;
 }

xpcom/components/nsComponentManager.h

@@ -68,7 +68,7 @@
 
 #ifdef MOZ_OMNIJAR
 #include "mozilla/Omnijar.h"
-#include "nsIManifestLoader.h"
+#include "nsManifestZIPLoader.h"
 #endif
 
 struct nsFactoryEntry;
@@ -259,32 +259,28 @@ public:
                           KnownModule* aModule);
     void RegisterContractID(const mozilla::Module::ContractIDEntry* aEntry);
 
-    void RegisterLocation(NSLocationType aType, nsILocalFile* aLocation,
-                          bool aChromeOnly);
-
 #ifdef MOZ_OMNIJAR
-    void RegisterOmnijar(bool aChromeOnly);
+    void RegisterOmnijar(const char* aPath, bool aChromeOnly);
 #endif
 
-    void GetManifestsInDirectory(nsILocalFile* aDirectory,
-                                 nsCOMArray<nsILocalFile>& aManifests);
-
     void RegisterManifestFile(NSLocationType aType, nsILocalFile* aFile,
                               bool aChromeOnly);
 
     struct ManifestProcessingContext
     {
-        ManifestProcessingContext(NSLocationType aType, nsILocalFile* aFile)
+        ManifestProcessingContext(NSLocationType aType, nsILocalFile* aFile, bool aChromeOnly)
             : mType(aType)
             , mFile(aFile)
             , mPath(NULL)
+            , mChromeOnly(aChromeOnly)
         { }
 
 #ifdef MOZ_OMNIJAR
-        ManifestProcessingContext(NSLocationType aType, const char* aPath)
+        ManifestProcessingContext(NSLocationType aType, const char* aPath, bool aChromeOnly)
             : mType(aType)
             , mFile(mozilla::OmnijarPath())
             , mPath(aPath)
+            , mChromeOnly(aChromeOnly)
         { }
 #endif
 
@@ -293,8 +289,10 @@ public:
         NSLocationType mType;
         nsILocalFile* mFile;
         const char* mPath;
+        bool mChromeOnly;
     };
 
+    void ManifestManifest(ManifestProcessingContext& cx, int lineno, char *const * argv);
     void ManifestBinaryComponent(ManifestProcessingContext& cx, int lineno, char *const * argv);
     void ManifestXPT(ManifestProcessingContext& cx, int lineno, char *const * argv);
     void ManifestComponent(ManifestProcessingContext& cx, int lineno, char *const * argv);
@@ -331,8 +329,7 @@ private:
     ~nsComponentManagerImpl();
 
 #ifdef MOZ_OMNIJAR
-    nsIManifestLoader* mManifestLoader;
+    nsAutoPtr<nsManifestZIPLoader> mManifestLoader;
-    bool mRegisterJARChromeOnly;
 #endif
 };
 

xpcom/components/nsIManifestLoader.idl

@@ -1,91 +0,0 @@
-/* -*- Mode: IDL; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is the external XPT loader interface.
- *
- * The Initial Developer of the Original Code is
- * Netscape Communications Corp.
- * Portions created by the Initial Developer are Copyright (C) 2002
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- *   John Bandhauer <jband@netscape.com>
- *   Alec Flett <alecf@netscape.com>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either of the GNU General Public License Version 2 or later (the "GPL"),
- * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-
-#include "nsISupports.idl"
-#include "nsILocalFile.idl"
-#include "nsIInputStream.idl"
-
-/**
- * Implement nsIXPTLoaderSink if you want to enumerate the entries in
- * an XPT archive of some kind
- */
-[scriptable, uuid(6E48C500-8682-4730-ADD6-7DB693B9E7BA)]
-interface nsIManifestLoaderSink : nsISupports
-{
-    /**
-     * Called by the loader for each components / *.manifest and
-     * chrome / *.manifest in the archive.
-     * @param itemName the name of this particular item in the archive
-     * @param index the index of the item inthe archive
-     * @param stream contains the contents of the xpt file
-     */
-    void foundEntry(in string itemName,
-                    in long index,
-                    in nsIInputStream xptData);
-};
-
-/**
- * The XPT loader interface: implemented by a loader to grab an input
- * stream which will be consumed by the interface loader.
- */
-[scriptable, uuid(368A15D9-17A9-4c2b-AC3D-A35B3A22B876)]
-interface nsIManifestLoader : nsISupports {
-    /**
-     * enumerate entries in the given archive
-     * for each entry found, the loader will call the sink's
-     * foundEntry() method with the appropriate information and a
-     * stream that the consumer can read from
-     * @param file the file to read from
-     * @param sink an object which will be called with each file found
-     *             in the file
-     */
-    void enumerateEntries(in nsILocalFile file,
-                          in nsIManifestLoaderSink sink );
-
-    /**
-     * Load a specific entry from the archive
-     * @param file the file to read from
-     * @param name the name of the xpt within the file
-     * @return an input stream that will read the raw xpt data from
-     *         the file
-     */
-    nsIInputStream loadEntry(in nsILocalFile file,
-                             in string name);
-};