From 026abf6d83aeb33adcaa390b957bba2df2215a61 Mon Sep 17 00:00:00 2001
From: Mike Hommey <mh+mozilla@glandium.org>
Date: Tue, 19 Feb 2013 11:02:12 +0100
Subject: [PATCH] Bug 840094 - Change how nsZipArchive logging works.
 r=taras,r=gps

Now log in a single file given by the MOZ_JAR_LOG_FILE environment variable.
Log entries contain the URI of the Zip archive, followed by the path in the
archive.
* * *
Bug 840094 - Fixup for debug builds failure because of nsZipArchive::CloseArchive being called several times
---
 build/pgo/profileserver.py                  |   4 +-
 client.mk                                   |   3 +-
 modules/libjar/nsZipArchive.cpp             | 105 ++++++++++++++------
 modules/libjar/nsZipArchive.h               |   5 +-
 python/mozbuild/mozpack/chrome/manifest.py  |   2 +-
 python/mozbuild/mozpack/mozjar.py           |  65 ++++++++++++
 python/mozbuild/mozpack/test/test_mozjar.py |  58 ++++++++++-
 toolkit/mozapps/installer/packager.mk       |   4 +-
 toolkit/mozapps/installer/packager.py       |  20 ++--
 9 files changed, 219 insertions(+), 47 deletions(-)

diff --git a/build/pgo/profileserver.py b/build/pgo/profileserver.py
index 16cc3a721f61e..05212ca75a249 100644
--- a/build/pgo/profileserver.py
+++ b/build/pgo/profileserver.py
@@ -20,7 +20,7 @@ from automationutils import getDebuggerInfo, addCommonOptions
 
 PORT = 8888
 PROFILE_DIRECTORY = os.path.abspath(os.path.join(SCRIPT_DIR, "./pgoprofile"))
-MOZ_JAR_LOG_DIR = os.path.abspath(os.getenv("JARLOG_DIR"))
+MOZ_JAR_LOG_FILE = os.path.abspath(os.getenv("JARLOG_FILE"))
 os.chdir(SCRIPT_DIR)
 
 class EasyServer(SocketServer.TCPServer):
@@ -47,7 +47,7 @@ if __name__ == '__main__':
   automation.initializeProfile(PROFILE_DIRECTORY)
   browserEnv = automation.environment()
   browserEnv["XPCOM_DEBUG_BREAK"] = "warn"
-  browserEnv["MOZ_JAR_LOG_DIR"] = MOZ_JAR_LOG_DIR
+  browserEnv["MOZ_JAR_LOG_FILE"] = MOZ_JAR_LOG_FILE
 
   url = "http://localhost:%d/index.html" % PORT
   appPath = os.path.join(SCRIPT_DIR, automation.DEFAULT_APP)
diff --git a/client.mk b/client.mk
index 0d8dfc2effe6e..8bca03b49704f 100644
--- a/client.mk
+++ b/client.mk
@@ -203,7 +203,8 @@ endif
 profiledbuild::
 	$(MAKE) -f $(TOPSRCDIR)/client.mk realbuild MOZ_PROFILE_GENERATE=1 MOZ_PGO_INSTRUMENTED=1
 	$(MAKE) -C $(PGO_OBJDIR) package MOZ_PGO_INSTRUMENTED=1 MOZ_INTERNAL_SIGNING_FORMAT= MOZ_EXTERNAL_SIGNING_FORMAT=
-	MOZ_PGO_INSTRUMENTED=1 OBJDIR=${PGO_OBJDIR} JARLOG_DIR=${PGO_OBJDIR}/jarlog/en-US $(PROFILE_GEN_SCRIPT)
+	rm -f ${PGO_OBJDIR}/jarlog/en-US.log
+	MOZ_PGO_INSTRUMENTED=1 OBJDIR=${PGO_OBJDIR} JARLOG_FILE=${PGO_OBJDIR}/jarlog/en-US.log $(PROFILE_GEN_SCRIPT)
 	$(MAKE) -f $(TOPSRCDIR)/client.mk maybe_clobber_profiledbuild
 	$(MAKE) -f $(TOPSRCDIR)/client.mk realbuild MOZ_PROFILE_USE=1
 
