xpcom is unable to always load .js component file

VERIFIED FIXED

Status

()

Core
XPCOM
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Sean Su, Assigned: dougt)

Tracking

Trunk
x86
Windows 2000
Points:
---
Bug Flags:
blocking1.3b +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

15.80 KB, patch
Sean Su
: review+
Alec Flett
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
Seth and I found a problem with xpcom trying to load a .js component from the
mozilla's component dir (as opposed to the GRE's component dir).

We were trying to fix bug 190465, and noticed that nsDownloadProgressListener.js
would only load consistently when it was located in the GRE's component dir.

If it was in the mozilla's component dir.  It would only load once, then the
feature would no longer work if you quit and restarted the browser.  Even this
one time was not always reproduceable.

Both seth and I were experiencing this, so I doubt it's something with my system
in particular.

Seth tested out other features that had .js components in the mozilla's
component dir.  They seem to have loaded fine.  So I'm not sure exactly what's
going on here.
(Assignee)

Comment 1

16 years ago
nor am I.

how are you registering this .js file?  
Not the registry, just XPCOM
Component: XPCOM Registry → XPCOM
(Assignee)

Comment 3

16 years ago
note that dan's comment was about the "component" attribute of this bug.
(Assignee)

Comment 4

16 years ago
this works for me.  

ssu/seth, can you reproduce this problem for me on monday?  If this is a
problem, I wanna know right away.
Flags: blocking1.3b?
(Reporter)

Comment 5

16 years ago
here are the steps with a new build that *contains* the fix to bug 190465:
1) make sure mozilla and GRE are not installed
2) install mozilla (default settings is what I used)
3) notice that doing a "File|Save Page As" (and actually saving the file) works.
4) quit mozilla when it runs at the end of the install
5) delete [mozilla]\components\*.dat
6) *move* the following files from [gre 1.3b]\components to [mozilla]\components:
     nsProgressDialog.js
     nsHelperAppDlg.js
     nsDownloadProgressListener.js
7) start mozilla
8) notice that doing a "File|Save Page As" (and actually saving the file) does
*not* work.

Dougt, if the steps above still do not reproduce this bug, I can swing by your
cube and help you reproduce this.
(Assignee)

Comment 6

16 years ago
where is xpc3250 in the install?  I do not see this file in any of the
compreg.dat files ssu has sent me via email.
(Assignee)

Comment 7

16 years ago
ignore that last comment.  
(Assignee)

Comment 8

16 years ago
duh!  found the problem.  
(Assignee)

Comment 9

16 years ago
Created attachment 112836 [details] [diff] [review]
patch v.1

Keep track of the component loaders.  If there is an additional component
loader added during the registration of gre components, we autoreg the
application components directory again (for the new non-native loaders only).  


I am pretty sure that this works.  ssu, can you give it a swing?
(Assignee)

Updated

16 years ago
Attachment #112836 - Flags: superreview?(alecf)
Attachment #112836 - Flags: review?(ssu)

Comment 10

16 years ago
Comment on attachment 112836 [details] [diff] [review]
patch v.1

generally this looks fine, but can we keep the DEBUG_dougt stuff? I think we
have enough registration spew already.
Also, I see lots of NS_COM/NS_EXPORT stuff - is that intentional? necessary?

sr=alecf with the DEBUG_dougt left, and an explanation for the NS_EXPORT stuff
Attachment #112836 - Flags: superreview?(alecf) → superreview+
(Assignee)

Comment 11

16 years ago
Comment on attachment 112836 [details] [diff] [review]
patch v.1

okay to the printf's.

Regarding the NS_EXPORT stuff, the function that I am removing this from isn't
used outside of XPCOM.	There is no reason to export it, nor (i guess) no real
reason include it with this bug.  It was just in my tree at the time I took the
diff.  If it is all the same, I would like to check this part in too.

Updated

16 years ago
Flags: blocking1.3b? → blocking1.3b+
(Reporter)

Comment 12

