40729 - implement nsINotification

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P3)

Description

[Warren Harris]

We should implement an nsIMessage (or nsIException) object that allows the 
mapping from error codes to error messages to be deferred, while preserving the 
ability to provide error message parameters.

Here's an example to illustrate: Right now, in necko, if you get a DNS failure 
this comes back in the OnStopRequest method. OnStopRequest gets passed an 
nsresult error status code, and a const PRUnichar* string that's the error 
message. This requires necko to format a nifty little message like "Host 
www.babes.com not found" which requires necko to depend on i18n, know what the 
current language is set to etc.

Enter nsIMessage. Instead, necko could just create an nsIMessage like this:

  nsCOMPtr<nsISupportsArray> args;
  NS_NewISupportsArray(getter_AddRefs(args));
  nsCOMPtr<nsIVariant> hostParam;
  NS_NewVariant(getter_AddRefs(hostParam), kVariantString, hostName);
  args->InsertElement(hostParam);
  nsCOMPtr<nsIMessage> msg;
  NS_NewMessage(getter_AddRefs(msg), errorStatus, args);
  ...
  observer->OnStopRequest(channel, ctxt, msg);

(Yes, this depends on nsIVariant which also needs to be defined.) Then, when 
the receiver (webshell) gets the message, they can call something like this:

  nsString msgStr;
  msg->Format(msgStr, ...language info...);

which gives them a string to put in a dialog. (Note that I believe that the 
i18n utilities already provide a format utility that allows strings with %1, 
%2, etc. for positioning the parameters in the message according to the 
language in question.)

Of course, this all begs the question of how nsIMessage::Format finds the 
component in question, and the error message strings for that component? This 
relates to the discussion we had yesterday in the architecture meeting where 
Vidur suggested the need to be able to specify error constants names (and error 
messages) from JS:

  try {
    ...
  } catch (e) {
    if (e == <symbolic name for error value goes here>) {
      ...
    }
  }

The question is, how do you specify the symbolic name (and error message) for a 
given nsresult error code?

Comment 1

[Warren Harris]

Subject: 
             Re: Components Loaded for Viewer
 Resent-Date: 
             Tue, 13 Jun 2000 13:54:42 -0700 (PDT)
 Resent-From: 
             [e-mail]
        Date: 
             Tue, 13 Jun 2000 13:49:36 -0700
        From: 
             [e-mail] (Gagan Saksena)
 Organization: 
             Netscape Communications Corp.
          To: 
             Warren Harris <[e-mail]>
         CC: 
             Judson Valeski <[e-mail]>, Andy Bell 
<[e-mail]>, [e-mail], Rick Potts <[e-mail]>,
             [e-mail]
  References: 
             1 , 2 , 3




I like this idea. Here are my thoughts- 

Warren Harris wrote: 

  I was just discussing with Chris Waterson the plan Rick Potts and I had for 
necko and string bundles (and I think we can come up with a similar plan for 
prefs). The idea is to
  create an nsIMessage interface (in xpcom) that contains an nsresult status 
code and an nsISupportsArray list of parameters.

Are we going to keep this status code unique like the error codes? It would seem 
to me that they will have to be unique across the app. We may still need a 
mechanism to associate
the parameters with that status code. This will be useful when for example-- you 
get an nsIMessage that really is a progress message (say) in which case the 
status code may
uniquely be some reserved value for progress messages but the parameters reflect 
the bytes read or something like that. 

  Necko will create one of these in the following cases: 

       to eliminate the error messages passed in OnStopRequest (the caller will 
get the nsIMessage and format it using string bundles, etc.) 
       to subsume OnProgress and OnStatus notifications (we can just have a 
generic notification that gets passed an nsIMessage) 

  Once created, the message object gets returned to the caller, and the caller 
can either take programmatic action based on the status code, or format a 
message using a string
  bundle, etc.

The programmatic action could also include creating a totally new nsIMessage 
object and propogating upwards. 

  This moves string bundle usage into the main application program, and out of 
the networking library. 

  (Note that we need another name for nsIMessage since mailnews already has 
something with that name.)

How about nsINotification? 

  For prefs, we should define something in xpcom like nsIProperties but with a 
callback mechanism to observe changes. Then necko will be passed one of these 
when it gets
  initialized. That will allow other clients to pass preferences to necko 
without necko depending on nsIPref or our preferences library directly.

I agree that this would be a good way to break dependency on prefs but is that 
really desirable right now? I guess the question I am trying to ask is how finer 
do we want to break
our components given the time we have with the effort required to do that? Do 
you really see a situation where we'd be using Necko without prefs? 

-Gagan 

    
  Warren 
    

  Judson Valeski wrote: 

    Andy Bell wrote: 

    > I'm particularly keen to try to get rid of chrome and rdf, because rdf 
    > (which seems to be pulled in by chrome) is one of the biggest components, 
    > and I don't want to provide chrome support. But untangling the 
    > interdependencies in the code is a bit hair-raising. 

    this would help string bundles as well. they use chrome urls to point to 
string 
    files. thus loading a string causes rdf to be loaded :-/. 

    Jud

Comment 2

[Warren Harris]

Cc'ing Ruslan

Comment 3

[Warren Harris]

I started implementing this, and started feeling queasy about allocating an 
object for every status notification. I found out that each time around the 
socket transport loop, we send out an OnStatus notification saying what state 
we're in. Only the DNS state needs to communicate any argument information.

Now I'm thinking that rather than nsINotification, we should pass 2 parameters: 
an nsresult status code (as we do now), and an nsIStringArray containing any 
message parameters (e.g. the host name for dns). 

Note that nsIStringArray doesn't exist yet, only nsStringArray which can't be 
used from idl.

Any reason why I shouldn't do it this way?

Comment 4

[Warren Harris]

Attaching my first cut at this -- in the netwerk directory only. I haven't 
converted the whole tree because I'm afraid this might be the wrong way to go.

On the positive side, it avoids having to read in a string bundle when necko 
starts up. But on the negative side, in the error case it does allocate a 
notification object. Also, it's a little harder to use since you have to 
construct an object instead of just passing parameters in the argument list 
(e.g. of OnStopRequest or OnStatus).

Let me know if you like this (Rick, Ruslan, Gagan), and I'll finish the project.

Comment 5

[Warren Harris]

Attachment: xpcom/ds/nsINotification.idl

Comment 6

[Warren Harris]

Attachment: xpcom/ds/nsNotification.h

Comment 7

[Warren Harris]

Attachment: xpcom/ds/nsNotification.cpp

Comment 8

[Warren Harris]

Attachment: diffs for mozilla/netwerk (only)

Comment 9

[Warren Harris]

Needed for status messages to work properly.

Comment 10

[Warren Harris]

In the end we decided not to implement an actual nsINotification object since 
that would entail another allocation for every status message. Instead, we're 
passing the nsresult status code and a string representing any status message 
parameters. The caller formats the message (using the new 
nsIStringBundleService::FormatStatusMessage entry point).

It's in.

Comment 11

[Tracy Walker [:tracy]]

please verify

Comment 12

[leger]

warren, please mark verified is all is well.

Comment 13

[Warren Harris]

asdf