Closed Bug 881667 Opened 6 years ago Closed 6 years ago

Expose initialization/shutdown stage to components

Categories

(Toolkit :: Startup and Profile System, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: Yoric, Assigned: Yoric)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, Whiteboard: [Async][Async Shutdown])

Attachments

(1 file, 2 obsolete files)

Attached file Possible API (obsolete) —
Bug 845190 is an example of a bug in which we crash because we find ourselves instantiating a component during the shutdown process.

I believe that the best way to solve this issue is to expose somewhere the current stage of initialization/shutdown.

Attaching a possible API for this.
Attachment #760860 - Flags: feedback?(benjamin)
Attachment #760860 - Attachment mime type: text/x-idl → text/plain
Hmm, why can't you just listen to the notification that interests you?  This seems possible to implement on top of an observer.
The issue that causes bug 845190 is actually a component that is (sometimes) instantiated after the notification has been sent, so it's too late to do anything.

Now, of course, this API could easily be implemented as an observer.
(In reply to David Rajchenbach Teller [:Yoric] from comment #2)
> The issue that causes bug 845190 is actually a component that is (sometimes)
> instantiated after the notification has been sent, so it's too late to do
> anything.

hm, so the problem there is that the standard gOnceAliveNowDead thing doesn't work because in some odd testing env the stream  transport service is never started.  I'm not sure it wouldn't be a good idea to just always bring it up fairly late in startup but lets not worry about that now.
Comment on attachment 760860 [details]
Possible API

>interface mozIRing {

why do you need an interface for this? a global enum / bool 
 seems like it would do just fine.

(comment 3 was  me trying to say I can't think of a way to avoid storing this state)
Global enum does it for C++. I imagine that it might be useful for JS, too, but I'm willing to start with just a C++ version.
(In reply to comment #5)
> Global enum does it for C++. I imagine that it might be useful for JS, too, but
> I'm willing to start with just a C++ version.

We should not add APIs just because they might some day be useful for JS.  :-)
1) I think this is good in theory
2) I do not think that this should start as a JS API, because it will be much harder to change once it is exposed to external code. NS_GetRunState is fine for now.
3) There should be a state before XPCOM_STARTUP
4) Remove SESSIONRESTORE_WINDOWS_RESTORED
5) I'm not convinced that any of QUIT_APPLICATION_REQUESTED/QUIT_APPLICATION_GRANTED/QUIT_APPLICATION should be added... their loopy semantics don't quite match the rest of anything. And with those removed, you can safely assert that the "next" runstate is always greater than the previous one.
6) But the xpcom-shutdown-threads state probably should be added.
7) Oh how I hate xpcom-will-shutdown, can we remove it?
Attachment #760860 - Flags: feedback?(benjamin)
Note that a JS version would be useful right now for bug 921157.
As discussed with bsmedberg over irc, it should be sufficient to provide nsIAppStartup::startingUp and to fix bug 922814.
Assignee: nobody → dteller
Attachment #760860 - Attachment is obsolete: true
Attachment #812808 - Flags: review?(justin.lebar+bug)
Blocks: 921157
Depends on: 922814
Attachment #812808 - Flags: review?(justin.lebar+bug) → review?(continuation)
I'm not an XPCOM peer.  I can try to figure out what is going on here, or bsmedberg can review it, depending on what he thinks makes sense.
Flags: needinfo?(benjamin)
I need to review this. It's in the wrong component anyway.
Component: XPCOM → Startup and Profile System
Flags: needinfo?(benjamin)
Product: Core → Toolkit
Attachment #812808 - Flags: review?(continuation)
Comment on attachment 812808 [details] [diff] [review]
Exposing nsIAppStartup::startingUp

Here is one possible implementation.
I wanted to ensure that startingUp is available exactly once final-ui-startup is completed, which complicates slightly the code.
Attachment #812808 - Flags: review?(benjamin)
Whiteboard: [Async][Async Shutdown]
Comment on attachment 812808 [details] [diff] [review]
Exposing nsIAppStartup::startingUp

+    [noscript] void setStartingUp(in boolean aValue);

Given the semantics, this should probably instead be

  [noscript] void doneStartingUp();

Also add a comment explaining that this is called from nsAppRunner.cpp and should not be called from elsewhere.

In the impl, assert that mStartupTime is true when the method is called (to verify that it is only called once).

r=me with those changes.
Attachment #812808 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/integration/fx-team/rev/b4f347a62071
Keywords: checkin-needed
Whiteboard: [Async][Async Shutdown] → [Async][Async Shutdown][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/b4f347a62071
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Async][Async Shutdown][fixed-in-fx-team] → [Async][Async Shutdown]
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.