881667 - Expose initialization/shutdown stage to components

Status: RESOLVED FIXED

(Toolkit :: Startup and Profile System, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

Attachment: Possible API

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.

Comment 1

[(no longer active)]

Hmm, why can't you just listen to the notification that interests you?  This seems possible to implement on top of an observer.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

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.

Comment 3

[Trevor Saunders (:tbsaunde)]

(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 4

[Trevor Saunders (:tbsaunde)]

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)

Comment 5

[David Teller [:Yoric] - still alive but not very active]

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.

Comment 6

[(no longer active)]

(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.  :-)

Comment 7

[Benjamin Smedberg]

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?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Note that a JS version would be useful right now for bug 921157.

Comment 9

[David Teller [:Yoric] - still alive but not very active]

Attachment: Exposing nsIAppStartup::startingUp

As discussed with bsmedberg over irc, it should be sufficient to provide nsIAppStartup::startingUp and to fix bug 922814.

Comment 10

[Andrew McCreight [:mccr8]]

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.

Comment 11

[Benjamin Smedberg]

I need to review this. It's in the wrong component anyway.

Comment 12

[David Teller [:Yoric] - still alive but not very active]

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.

Comment 13

[Benjamin Smedberg]

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.

Comment 14

[David Teller [:Yoric] - still alive but not very active]

Attachment: Exposing nsIAppStartup::startingUp, v2

Applied feedback
Try: https://tbpl.mozilla.org/?tree=Try&rev=106e931b9c2b

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/b4f347a62071

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4f347a62071