400887 - Audit XPTs and Gecko components

Status: RESOLVED INCOMPLETE

(Camino Graveyard :: General, defect)

Description

[Mark Mentovai]

We're not packaging all of the typelib files we should.  We ran into this in bug 371860.  For 1.6 and the trunk, we should do an audit of our XPT files, and package the ones that we need but are currently missing.  We might also want to rethink how we do XPTs, or implement some sort of checking, so that this stops happening to us.

Comment 1

[Mark Mentovai]

We may want to pre-build camino.xpt (with xpt_link) and have the project file depend only on that.  There's nothing you can do with individual typelib files from within Xcode, and we xpt_link this stuff together after the xcodebuild ordinarily (which is actually kind of a huge hack.)

This means that we'll need to maintain the list of desired typelibs somewhere outside of the xcodeproj (like the Makefile), but that might not be so bad if we, say, take all of the platform XPTs and subtract what we don't want/can't use.

Just an idea.

Comment 2

[Mark Mentovai]

Bug 371860 and bug 400302 fixed this for a few typelib files for Camino 1.5.3.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I don't want to start spamming this bug or setting false dependencies, but I'd also like to find a good way to collect puzzling web-content bugs that we should check during/after the XPT audit to see if they're also XPT-related.

Bug 379564, Bug 336156

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I don't know if we want to try this--at least the audit part--for a1 or just get to it quickly thereafter....

Comment 6

[Stuart Morgan]

As of today, this is the list of xpts we are not shipping in trunk that are available in dist/.

alerts.xpt
appshell.xpt
appstartup.xpt
autocomplete.xpt
camino.xpt
chardet.xpt
chrome.xpt
commandhandler.xpt
content_html.xpt
content_htmldoc.xpt
content_xmldoc.xpt
content_xtf.xpt
contentprefs.xpt
cookie.xpt
directory.xpt
dom_loadsave.xpt
dom_sidebar.xpt
dom_storage.xpt
downloadmanager.xpt
editor.xpt
extensions.xpt
find.xpt
history.xpt
htmlparser.xpt
imgicon.xpt
inspector.xpt
locale.xpt
lwbrk.xpt
mozbrwsr.xpt
mozfind.xpt
necko_cookie.xpt
oji.xpt
pippki.xpt
plugin.xpt
prefetch.xpt
profile.xpt
proxyObjInst.xpt
related.xpt
saxparser.xpt
urlformatter.xpt
webbrowserpersist.xpt
webshell_idls.xpt
widget_cocoa.xpt
windowds.xpt
xpautocomplete.xpt
xpcom_system.xpt
xpinstall.xpt
xultmpl.xpt
zipwriter.xpt

We know we need everything from bug 371860's patch (and I really think we should add them before a1, since having a regression from 1.5.x to 1.6a1 is undesireable); mento, how would we go about figuring out what else we want?

Comment 7

[Stuart Morgan]

In the spirit of "make this stop happening", should we think about doing the xpt copy with a shell script that snags all the .xpt files in dist/Embed/components, then deletes any that we explicitly say we *don't* want? I'd rather make Camino 10 or 20k bigger than keep having these problems.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

That's more-or-less what mento proposed in comment 1; I like the idea of the project only having to deal with one camino.xpt instead of 30 individual ones which we copy in non-static and xpt_link in static.

Comment 9

[Mark Mentovai]

I agree, I'm leaning towards something along the lines of what I proposed in comment 1, and I should have something later today or tomorrow.

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

We also need to do components here; see bug 402243.

Comment 12

[Mark Mentovai]

Attachment: Results of Camino 1.5.x xpt/component audit

These are all of the XPTs and components in Camino 1.5.x, taken from the current CAMINO_1_5_BRANCH, so they include the fixes for bugs 371860, 400302, and 402243.  Anything marked with a - is built but not shipped with Camino.  The first section includes XPTs, and I've verified that it's the same in our static and nonstatic builds.  The second section includes component libraries included in the nonstatic build.  The third section includes static components in the static build.  The static build also includes three dynamically loaded bundle components on this branch: libmyspell.dylib and libspellchecker.dylib are in Camino, and libxpinstall.dylib is not.  I've also verified that the static and nonstatic component lists agree with each other.

The results of this audit are that there's not presently anything DOM-accessible that we're missing, except dom_sidebar.xpt, which we purposely skipped.  There are GRE-packaged items that extensions would have access to, but I don't care about those for the 1.5.x series of releases.  I really only care about broken content crap, and it looks like we're covered on that front.

For 1.6 and the trunk, we should think about taking modules (XPTs and binary components) that are included in the GRE, and weigh the bloat against the advantages of maybe supporting a couple of extra extensions.  But to cover web content, we should be covered if we simply fix 371860, 400302, 402243 by adding the missing files.  (This assumes that the trunk didn't add more junk we aren't yet bundling.)

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #12)
> The results of this audit are that there's not presently anything
> DOM-accessible that we're missing, except dom_sidebar.xpt, which we purposely
> skipped.  There are GRE-packaged items that extensions would have access to,
> but I don't care about those for the 1.5.x series of releases.  I really only
> care about broken content crap, and it looks like we're covered on that front.