diff --git a/modules/libjar/nsZipArchive.cpp b/modules/libjar/nsZipArchive.cpp
index ba60414970ff1..a25cfadb19a7c 100644
--- a/modules/libjar/nsZipArchive.cpp
+++ b/modules/libjar/nsZipArchive.cpp
@@ -55,6 +55,9 @@
 #  endif
 #endif  /* XP_UNIX */
 
+#ifdef XP_WIN
+#include "private/pprio.h"  // To get PR_ImportFile
+#endif
 
 using namespace mozilla;
 
@@ -70,6 +73,71 @@ static uint32_t HashName(const char* aName, uint16_t nameLen);
 static nsresult ResolveSymlink(const char *path);
 #endif
 
+class ZipArchiveLogger {
+public:
+  void Write(const nsACString &zip, const char *entry) const {
+    if (!fd) {
+      char *env = PR_GetEnv("MOZ_JAR_LOG_FILE");
+      if (!env)
+        return;
+
+      nsCOMPtr<nsIFile> logFile;
+      nsresult rv = NS_NewLocalFile(NS_ConvertUTF8toUTF16(env), false, getter_AddRefs(logFile));
+      if (NS_FAILED(rv))
+        return;
+
+      // Create the log file and its parent directory (in case it doesn't exist)
+      logFile->Create(nsIFile::NORMAL_FILE_TYPE, 0644);
+
+      PRFileDesc* file;
+#ifdef XP_WIN
+      // PR_APPEND is racy on Windows, so open a handle ourselves with flags that
+      // will work, and use PR_ImportFile to make it a PRFileDesc.
+      // This can go away when bug 840435 is fixed.
+      nsAutoString path;
+      logFile->GetPath(path);
+      if (path.IsEmpty())
+        return;
+      HANDLE handle = CreateFileW(path.get(), FILE_APPEND_DATA, FILE_SHARE_WRITE,
+                                  NULL, OPEN_ALWAYS, 0, NULL);
+      if (handle == INVALID_HANDLE_VALUE)
+        return;
+      file = PR_ImportFile((PROsfd)handle);
+      if (!file)
+        return;
+#else
+      rv = logFile->OpenNSPRFileDesc(PR_WRONLY|PR_CREATE_FILE|PR_APPEND, 0644, &file);
+      if (NS_FAILED(rv))
+        return;
+#endif
+      fd = file;
+    }
+    nsCString buf(zip);
+    buf.Append(" ");
+    buf.Append(entry);
+    buf.Append('\n');
+    PR_Write(fd, buf.get(), buf.Length());
+  }
+
+  void AddRef() {
+    MOZ_ASSERT(refCnt >= 0);
+    ++refCnt;
+  }
+
+  void Release() {
+    MOZ_ASSERT(refCnt > 0);
+    if ((0 == --refCnt) && fd) {
+      PR_Close(fd);
+      fd = NULL;
+    }
+  }
+private:
+  int refCnt;
+  mutable PRFileDesc *fd;
+};
+
+static ZipArchiveLogger zipLog;
+
 //***********************************************************
 // For every inflation the following allocations are done:
 // malloc(1 * 9520)
@@ -189,27 +257,9 @@ nsresult nsZipArchive::OpenArchive(nsZipHandle *aZipHandle)
 
   //-- get table of contents for archive
   nsresult rv = BuildFileList();
-  char *env = PR_GetEnv("MOZ_JAR_LOG_DIR");
-  if (env && NS_SUCCEEDED(rv) && aZipHandle->mFile) {
-    nsCOMPtr<nsIFile> logFile;
-    nsresult rv2 = NS_NewLocalFile(NS_ConvertUTF8toUTF16(env), false, getter_AddRefs(logFile));
-    
-    if (!NS_SUCCEEDED(rv2))
-      return rv;
-
-    // Create a directory for the log (in case it doesn't exist)
-    logFile->Create(nsIFile::DIRECTORY_TYPE, 0700);
-
-    nsAutoString name;
-    nsCOMPtr<nsIFile> file = aZipHandle->mFile.GetBaseFile();
-    file->GetLeafName(name);
-    name.Append(NS_LITERAL_STRING(".log"));
-    logFile->Append(name);
-
-    PRFileDesc* fd;
-    rv2 = logFile->OpenNSPRFileDesc(PR_WRONLY|PR_CREATE_FILE|PR_APPEND, 0644, &fd);
-    if (NS_SUCCEEDED(rv2))
-      mLog = fd;
+  if (NS_SUCCEEDED(rv)) {
+    if (aZipHandle->mFile)
+      aZipHandle->mFile.GetURIString(mURI);
   }
   return rv;
 }
