Closed Bug 511984 Opened 10 years ago Closed 10 years ago

Enable fast-startup component for Firefox

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(status1.9.2 beta1-fixed)

RESOLVED FIXED
mozilla3.7a1
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: dietrich, Assigned: Dolske)

References

Details

(Keywords: perf, Whiteboard: [nv])

Attachments

(1 file, 2 obsolete files)

No description provided.
Whiteboard: [ts]
This isn't really general [ts], because we don't want to do this on all platforms.  This is a specific fix on platforms with currently exceptionally long startup time, like Windows CE.

Two things need to be done:

1) The patch uses toolkit.defaultChromeURI, which isn't present in firefox.  It should use browser.chromeURL, but that won't work since that's "chrome://browser/content/", and the actual window is "chrome://browser/content/browser.xul".  So the easiest thing to do would be to check for defaultChromeURI, if it's not set/present, then check for browser.chromeURI... if it's present, then use "chrome://browser/content/browser.xul".  Or we can fix chromeURL to have the full URL.

2) The faststart pieces need to be added to the packaging manifest.
Whiteboard: [ts] → [ts][nv]
Assignee: nobody → dolske
Why is it that Firefox doesn't simply use toolkit.defaultChromeURI?  FastStartup.js isn't the only code which references it.
Wait, why are you using .defaultChromeURI *or* browser.chromeURL? Neither is meant to be a general solution to anything. If you want to do "the normal thing that an application does on startup" you should dispatch a commandline with no arguments.
That -is- how dispatching command-lines works, but we use the chromeURI information to detect the opening of top-level DOM windows.  Is there a better way?
We can probably just ditch all the browser chrome URI checking entirely.
That's not a bad idea; we can probably just look at the number of visible open windows.
Attached patch Patch v.1 (obsolete) — Splinter Review
I checked with some debug output to confirm that the hidden window wasn't causing problems... It wasn't. When launching Firefox (with the faststart process already running), the observer only sees 1 domwindowopened notification, for the real Firefox window.

(FYI: this is on top of a whitespace only patch to convert DOS line endings in toolkit/components/faststart).
Attachment #401351 - Flags: review?(vladimir)
Attached patch Patch v.2 (obsolete) — Splinter Review
Oops, forgot to fix the package manifest filename.
Attachment #401351 - Attachment is obsolete: true
Attachment #401352 - Flags: review?(vladimir)
Attachment #401351 - Flags: review?(vladimir)
Comment on attachment 401352 [details] [diff] [review]
Patch v.2

Looks good, can you make the _browserWindowCount-- only happen if _browserWindowCount is > 0 though?  Dont' want it going below 0 in case some windows get opened before we start listening to window open/close.  With that, approved for 1.9.2 as well.
Attachment #401352 - Flags: review?(vladimir)
Attachment #401352 - Flags: review+
Attachment #401352 - Flags: approval1.9.2+
Attached patch Patch v.3Splinter Review
Attachment #401352 - Attachment is obsolete: true
Whiteboard: [ts][nv] → [nv]
Pushed http://hg.mozilla.org/mozilla-central/rev/be5b1c826993

Pushed to 192: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cb7990bd183f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7
(In reply to comment #10)
> (From update of attachment 401352 [details] [diff] [review])
> Looks good, can you make the _browserWindowCount-- only happen if
> _browserWindowCount is > 0 though?  Dont' want it going below 0 in case some
> windows get opened before we start listening to window open/close.  With that,
> approved for 1.9.2 as well.

Why should this be necessary?  We're initialized and listening well before the browser's first window is ever built, even in the case where we're started without -faststart-hidden
Blocks: 503483
Target Milestone: Firefox 3.7 → Firefox 3.7a1
Out of curiosity (sorry for the very very late add'l drive-by), why is so much of this wrapped in add'l #ifdef WINCE guards?  Can't we just control whether this is on or off by fixing the mozconfigs for whichever platform?  Why restrict to only WINCE?
The copious #ifdef WINCE guards are there just because the focus on getting this working was, AFAIK, entirely for WinCE/WinMo. So it serves as a warning for someone trying to blindly flip this on for OS/2 or whatever that they should probably do some investigation and testing first.
Component: Build Config → General
Product: Firefox → Firefox Build System
Keywords: perf
Target Milestone: Firefox 3.7a1 → mozilla3.7a1
Keywords: perf
You need to log in before you can comment on or make changes to this bug.