Closed Bug 507288 Opened 15 years ago Closed 15 years ago

move greprefs/*js into greprefs.js

Categories

(Toolkit :: Preferences, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ts])

Attachments

(2 files, 9 obsolete files)

      No description provided.
Note: these are not JS files, even though they appear to be. They are read by the prefservice using file I/O and a directory enumerator.

You can't do this without adding some serious logic to prefservice to read prefs from JARs or channels.
Depends on: 508421
It seems that all the code does is read the files in and combine them on startup. Would it not make sense to do the combining step at build-time?

Then we could move all.js(including the other files) into res/ and get rid of that readdir.
That would work for xulrunner, yeah. For the app prefs you'd either need a well-known filename or some other way of knowing where the prefs are...
Whiteboard: [ts]
So to clarify, we can move app prefs into a single .js file, into jar.
Summary: move greprefs/*js into res/ so those files can be jarred → move greprefs/*js into res/greprefs.js so those files can be jarred
this runs ~2-10x faster
Assignee: nobody → tglek
Attached patch greprefs.js (obsolete) — Splinter Review
Attachment #395179 - Attachment is obsolete: true
Attachment #398035 - Flags: review?(benjamin)
Blocks: 513027
Comment on attachment 398035 [details] [diff] [review]
greprefs.js

>diff --git a/modules/libpref/src/nsPrefService.cpp b/modules/libpref/src/nsPrefService.cpp

>+static nsresult openPrefInputStream(nsIInputStream *aInStr) {

style:
static nsresult
openPrefInputStream(nsIInputStream* aInStr)
{

same thing for openPrefFile below.

At first glance it appears that security-prefs.js and xpinstall.js are just being skipped altogether. What's up with that?
Attachment #398035 - Flags: review?(benjamin) → review-
Depends on: 514299
the preprocessor solution is much nicer, thanks for suggesting that on irc.
Attachment #398035 - Attachment is obsolete: true
Attachment #398248 - Flags: review?(benjamin)
Flags: in-testsuite-
Attachment #398248 - Flags: review?(benjamin) → review+
No longer depends on: 514299
slashslash busts strings with // in them. This is ready to checkin
Attachment #398248 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/40811b076d51
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
backed out, since this caused new orange, e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1252853301.1252854535.8760.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
What is the impact of this change on locking prefs?
See http://kb.mozillazine.org/Locking_preferences

A long time ago it was advised to modify all.js , but that would cause problems with updates.
So currently it is advised to create an extra file local-settings.js in the greprefs folder to store the pref: 
pref("general.config.filename", "mozilla.cfg");

Will *.js files in greprefs still be read or is there another solution possible that won't require to modify Firefox files?
Looks like we got a chicken/egg problem. We need to access greprefs.js via a url, but in order to even make a url, we need nsIOService, which needs prefs.....which is bad.

Unless someone has a bright idea on how to read stuff out of a jar without chicken/egg issues(and work with flat chrome), I'm all ears.
Doing stuff with files is much uglier than with stuff in jar.mn. 

Benjamin or Ted, for some reason $(DESTDIR) is not is not being set in my makefile code.

I get echo "" out of that echo line, what am I doing wrong?
Attachment #399333 - Attachment is obsolete: true
Attached patch bin/greprefs.js (obsolete) — Splinter Review
Attachment #401074 - Attachment is obsolete: true
Attachment #401098 - Flags: review?(benjamin)
Summary: move greprefs/*js into res/greprefs.js so those files can be jarred → move greprefs/*js into greprefs.js
Attached patch bin/greprefs.js (obsolete) — Splinter Review
forgot to preprocess the prefs in last rev.
Attachment #401098 - Attachment is obsolete: true
Attachment #401316 - Flags: review?(benjamin)
Attachment #401098 - Flags: review?(benjamin)
You'll need to clean up the three deprecated files in removed-files.in won't you ?
Comment on attachment 401316 [details] [diff] [review]
bin/greprefs.js

>diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in
>--- a/browser/installer/package-manifest.in
>+++ b/browser/installer/package-manifest.in
>@@ -381,9 +381,7 @@
> @BINPATH@/@PREF_DIR@/firefox.js
> @BINPATH@/@PREF_DIR@/firefox-branding.js
> @BINPATH@/@PREF_DIR@/channel-prefs.js
>-@BINPATH@/greprefs/all.js
>-@BINPATH@/greprefs/security-prefs.js
>-@BINPATH@/greprefs/xpinstall.js
>+@BINPATH@/greprefs.js

You made changes here but didn't add anything to removed-files...

>diff --git a/modules/libpref/src/Makefile.in b/modules/libpref/src/Makefile.in

>+greprefs.js: $(topsrcdir)/xpinstall/public/xpinstall.js $(topsrcdir)/netwerk/base/public/security-prefs.js $(srcdir)/init/all.js

Can you make a GREPREF_FILES variable for this? I'd really like to split all.js into more pieces across the tree.

r=me with nits fixed
Attachment #401316 - Flags: review?(benjamin) → review+
Attached patch addressed review comments (obsolete) — Splinter Review
Fixed nits. carrying over Benjamins review
Attachment #401316 - Attachment is obsolete: true
Attachment #403893 - Flags: review+
Keywords: checkin-needed
Attached patch addressed review comments (obsolete) — Splinter Review
This is the patch i meant to upload
Attachment #403893 - Attachment is obsolete: true
Attachment #403898 - Flags: review+
Accidentally attached wrong file above[again]
Attachment #403898 - Attachment is obsolete: true
Attachment #403899 - Flags: review+
OS: Linux → All
Hardware: x86 → All
http://hg.mozilla.org/mozilla-central/rev/0e8fae2dd64c
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Depends on: 521192
No longer blocks: 521004
Depends on: 521004
This bug has screwed up the javascript instrumentation logic for nightly instrumented builds.

Please notify catlee and murali whenever *.js files from defaults and greprefs are moved to other locations.
Blocks: 528954
Attachment 403899 [details] [diff] added the three old greprefs files to removed-files.in but in a Mac specific section. This just moves to affect all platforms.
Attachment #448314 - Flags: review?(ted.mielczarek)
Attachment #448314 - Flags: review?(ted.mielczarek) → review+
You need to log in before you can comment on or make changes to this bug.