@@ -300,13 +350,8 @@ MOZ_WIN_MEM_TRY_BEGIN
       if ((len == item->nameLength) && 
           (!memcmp(aEntryName, item->Name(), len))) {
         
-        if (mLog) {
-          // Successful GetItem() is a good indicator that the file is about to be read
-          char *tmp = PL_strdup(aEntryName);
-          tmp[len]='\n';
-          PR_Write(mLog, tmp, len+1);
-          PL_strfree(tmp);
-        }
+        // Successful GetItem() is a good indicator that the file is about to be read
+        zipLog.Write(mURI, aEntryName);
         return item; //-- found it
       }
       item = item->next;
@@ -742,6 +787,8 @@ nsZipArchive::nsZipArchive()
   : mRefCnt(0)
   , mBuiltSynthetics(false)
 {
+  zipLog.AddRef();
+
   MOZ_COUNT_CTOR(nsZipArchive);
 
   // initialize the table to NULL
@@ -756,6 +803,8 @@ nsZipArchive::~nsZipArchive()
   CloseArchive();
 
   MOZ_COUNT_DTOR(nsZipArchive);
+
+  zipLog.Release();
 }
 
 
diff --git a/modules/libjar/nsZipArchive.h b/modules/libjar/nsZipArchive.h
index 4b8229bbebe2e..895a19ebd1d66 100644
--- a/modules/libjar/nsZipArchive.h
+++ b/modules/libjar/nsZipArchive.h
@@ -217,9 +217,8 @@ private:
   // file handle
   nsRefPtr<nsZipHandle> mFd;
 
-  // logging handle
-  mozilla::AutoFDClose mLog;
-
+  // file URI, for logging
+  nsCString mURI;
 
 private:
   //--- private methods ---
diff --git a/python/mozbuild/mozpack/chrome/manifest.py b/python/mozbuild/mozpack/chrome/manifest.py
index 57a19a67ba365..39ab0029b699a 100644
--- a/python/mozbuild/mozpack/chrome/manifest.py
+++ b/python/mozbuild/mozpack/chrome/manifest.py
@@ -349,7 +349,7 @@ def parse_manifest(root, path, fileobj=None):
     if not fileobj:
         fileobj = open(path)
     linenum = 0
-    for line in fileobj.readlines():
+    for line in fileobj:
         linenum += 1
         with errors.context(path, linenum):
             e = parse_manifest_line(base, line)
diff --git a/python/mozbuild/mozpack/mozjar.py b/python/mozbuild/mozpack/mozjar.py
index 9baf13e2cda39..4228ee48f440d 100644
--- a/python/mozbuild/mozpack/mozjar.py
+++ b/python/mozbuild/mozpack/mozjar.py
@@ -11,6 +11,8 @@ from zipfile import (
     ZIP_DEFLATED,
 )
 from collections import OrderedDict
+from urlparse import urlparse, ParseResult
+import mozpack.path
 
 JAR_STORED = ZIP_STORED
 JAR_DEFLATED = ZIP_DEFLATED
@@ -279,6 +281,12 @@ class JarFileReader(object):
         '''
         return self.read().splitlines(True)
 
+    def __iter__(self):
+        '''
+        Iterator, to support the "for line in fileobj" constructs.
+        '''
+        return iter(self.readlines())
+
     def seek(self, pos, whence=os.SEEK_SET):
         '''
         Change the current position in the uncompressed data. Subsequent reads
@@ -732,3 +740,60 @@ class Deflater(object):
         if self.compressed:
             return self._deflated.getvalue()
         return self._data.getvalue()
