467464 - Remove support for loading XPT information from ZIPs and JARs

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[Benjamin Smedberg]

I've been examining the xpt loading time and process; I found to my surprise that we have code to load XPT files from JARs and ZIP files. I'm sure we don't actually use this code, and I bet that it doesn't work at all because of load ordering. As prep for futher work I'd like to just remove this code.

Comment 1

[Benjamin Smedberg]

Attachment: Remove support for loading XPTs out of ZIPs and JARs, rev. 1

Comment 2

[timeless]

so. i came to the conclusion that it didn't work last night.

however mobile is barking up a bad tree, and having this work seems like a better tree than the one they're barking up.

Comment 3

[Benjamin Smedberg]

I don't think so... of course the current xpti.dat + xptiloader is inefficient, but I think we can make it efficient without resorting to a JAR abstraction.

Comment 4

[Alfred Kayser]

What are the current plans for this?
Either we make libjar simpler by removing this XPT.zip support, 
or we make XPT loading more efficient by packaging them into a 'zip' file?

Comment 5

[(not currently active) Ted Mielczarek]

We should remove it. All of our shipping applications link their XPT files into a single XPT anyway.

Comment 6

[Dão Gottwald [:dao]]

The review request in this bug lacks a requestee.

Comment 7

[Alfred Kayser]

If needed and acceptable, I can do the review (as contributor to libjar and friends).

Some functions can probably be simplified, e.g.:
817 class xptiFileType
818 {
819 public:
820     enum Type {UNKNOWN = -1, XPT = 0, ZIP = 1 };
821 
822     static Type GetType(const char* name);
823 
824     static PRBool IsUnknown(const char* name)
825         {return GetType(name) == UNKNOWN;}
826 
827     static PRBool IsXPT(const char* name)
828         {return GetType(name) == XPT;}
829 
830     static PRBool IsZip(const char* name)
831         {return GetType(name) == ZIP;}
832 private:
833     xptiFileType(); // no implementation
834 };
For just doing IsXPT(filename), which is just strcasecmp(.xpt).

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 350892 [details] [diff] [review]
Remove support for loading XPTs out of ZIPs and JARs, rev. 1

I'm fine with it from an architectural perspective.

Comment 9

[Benjamin Smedberg]

This work has been subsumed into bug 568691.

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 350892 [details] [diff] [review]
Remove support for loading XPTs out of ZIPs and JARs, rev. 1

Clearing review requests on a patch from bsmedberg and sr+'d by shaver.

Career limiting move, or cleaning up requests on old bugs? You be the judge. :) Please reopen or refile if this is still somehow relevant.