I mentioned this to mento in channel, but I'll repeat it here for posterity: for 1.6 and beyond, I'm also concerned with stuff that our Gecko-based app-components depend on (e.g., FAYT, crazy security bugs, etc.), so I do think we need to carefully weigh taking modules (XPTs and binary components) that are included in the GRE.

I think what blocks 1.6a1 is only parity with 1.5.4, though (and perhaps project changes to depend on camino.xpt instead of multiple xpts, if that's "easy" enough).

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Er, officially minusing this for 1.5.4.

In the next few days, I'll forward-port mento's and my existing 1_5 xpt/component fixes to the 18branch and trunk so that we can land them for 1.6a1 soon.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: trunk patch

Trunk was fairly easy, as the heavy lifting of mozStorage was done earlier in the year in bug 385074.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: 18branch patch

Some notes here:

1) The Camino.app change is actually Xcode undoing the spurious change there that happened with the Sparkle landing.

2) Why does Xcode now try to "helpfully" add library search paths that already are in the library search paths, and insert *2* ugly lines per buildstyle when doing so?  :/ I went back and removed those six spurious lines.

Comment 17

[Mark Mentovai]

Comment on attachment 291018 [details] [diff] [review]
trunk patch

Looks good.

In both of these patches, it looks like there are a couple of lists that you kinda-sorta kept sorted but not really.  I know that they're a mess as it is, but is there any reason that you put content_xmldoc between content_base and content_html in the last group in this patch?  That part of that list was mostly in good shape.  The same pattern crops up a few more times in the branch patch.

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: trunk patch, as checked in

(In reply to comment #17)
> but is there any reason that you put content_xmldoc between content_base and
> content_html in the last group in this patch?  That part of that list was
> mostly in good shape.  The same pattern crops up a few more times in the branch
> patch.

Oops, that's me not paying close enough attention to the strange sorting order Xcode used when adding files to the project and adding them to/moving them between copy phases :(

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: 18branch patch, as checked in

Comment 20

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Ladies and gentle-geckos, we are code complete for Camino 1.6a1.

Leaving this bug open for remaining work to take care of comment 1 and the last pgh of comment 12.

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Er, and the rest of the work on this bug should block 1.6 final.

Comment 22

[Eiichi]

Landing an attachment (id=291048) breaks the Local News section of the www.cnn.com page.
The Local News section displays correctly with 12/1/07 branch build.
But the Local News section of the www.cnn.com page is with 12/2/07 branch build.

Comment 23

[Eiichi]

Attachment: Screenshot; latest-1.6-M1.8 12/1/07

Comment 24

[Eiichi]

Attachment: Screenshot; tinderbox-builds/cb-xserve01-1.6-M1.8 12/2/07

Comment 25

[Eiichi]

Sorry, I missed Comment #22.
The Local News section of the www.cnn.com page is blank with 12/2/07 branch build.

Comment 26

[philippe (part-time)]

(In reply to comment #22)
> Landing an attachment (id=291048) [details] breaks the Local News section of the
> www.cnn.com page.
> The Local News section displays correctly with 12/1/07 branch build.
> But the Local News section of the www.cnn.com page is with 12/2/07 branch
> build.

It displays the same on Camino trunk (2hours old build), Minefield(2007120118), BonEcho(20071130): empty (as seen in attachment 291060 [details]).

Comment 27

[philippe (part-time)]

Eiichi,
Do you have cookies for cnn.com blocked ?
If so, what you see is bug 389002.

Comment 28

[Eiichi]

(In reply to comment #27)
> Eiichi,
> Do you have cookies for cnn.com blocked ?
> If so, what you see is bug 389002.

Yes, I've blocked cookies for cnn.com.
And I accept cookies for cnn.com, then the Local News section displays correctly.
Thanks philippe.

Comment 29

[Eiichi]

One more thing, when I open http://scragz.com/tech/mozilla/test-rendering-time.php Console logs as below with latest-1.6-M1.8 12/2/07.
Console logs nothing  with latest-1.6-M1.8 12/1/07.

Camino[26006] *** WARNING: Method runModalForWindow:relativeToWindow: in class NSApplication is obsolete and will be removed in release GM ***

Comment 30

[Eiichi]

Sorry again.
I've forgotten Bug 221730.
I tested latest-1.6-M1.8 12/2/07 on Desktop.
I run 12/2/07 in /Applications, then Console logs nothing.

Comment 31

[Stuart Morgan]

I talked to mento, and we're going to call this good for 1.6 rather than add churn at this point in the cycle. We'll take a look for 2.0.

Comment 32

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Toggling flags per comment 31.

Comment 33

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

While working on bug 488949 I hit JS exceptions stemming from XPCOM invalid IID errors for 

@mozilla.org/xre/app-info;1
@mozilla.org/intl/scriptabledateformat;1

The former is XULAppInfo (xpcom_system.xpt), the latter is nsIScriptableDateFormat (locale.xpt).

ChimericalConsole++

Comment 34

[Stuart Morgan]

Too late to do this for 2.0; we should still do a sweep at some point though.