16 years ago
Comment on attachment 112836 [details] [diff] [review]
patch v.1

xpinstall now errors out from AutoRegisterComponentByLoaderIndex().

It was not going into the for() loop.  fromLoaderType is 0.  mNLoaderData is 1.
Attachment #112836 - Flags: review?(ssu) → review-
(Assignee)

Comment 13

16 years ago
Comment on attachment 112836 [details] [diff] [review]
patch v.1

grumble.... wrong patch.
Attachment #112836 - Attachment is obsolete: true
(Assignee)

Comment 14

16 years ago
Created attachment 112864 [details] [diff] [review]
patch v.2

this is the real deal.
(Assignee)

Comment 15

16 years ago
Comment on attachment 112864 [details] [diff] [review]
patch v.2

sorry about that, the only different between the two patchs is alecf's
suggestion regarding the printf, and i made that AutoRegisterByLoaderIndex more
encapsulated by not exposing the loader id in the method and called this new
method AutoRegisterNonNativeComponents.  This makes more sense.
Attachment #112864 - Flags: superreview?(alecf)
Attachment #112864 - Flags: review?(ssu)

Comment 16

16 years ago
Comment on attachment 112864 [details] [diff] [review]
patch v.2

>+    // registers only the files in spec's location by loaders other than the
>+    // native loader.  This is an optimization method only.
>+    nsresult AutoRegisterNonNativeComponents(nsIFile* spec);
the comment is wrong: if spec is null you intentionally check some other
directory :)

Comment 17

16 years ago
Comment on attachment 112864 [details] [diff] [review]
patch v.2

sr=alecf
Attachment #112864 - Flags: superreview?(alecf) → superreview+
Comment on attachment 112864 [details] [diff] [review]
patch v.2

Approval granted with the proviso that you need to get r= from ssu (or someone
else up on what's going on here with GRE/etc).
Attachment #112864 - Flags: approval1.3b+
(Reporter)

Comment 19

16 years ago
Comment on attachment 112864 [details] [diff] [review]
patch v.2

still not working for xpinstall being triggered via xpistub.dll.

I'll talk with you off line to help you reproduce this.
Attachment #112864 - Flags: review?(ssu) → review-
(Assignee)

Comment 20

16 years ago
ssu - why are you missing this patch on some error that you are seeing in a
totally different area?  
(Reporter)

Comment 21

16 years ago
because xpinstall requires xpcom, and xpinstall is used by the native installer.
(Assignee)

Comment 22

16 years ago
never mind... ssu found the problem.... i shouldn't have default the return
result to NS_ERROR_UNEXCEPTED.  ssu say r=
(Reporter)

Comment 23

16 years ago
r=ssu for changing the default return value.
(Reporter)

Comment 24

16 years ago
Comment on attachment 112864 [details] [diff] [review]
patch v.2

contingent on changing the default return value to NS_OK per our talk.
Attachment #112864 - Flags: review- → review+
(Assignee)

Comment 25

16 years ago
Thanks everyone.

Checking in components/nsCategoryManager.cpp;
/cvsroot/mozilla/xpcom/components/nsCategoryManager.cpp,v  <-- 
nsCategoryManager.cpp
new revision: 1.36; previous revision: 1.35
done
Checking in components/nsComponentManager.cpp;
/cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v  <-- 
nsComponentManager.cpp
new revision: 1.219; previous revision: 1.218
done
Checking in components/nsComponentManager.h;
/cvsroot/mozilla/xpcom/components/nsComponentManager.h,v  <--  nsComponentManager.h
new revision: 1.78; previous revision: 1.77
done
Checking in components/nsRegistry.cpp;
/cvsroot/mozilla/xpcom/components/nsRegistry.cpp,v  <--  nsRegistry.cpp
new revision: 1.72; previous revision: 1.71
done
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 26

16 years ago
this fix seems to be working.  I verified by doing a manual fix of bug 191048. 
the download manager dialog shows up and works after I move
nsDownloadProgressListener.js from GRE to Mozilla.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.