+
+
+class JarLog(dict):
+    '''
+    Helper to read the file Gecko generates when setting MOZ_JAR_LOG_FILE.
+    The jar log is then available as a dict with the jar path as key (see
+    canonicalize for more details on the key value), and the corresponding
+    access log as a list value. Only the first access to a given member of
+    a jar is stored.
+    '''
+    def __init__(self, file=None, fileobj=None):
+        if not fileobj:
+            fileobj = open(file, 'r')
+        urlmap = {}
+        for line in fileobj:
+            url, path = line.strip().split(None, 1)
+            if not url or not path:
+                continue
+            if url not in urlmap:
+                urlmap[url] = JarLog.canonicalize(url)
+            jar = urlmap[url]
+            entry = self.setdefault(jar, [])
+            if path not in entry:
+                entry.append(path)
+
+    @staticmethod
+    def canonicalize(url):
+        '''
+        The jar path is stored in a MOZ_JAR_LOG_FILE log as a url. This method
+        returns a unique value corresponding to such urls.
+        - file:///{path} becomes {path}
+        - jar:file:///{path}!/{subpath} becomes ({path}, {subpath})
+        - jar:jar:file:///{path}!/{subpath}!/{subpath2} becomes
+           ({path}, {subpath}, {subpath2})
+        '''
+        if not isinstance(url, ParseResult):
+            # Assume that if it doesn't start with jar: or file:, it's a path.
+            if not url.startswith(('jar:', 'file:')):
+                url = 'file:///' + os.path.abspath(url)
+            url = urlparse(url)
+        assert url.scheme
+        assert url.scheme in ('jar', 'file')
+        if url.scheme == 'jar':
+            path = JarLog.canonicalize(url.path)
+            if isinstance(path, tuple):
+                return path[:-1] + tuple(path[-1].split('!/', 1))
+            return tuple(path.split('!/', 1))
+        if url.scheme == 'file':
+            assert os.path.isabs(url.path)
+            path = url.path
+            # On Windows, url.path will be /drive:/path ; on Unix systems,
+            # /path. As we want drive:/path instead of /drive:/path on Windows,
+            # remove the leading /.
+            if os.path.isabs(path[1:]):
+                path = path[1:]
+            path = os.path.realpath(path)
+            return mozpack.path.normsep(os.path.normcase(path))
diff --git a/python/mozbuild/mozpack/test/test_mozjar.py b/python/mozbuild/mozpack/test/test_mozjar.py
index c4c73d8e22a72..8e155be13dcd3 100644
--- a/python/mozbuild/mozpack/test/test_mozjar.py
+++ b/python/mozbuild/mozpack/test/test_mozjar.py
@@ -9,11 +9,16 @@ from mozpack.mozjar import (
     JarReader,
     JarWriter,
     Deflater,
-    OrderedDict,
+    JarLog,
 )
+from collections import OrderedDict
 from mozpack.test.test_files import MockDest
 import unittest
 import mozunit
+from cStringIO import StringIO
+from urllib import pathname2url
+import mozpack.path
+import os
 
 
 class TestJarStruct(unittest.TestCase):
@@ -255,5 +260,56 @@ class TestPreload(unittest.TestCase):
         self.assertEqual(files[2].filename, 'foo')
 
 
+class TestJarLog(unittest.TestCase):
+    def test_jarlog(self):
+        base = 'file:' + pathname2url(os.path.abspath(os.curdir))
+        s = StringIO('\n'.join([
+            base + '/bar/baz.jar first',
+            base + '/bar/baz.jar second',
+            base + '/bar/baz.jar third',
+            base + '/bar/baz.jar second',
+            base + '/bar/baz.jar second',
+            'jar:' + base + '/qux.zip!/omni.ja stuff',
+            base + '/bar/baz.jar first',
+            'jar:' + base + '/qux.zip!/omni.ja other/stuff',
+            'jar:' + base + '/qux.zip!/omni.ja stuff',
+            base + '/bar/baz.jar third',
+            'jar:jar:' + base + '/qux.zip!/baz/baz.jar!/omni.ja nested/stuff',
+            'jar:jar:jar:' + base + '/qux.zip!/baz/baz.jar!/foo.zip!/omni.ja' +
+            ' deeply/nested/stuff',
+        ]))
+        log = JarLog(fileobj=s)
+        canonicalize = lambda p: \
+            mozpack.path.normsep(os.path.normcase(os.path.realpath(p)))
+        baz_jar = canonicalize('bar/baz.jar')
+        qux_zip = canonicalize('qux.zip')
+        self.assertEqual(set(log.keys()), set([
+            baz_jar,
+            (qux_zip, 'omni.ja'),
+            (qux_zip, 'baz/baz.jar', 'omni.ja'),
+            (qux_zip, 'baz/baz.jar', 'foo.zip', 'omni.ja'),
+        ]))
+        self.assertEqual(log[baz_jar], [
+            'first',
+            'second',
+            'third',
+        ])
+        self.assertEqual(log[(qux_zip, 'omni.ja')], [
+            'stuff',
+            'other/stuff',
+        ])
+        self.assertEqual(log[(qux_zip, 'baz/baz.jar', 'omni.ja')],
+                         ['nested/stuff'])
+        self.assertEqual(log[(qux_zip, 'baz/baz.jar', 'foo.zip',
+                              'omni.ja')], ['deeply/nested/stuff'])
+
+        # The above tests also indirectly check the value returned by
+        # JarLog.canonicalize for various jar: and file: urls, but
+        # JarLog.canonicalize also supports plain paths.
+        self.assertEqual(JarLog.canonicalize(os.path.abspath('bar/baz.jar')),
+                         baz_jar)
+        self.assertEqual(JarLog.canonicalize('bar/baz.jar'), baz_jar)
+
+
 if __name__ == '__main__':
     mozunit.main()
