Closed
Bug 392251
Opened 17 years ago
Closed 17 years ago
Load extensions from appdir/distribution/bundles
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: hello, Assigned: benjamin)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
hello
:
review+
|
Details | Diff | Splinter Review |
The goal for these is to "future-proof" for things we might want to do but the ini format won't support.
Note that this is *not* meant to be for 'companion' add-ons, those need to end up in the user's profile.
I think we should put them in an 'extensions' directory under distribution, just to keep things tidy.
Comment 1•17 years ago
|
||
What extension manager features are these going to use (e.g. add-on update, compatibility check, enable / disable, etc.)?
Assignee | ||
Comment 2•17 years ago
|
||
None: they are going to be loaded as extension directories without any compatibility checks and unknown to the EM.
Comment 3•17 years ago
|
||
If they aren't going to use any of the EM then why not make this another chrome / components / etc. dir(s)?
Reporter | ||
Comment 4•17 years ago
|
||
That would be fine by me, FWIW. I said 'extensions' in the bug based on my conversation with Benjamin earlier today.
However, making them extensions does have the advantage that it'd be easier to drop in e.g. a search plugin.
Comment 5•17 years ago
|
||
I talked with Benjamin today about this and his plan is for it to add this to nsXREDirProvider in the same way that enabled extensions are added without these extensions ever touching the Extension Manager. The name of the dir shouldn't be extensions to avoid possible confusion with other extensions directories that are managed by the Extension Manager since people will likely assume that this new directory should behave the same way as EM managed extensions directories.
How about bundles for the name of the directory?
Reporter | ||
Comment 6•17 years ago
|
||
(In reply to comment #5)
> How about bundles for the name of the directory?
Sold. Go go go! ;-)
Assignee | ||
Comment 7•17 years ago
|
||
Attachment #276867 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 8•17 years ago
|
||
I don't think I can create a usable testcase for this until we have the XR-based test harness.
Flags: in-testsuite?
Updated•17 years ago
|
Attachment #276867 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 9•17 years ago
|
||
reassigning to dmills for actual landing, since I'm away
Assignee: benjamin → thunder
Assignee | ||
Updated•17 years ago
|
Assignee: thunder → benjamin
Assignee | ||
Comment 10•17 years ago
|
||
Fixed on trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: dev-doc-needed
Resolution: --- → FIXED
Summary: Load extensions from appdir/distribution/extensions → Load extensions from appdir/distribution/bundles
Assignee | ||
Comment 11•17 years ago
|
||
Backed out due to a Ts regression.
Assignee | ||
Comment 12•17 years ago
|
||
I found an old bug that I forgot about: we read extensions.ini many times (at least 6 on each startup) and with the first patch we were also enumerating the distribution/bundles directory 5+ times, and enumerating directories is an unpleasant Ts hit.
This patch caches the results of reading extensions.ini and enumerating the bundles directory and should improve Ts a bit, rather than hurt it.
Attachment #276867 -
Attachment is obsolete: true
Attachment #278637 -
Flags: review?(thunder)
Reporter | ||
Comment 13•17 years ago
|
||
Comment on attachment 278637 [details] [diff] [review]
Only read extensions.ini once, and only enumerate the directory once, rev. 2
Fix under XRE_EXTENSIONS_DIR_LIST:
+ LoadDirsIntoArray(mThemeDirectories,
+ kAppendNothing, directories);
Should be:
+ LoadDirsIntoArray(mExtensionDirectories,
+ kAppendNothing, directories);
Other than that, it looks ok.
Attachment #278637 -
Flags: review?(thunder) → review+
Assignee | ||
Comment 14•17 years ago
|
||
Checked in and backed out again due to Ts regression (about 3%). Other than enumerating a nonexistent directory taking 3ms, I can't think of a reason for this. I may try landing rev. 2 without the distribution/bundles chunk sometime tomorrow to see whether that has any effects.
Reporter | ||
Comment 15•17 years ago
|
||
I'm a little worried about this bug. Have you tried doing any profiling locally to see why it's spending more time? It doesn't make sense to me that the latest version is that much more expensive than what was there before.
Assignee | ||
Comment 16•17 years ago
|
||
Oh, check out the version I landed today... especially http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsXREDirProvider.cpp&branch=&root=/cvsroot&subdir=/mozilla/toolkit/xre&command=DIFF_FRAMESET&rev1=1.62&rev2=1.63 ... it introduced a behavior change where appdir/components was being registered twice. I fixed that and the Ts regression went away. I intend to remove the LOAD_DISTRO_BUNDLES ifdefs tomorrow morning.
Reporter | ||
Comment 17•17 years ago
|
||
Ah, I didn't see that go by! Excellent news, thanks :-)
Assignee | ||
Comment 18•17 years ago
|
||
Re-enabled the LOAD_DISTRO_BUNDLES code this morning, no Ts effect. yay.
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Firefox → Toolkit
Updated•15 years ago
|
Keywords: dev-doc-needed
Comment 19•15 years ago
|
||
It's not inherently obvious (to me, anyway) where these extensions are being loaded from. I even spent time reading the patch, and still couldn't figure it out.
Where in relation to the executable is the bundles directory?
Comment 20•15 years ago
|
||
(In reply to comment #19)
> It's not inherently obvious (to me, anyway) where these extensions are being
> loaded from. I even spent time reading the patch, and still couldn't figure it
> out.
>
> Where in relation to the executable is the bundles directory?
If the executable is in <appdir>/firefox.exe then the bundles dir is in <appdir>/distribution/bundles
Comment 21•15 years ago
|
||
How about on Mac? in <path to firefox-bin>/distribution/bundles?
Comment 22•15 years ago
|
||
(In reply to comment #21)
> How about on Mac? in <path to firefox-bin>/distribution/bundles?
Should be yes
Comment 23•15 years ago
|
||
This is now mentioned here:
https://developer.mozilla.org/en/Extension_Packaging#Including_add-ons_in_a_customized_application
And on the main Firefox 3.6 for developers page.
Keywords: dev-doc-needed → dev-doc-complete
Comment 24•15 years ago
|
||
(In reply to comment #23)
> This is now mentioned here:
>
> https://developer.mozilla.org/en/Extension_Packaging#Including_add-ons_in_a_customized_application
>
> And on the main Firefox 3.6 for developers page.
FWIW this feature has been there since Firefox 3.0.
Comment 25•15 years ago
|
||
Interesting. Wonder why it just came up on my list of 3.6 items. I'll revise the content.
You need to log in
before you can comment on or make changes to this bug.
Description
•