diff --git a/toolkit/mozapps/installer/packager.mk b/toolkit/mozapps/installer/packager.mk
index fcbd3fda7705b..7b9c30046763e 100644
--- a/toolkit/mozapps/installer/packager.mk
+++ b/toolkit/mozapps/installer/packager.mk
@@ -98,7 +98,7 @@ endif # LIBXUL_SDK
 
 _ABS_DIST = $(call core_abspath,$(DIST))
 JARLOG_DIR = $(call core_abspath,$(DEPTH)/jarlog/)
-JARLOG_DIR_AB_CD = $(JARLOG_DIR)/$(AB_CD)
+JARLOG_FILE_AB_CD = $(JARLOG_DIR)/$(AB_CD).log
 
 TAR_CREATE_FLAGS := --exclude=.mkdir.done $(TAR_CREATE_FLAGS)
 CREATE_FINAL_TAR = $(TAR) -c --owner=0 --group=0 --numeric-owner \
@@ -591,7 +591,7 @@ stage-package: $(MOZ_PKG_MANIFEST)
 		$(addprefix --removals ,$(MOZ_PKG_REMOVALS)) \
 		$(if $(filter-out 0,$(MOZ_PKG_FATAL_WARNINGS)),,--ignore-errors) \
 		$(if $(MOZ_PACKAGER_MINIFY),--minify) \
-		$(if $(JARLOG_DIR),--jarlogs $(JARLOG_DIR_AB_CD)) \
+		$(if $(JARLOG_DIR),$(addprefix --jarlog ,$(wildcard $(JARLOG_FILE_AB_CD)))) \
 		$(if $(OPTIMIZEJARS),--optimizejars) \
 		$(addprefix --unify ,$(UNIFY_DIST)) \
 		$(MOZ_PKG_MANIFEST) $(DIST) $(DIST)/$(STAGEPATH)$(MOZ_PKG_DIR) \
diff --git a/toolkit/mozapps/installer/packager.py b/toolkit/mozapps/installer/packager.py
index 7cfdad3596b4a..20e420495ac5f 100644
--- a/toolkit/mozapps/installer/packager.py
+++ b/toolkit/mozapps/installer/packager.py
@@ -230,8 +230,8 @@ def main():
                         help='Transform errors into warnings.')
     parser.add_argument('--minify', action='store_true', default=False,
                         help='Make some files more compact while packaging')
-    parser.add_argument('--jarlogs', default='', help='Base directory where ' +
-                        'to find jar content access logs')
+    parser.add_argument('--jarlog', default='', help='File containing jar ' +
+                        'access logs')
     parser.add_argument('--optimizejars', action='store_true', default=False,
                         help='Enable jar optimizations')
     parser.add_argument('--unify', default='',
@@ -331,13 +331,15 @@ def main():
                                                     libname)))
 
     # Setup preloading
-    if args.jarlogs:
-        jarlogs = FileFinder(args.jarlogs)
-        for p, log in jarlogs:
-            if p.endswith('.log'):
-                p = p[:-4]
-            if copier.contains(p) and isinstance(copier[p], Jarrer):
-                copier[p].preload([l.strip() for l in log.open().readlines()])
+    if args.jarlog and os.path.exists(args.jarlog):
+        from mozpack.mozjar import JarLog
+        log = JarLog(args.jarlog)
+        for p, f in copier:
+            if not isinstance(f, Jarrer):
+                continue
+            key = JarLog.canonicalize(os.path.join(args.destination, p))
+            if key in log:
+                f.preload(log[key])
 
     # Fill startup cache
     if isinstance(formatter, OmniJarFormatter) and launcher.can_launch():
-- 
GitLab