Closed Bug 157136 Opened 22 years ago Closed 22 years ago

Freeze core component scripting interfaces

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: dougt, Assigned: dougt)

References

Details

Attachments

(4 files, 3 obsolete files)

Interfaces
  mozIJSSubScriptLoader
    Load supscripts
  nsIScriptableInputStream
    Require to use NECKO.
  nsIScriptableTimer
    Access timers.
Blocks: 70229
Blocks: 157137
Comment on attachment 92673 [details] [diff] [review]
Freezes nsIScriptableInputStream

>+ * nsIScriptableInputStream provides access to the nsIInputStream.  For example,
>+ * one can wrap a nsIInputStream passed on a OnDataAvailable and  be able to 
>+ * |read| from the stream into a string object.

how about this rewording:

  nsIScriptableInputStream provides Javascript code with a method of reading
  from a nsIInputStream (note: nsIInputStream::read is non-scriptable).

mentioning OnDataAvailable is a little awkward here since OnDataAvailable
is a necko method name.  someone reading through xpcom interfaces wouldn't
necessarily be familiar with necko.

also, i wonder if we should comment about the wierdness of returning binary
data as a null-terminated wide-character array ;)  i.e., the result returned
from |read| is not necessarily ASCII.

sr=darin with or without these documentation changes (the interface itself
looks freezable).
Attachment #92673 - Flags: superreview+
QA Contact: mdunn → depstein
Comment on attachment 92673 [details] [diff] [review]
Freezes nsIScriptableInputStream

r=rginda
Attachment #92673 - Flags: review+
Checking in nsIScriptableInputStream.idl;
/cvsroot/mozilla/xpcom/io/nsIScriptableInputStream.idl,v  <-- 
nsIScriptableInputStream.idl
new revision: 1.3; previous revision: 1.2
done


one down.
samir, i want to free a scriptable timer interface. your comments in the
nsIScriptableTimer suggests wanted this interface only temporary.  What is wrong
with just using your interface for our frozen embedding timer interface? 
Assuming of course, that getting a timer via the nsIOberver interface is ok.
Doug, 
I think the idea was that nsITimer would be made scriptable.  The original goal
was to have this happen for 1.0.  I consulted pavlov about this prior to 1.0 and
he said it was OK for me to create the temporary nsIScriptableTimer interface in
lieu of nsITimer not being scriptable and that converting nsITimer to be
scriptable was on his list of 1.0 tasks.  You may want to consult Brendan about
freezing this.
Comment on attachment 94939 [details] [diff] [review]
Freezes mozIJSSubScriptLoader.idl

OK by me, r=rginda
Attachment #94939 - Flags: review+
Comment on attachment 94939 [details] [diff] [review]
Freezes mozIJSSubScriptLoader.idl

the comments don't match the interface - looks like rob intended to eventually
have some other object there?
Comment on attachment 94939 [details] [diff] [review]
Freezes mozIJSSubScriptLoader.idl

ok now I see rob seems to be fine with it :)

anyway, sr=alecf if you clean up the comments.
Attachment #94939 - Flags: superreview+
alecf: there's that available() which goes nicely with count() ;-)

wasn't there a discussion about not freezing interfaces that used string/wstring?
timeless, there is no fixed rule about which strings to use.  
alecf: there is nothing wrong with the comment in mozIJSSubScriptLoader.idl. 
The implementation uses xpconnect magic to return a native jsval as a return
value, and to allow for an optional jsval as a second parameter.
patch 94939 checked in to trunk.  
Attached patch Freezes nsIScriptableTimer (obsolete) — Splinter Review
Freezes nsIScriptableTimer as-is.
Comment on attachment 96720 [details] [diff] [review]
Freezes nsIScriptableTimer

woah! Why aren't we just making nsITimer scriptable? Why must "Scriptable"
appear in this interface name?

I strongly object to this patch! There is some history too, which dougt
probably wasn't aware of: Months ago, samir came to me asking what he should do
about a timer that was scriptable. I told him to make nsITimer scriptable, and
change the few consumers who might be using nsITimer. He said that sounded like
a lot of work and I said that was part of the deal when you needed to change
core infrastructure. He walked away seeming quite reluctant about the whole
thing.

A week or so later, he went and implemented nsIScriptableTimer, and never asked
for my review or at least a design review - I didn't see the results until the
patch had landed.

I'm severly disappointed with how this whole thing was handled. 

If you look at nsITimer, it would be EASY to make it scriptable:

1) change one of the Init() methods to initWithCallback() (because IDL doesn't
allow multiple interfaces with the same name)

2) tweak a few of the IsIdle/SetDelay/etc methods to be XPCOM-friendly, and
mark the rest [noscript]

both of these changes will require minor updates to consumers, but they will be
mechanical, low-risk changes. 

Lets do the right thing while we still have time.
Attachment #96720 - Flags: needs-work+
without choosing sides, i just wanted to voice one advantage of going with
nsIScriptableTimer (or whatever it might be called).  from reading
nsIScriptableTimer, i see that it is a much simpler interface than nsITimer. 
nsITimer appears to expose some extra functionality that we may or may not want
to freeze (i.e., support _forever_).

so my question is, are we happy freezing all of the functionality exposed by
nsITimer?  if so, then i think alecf is spot on, and we should do away with
nsIScriptableTimer.  if not, then maybe there is room for an alternate interface
designed not only for scriptability but also for embedders to use.

just my 2 cents.
nsITimer gives you the follow advantages over the nsIScriptableTimer:
1.  Ablity to set a callback to a function.
2.  Ablity to use a specific interface to recieve the timer notification. (no
need to use the nsIObserver)
3.  Ablity to query and change the timer's delay, and type.
4.  The ablity to query if it is an IDLE timer (which is really a implementation
feature).
5.  The ablity to query the closure

I think that I am with alec.  Lets kill nsIScriptableTimer and get a clean
nsITimer interface.  Patch coming up...
This is a patch for xpcom/thread.  A full tree patch will be coming up shortly.
 This patch removes the following:

nsIScriptableTimer.idl
nsITimer.h
nsITimerCallback.h

The new interface is named nsITimer.
ignore the implementation for now.  the interface here is the important part.
Attached patch nsITimer v.2 (xpcom/thread only) (obsolete) — Splinter Review
Attachment #97081 - Attachment is obsolete: true
NS_ => TYPE_ ?
+   * After firing, a NS_REPEATING_SLACK timer is stopped and not restarted
+   * until its callback completes.  Specified timer period will be at least
+   * the time between when processing for last firing the callback completes
+   * and when the next firing occurs.
After firing, a TYPE_REPEATING_SLACK timer will not restart until its callback
completes.  The specified timer period is the minimum interval between when one
callback completes and the next callback can be fired.
+   *
+   * This is the preferable repeating type for most situations.
This type of timer is prefered for most situations.
+   */
+  const short TYPE_REPEATING_SLACK    = 1;

NS_ => TYPE_ ?
+   * An NS_REPEATING_PRECISE repeating timer aims to have constant period
+   * between firings.  The processing time for each timer callback should not
+   * influence the timer period.  However, if the processing for the last
+   * timer firing could not be completed until just before the next firing
+   * occurs, then you could have two timer notification routines being
+   * executed in quick succession.
A TYPE_REPEATING_PRECISE repeating timer is supposed to have a constant period
between firings.  The processing time for each timer callback should not
influence the timer period.  If processing for the last timer firing could not
complete until just before the next scheduled firing, then two timer
notification routines may execute in quick succession.
+   */
+  const short TYPE_REPEATING_PRECISE  = 2;

+   * Initialize a timer that will fire after the said delay.
said=>specified
+   * A user must keep a reference to this timer till it is 
till=>until
<note this line ended with "is">
+   * is no longer needed or has been cancelled.
<note this line began with "is">
is no longer needed or has been cancelled=>cancelled or no longer needed

+   * The opaque pointer pass to initWithCallback.
pass=>passed
--
functional complaints:

+   * @return true if the timer is an idle timer, false otherwise
+   */
+  boolean isIdle();

(a) Timer is idle and (b) Timer is an Idle Timer are different things, the
documentation clearly implies (b), but the function name leads me to (a).. oh
well, you'll ignore this complaint.

+   * The timer type : one shot or repeating
+   */  
+  attribute unsigned long type;
the documentation implies two states in which case you could use a boolean, but
i suspect you meant for this to match the 
+  const short TYPE_REPEATING_SLACK    = 1;
things. in which case, i have to ask why it's a long and the constants are shorts.
it would probably be easier if you just made the changes and created a patch!  I
will do my best to intergrate your comment suggestions.  Thanks for the feedback.
note this doesn't cover the second part of my comments which are questions that
i'd like answer.
Attached patch nsITimer v.3Splinter Review
does not include timeless's comment changes.
Attachment #96720 - Attachment is obsolete: true
Attachment #97084 - Attachment is obsolete: true
*** Bug 128547 has been marked as a duplicate of this bug. ***
Comment on attachment 97255 [details] [diff] [review]
nsITimer v.3

looks great!
so nice to get rid of nsIScriptableTimer...
sr=alecf
Attachment #97255 - Flags: superreview+
Checking in accessible/src/base/nsRootAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v  <-- 
nsRootAccessible.cpp
new revision: 1.49; previous revision: 1.48
done
Checking in accessible/src/base/nsRootAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.h,v  <--  nsRootAccessible.h
new revision: 1.29; previous revision: 1.28
done
Checking in content/base/src/nsPagePrintTimer.cpp;
/cvsroot/mozilla/content/base/src/nsPagePrintTimer.cpp,v  <--  nsPagePrintTimer.cpp
new revision: 1.2; previous revision: 1.1
done
Checking in content/base/src/nsPagePrintTimer.h;
/cvsroot/mozilla/content/base/src/nsPagePrintTimer.h,v  <--  nsPagePrintTimer.h
new revision: 1.2; previous revision: 1.1
done
Checking in content/base/src/nsSelection.cpp;
/cvsroot/mozilla/content/base/src/nsSelection.cpp,v  <--  nsSelection.cpp
new revision: 3.132; previous revision: 3.131
done
Checking in content/html/document/src/nsHTMLContentSink.cpp;
/cvsroot/mozilla/content/html/document/src/nsHTMLContentSink.cpp,v  <-- 
nsHTMLContentSink.cpp
new revision: 3.572; previous revision: 3.571
done
Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.471; previous revision: 1.470
done
Checking in docshell/base/nsDocShell.h;
/cvsroot/mozilla/docshell/base/nsDocShell.h,v  <--  nsDocShell.h
new revision: 1.129; previous revision: 1.128
done
Checking in dom/src/base/nsGlobalWindow.cpp;
/cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v  <--  nsGlobalWindow.cpp
new revision: 1.545; previous revision: 1.544
done
Checking in dom/src/base/nsJSEnvironment.cpp;
/cvsroot/mozilla/dom/src/base/nsJSEnvironment.cpp,v  <--  nsJSEnvironment.cpp
new revision: 1.179; previous revision: 1.178
done
Checking in dom/src/base/nsJSEnvironment.h;
/cvsroot/mozilla/dom/src/base/nsJSEnvironment.h,v  <--  nsJSEnvironment.h
new revision: 1.58; previous revision: 1.57
done
Checking in editor/composer/src/nsComposerCommandsUpdater.cpp;
/cvsroot/mozilla/editor/composer/src/nsComposerCommandsUpdater.cpp,v  <-- 
nsComposerCommandsUpdater.cpp
new revision: 1.6; previous revision: 1.5
done
Checking in editor/composer/src/nsComposerCommandsUpdater.h;
/cvsroot/mozilla/editor/composer/src/nsComposerCommandsUpdater.h,v  <-- 
nsComposerCommandsUpdater.h
new revision: 1.4; previous revision: 1.3
done
Checking in editor/composer/src/nsInterfaceState.cpp;
/cvsroot/mozilla/editor/composer/src/nsInterfaceState.cpp,v  <-- 
nsInterfaceState.cpp
new revision: 1.43; previous revision: 1.42
done
Checking in editor/composer/src/nsInterfaceState.h;
/cvsroot/mozilla/editor/composer/src/nsInterfaceState.h,v  <--  nsInterfaceState.h
new revision: 1.30; previous revision: 1.29
done
Checking in embedding/browser/webBrowser/nsDocShellTreeOwner.cpp;
/cvsroot/mozilla/embedding/browser/webBrowser/nsDocShellTreeOwner.cpp,v  <-- 
nsDocShellTreeOwner.cpp
new revision: 1.68; previous revision: 1.67
done
Checking in extensions/inspector/base/src/inSearchLoop.cpp;
/cvsroot/mozilla/extensions/inspector/base/src/inSearchLoop.cpp,v  <-- 
inSearchLoop.cpp
new revision: 1.8; previous revision: 1.7
done
Checking in extensions/inspector/base/src/inSearchLoop.h;
/cvsroot/mozilla/extensions/inspector/base/src/inSearchLoop.h,v  <--  inSearchLoop.h
new revision: 1.7; previous revision: 1.6
done
Checking in extensions/pref/autoconfig/src/nsAutoConfig.cpp;
/cvsroot/mozilla/extensions/pref/autoconfig/src/nsAutoConfig.cpp,v  <-- 
nsAutoConfig.cpp
new revision: 1.9; previous revision: 1.8
done
Checking in extensions/pref/autoconfig/src/nsAutoConfig.h;
/cvsroot/mozilla/extensions/pref/autoconfig/src/nsAutoConfig.h,v  <-- 
nsAutoConfig.h
new revision: 1.3; previous revision: 1.2
done
Checking in extensions/typeaheadfind/src/nsTypeAheadFind.cpp;
/cvsroot/mozilla/extensions/typeaheadfind/src/nsTypeAheadFind.cpp,v  <-- 
nsTypeAheadFind.cpp
new revision: 1.11; previous revision: 1.10
done
Checking in extensions/typeaheadfind/src/nsTypeAheadFind.h;
/cvsroot/mozilla/extensions/typeaheadfind/src/nsTypeAheadFind.h,v  <-- 
nsTypeAheadFind.h
new revision: 1.6; previous revision: 1.5
done
Checking in js/src/xpconnect/tests/components/xpctest_echo.cpp;
/cvsroot/mozilla/js/src/xpconnect/tests/components/xpctest_echo.cpp,v  <-- 
xpctest_echo.cpp
new revision: 1.37; previous revision: 1.36
done
Checking in js/src/xpconnect/tests/components/xpctest_private.h;
/cvsroot/mozilla/js/src/xpconnect/tests/components/xpctest_private.h,v  <-- 
xpctest_private.h
new revision: 1.24; previous revision: 1.23
done
Checking in layout/base/src/nsCaret.cpp;
/cvsroot/mozilla/layout/base/src/nsCaret.cpp,v  <--  nsCaret.cpp
new revision: 1.94; previous revision: 1.93
done
Checking in layout/html/base/src/nsObjectFrame.cpp;
/cvsroot/mozilla/layout/html/base/src/nsObjectFrame.cpp,v  <--  nsObjectFrame.cpp
new revision: 1.358; previous revision: 1.357
done
Checking in layout/html/base/src/nsPresShell.cpp;
/cvsroot/mozilla/layout/html/base/src/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.567; previous revision: 3.566
done
Checking in layout/html/base/src/nsTextFrame.cpp;
/cvsroot/mozilla/layout/html/base/src/nsTextFrame.cpp,v  <--  nsTextFrame.cpp
new revision: 1.386; previous revision: 1.385
done
Checking in layout/html/forms/src/nsListControlFrame.cpp;
/cvsroot/mozilla/layout/html/forms/src/nsListControlFrame.cpp,v  <-- 
nsListControlFrame.cpp
new revision: 1.268; previous revision: 1.267
done
Checking in layout/xul/base/src/nsListBoxBodyFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsListBoxBodyFrame.cpp,v  <-- 
nsListBoxBodyFrame.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in layout/xul/base/src/nsMenuBarFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuBarFrame.cpp,v  <--  nsMenuBarFrame.cpp
new revision: 1.104; previous revision: 1.103
done
Checking in layout/xul/base/src/nsMenuFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuFrame.cpp,v  <--  nsMenuFrame.cpp
new revision: 1.232; previous revision: 1.231
done
Checking in layout/xul/base/src/nsMenuFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsMenuFrame.h,v  <--  nsMenuFrame.h
new revision: 1.89; previous revision: 1.88
done
Checking in layout/xul/base/src/nsMenuPopupFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v  <-- 
nsMenuPopupFrame.cpp
new revision: 1.179; previous revision: 1.178
done
Checking in layout/xul/base/src/nsMenuPopupFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.h,v  <--  nsMenuPopupFrame.h
new revision: 1.56; previous revision: 1.55
done
Checking in layout/xul/base/src/nsPopupSetFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsPopupSetFrame.h,v  <--  nsPopupSetFrame.h
new revision: 1.36; previous revision: 1.35
done
Checking in layout/xul/base/src/nsRepeatService.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsRepeatService.cpp,v  <--  nsRepeatService.cpp
new revision: 1.23; previous revision: 1.22
done
Checking in layout/xul/base/src/nsRepeatService.h;
/cvsroot/mozilla/layout/xul/base/src/nsRepeatService.h,v  <--  nsRepeatService.h
new revision: 1.8; previous revision: 1.7
done
Checking in layout/xul/base/src/nsScrollBoxFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsScrollBoxFrame.cpp,v  <-- 
nsScrollBoxFrame.cpp
new revision: 1.33; previous revision: 1.32
done
Checking in layout/xul/base/src/nsScrollbarButtonFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsScrollbarButtonFrame.cpp,v  <-- 
nsScrollbarButtonFrame.cpp
new revision: 1.27; previous revision: 1.26
done
Checking in layout/xul/base/src/nsScrollbarButtonFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsScrollbarButtonFrame.h,v  <-- 
nsScrollbarButtonFrame.h
new revision: 1.14; previous revision: 1.13
done
Checking in layout/xul/base/src/nsSliderFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsSliderFrame.cpp,v  <--  nsSliderFrame.cpp
new revision: 1.90; previous revision: 1.89
done
Checking in layout/xul/base/src/nsSliderFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsSliderFrame.h,v  <--  nsSliderFrame.h
new revision: 1.38; previous revision: 1.37
done
Checking in layout/xul/base/src/nsXULTooltipListener.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsXULTooltipListener.cpp,v  <-- 
nsXULTooltipListener.cpp
new revision: 1.16; previous revision: 1.15
done
Checking in layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp,v  <-- 
nsTreeBodyFrame.cpp
new revision: 1.136; previous revision: 1.135
done
Checking in layout/xul/base/src/tree/src/nsTreeSelection.cpp;
/cvsroot/mozilla/layout/xul/base/src/tree/src/nsTreeSelection.cpp,v  <-- 
nsTreeSelection.cpp
new revision: 1.27; previous revision: 1.26
done
Checking in mailnews/base/search/src/nsMsgSearchSession.cpp;
/cvsroot/mozilla/mailnews/base/search/src/nsMsgSearchSession.cpp,v  <-- 
nsMsgSearchSession.cpp
new revision: 1.42; previous revision: 1.41
done
Checking in mailnews/base/src/nsMessengerWinIntegration.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMessengerWinIntegration.cpp,v  <-- 
nsMessengerWinIntegration.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in mailnews/base/src/nsMsgBiffManager.cpp;
/cvsroot/mozilla/mailnews/base/src/nsMsgBiffManager.cpp,v  <--  nsMsgBiffManager.cpp
new revision: 1.42; previous revision: 1.41
done
Checking in mailnews/base/src/nsMsgBiffManager.h;
/cvsroot/mozilla/mailnews/base/src/nsMsgBiffManager.h,v  <--  nsMsgBiffManager.h
new revision: 1.14; previous revision: 1.13
done
Checking in mailnews/news/src/nsNNTPProtocol.cpp;
/cvsroot/mozilla/mailnews/news/src/nsNNTPProtocol.cpp,v  <--  nsNNTPProtocol.cpp
new revision: 1.322; previous revision: 1.321
done
Checking in mailnews/news/src/nsNNTPProtocol.h;
/cvsroot/mozilla/mailnews/news/src/nsNNTPProtocol.h,v  <--  nsNNTPProtocol.h
new revision: 1.93; previous revision: 1.92
done
Checking in mailnews/news/src/nsNntpIncomingServer.cpp;
/cvsroot/mozilla/mailnews/news/src/nsNntpIncomingServer.cpp,v  <-- 
nsNntpIncomingServer.cpp
new revision: 1.126; previous revision: 1.125
done
Checking in modules/libpr0n/decoders/mng/imgContainerMNG.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/mng/imgContainerMNG.cpp,v  <-- 
imgContainerMNG.cpp
new revision: 1.12; previous revision: 1.11
done
Checking in modules/libpr0n/decoders/mng/imgContainerMNG.h;
/cvsroot/mozilla/modules/libpr0n/decoders/mng/imgContainerMNG.h,v  <-- 
imgContainerMNG.h
new revision: 1.3; previous revision: 1.2
done
Checking in modules/libpr0n/src/imgContainer.cpp;
/cvsroot/mozilla/modules/libpr0n/src/imgContainer.cpp,v  <--  imgContainer.cpp
new revision: 1.33; previous revision: 1.32
done
Checking in modules/libpr0n/src/imgContainer.h;
/cvsroot/mozilla/modules/libpr0n/src/imgContainer.h,v  <--  imgContainer.h
new revision: 1.16; previous revision: 1.15
done
Checking in modules/plugin/base/src/nsPluginViewer.cpp;
/cvsroot/mozilla/modules/plugin/base/src/nsPluginViewer.cpp,v  <-- 
nsPluginViewer.cpp
new revision: 1.124; previous revision: 1.123
done
Checking in netwerk/protocol/ftp/src/nsFtpProtocolHandler.cpp;
/cvsroot/mozilla/netwerk/protocol/ftp/src/nsFtpProtocolHandler.cpp,v  <-- 
nsFtpProtocolHandler.cpp
new revision: 1.71; previous revision: 1.70
done
Checking in netwerk/protocol/http/src/nsHttpHandler.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpHandler.cpp,v  <-- 
nsHttpHandler.cpp
new revision: 1.65; previous revision: 1.64
done
Checking in security/manager/ssl/src/nsNSSComponent.cpp;
/cvsroot/mozilla/security/manager/ssl/src/nsNSSComponent.cpp,v  <-- 
nsNSSComponent.cpp
new revision: 1.88; previous revision: 1.87
done
Checking in security/manager/ssl/src/nsNSSComponent.h;
/cvsroot/mozilla/security/manager/ssl/src/nsNSSComponent.h,v  <--  nsNSSComponent.h
new revision: 1.24; previous revision: 1.23
done
Checking in view/src/nsScrollPortView.h;
/cvsroot/mozilla/view/src/nsScrollPortView.h,v  <--  nsScrollPortView.h
new revision: 3.16; previous revision: 3.15
done
Checking in view/src/nsScrollingView.cpp;
/cvsroot/mozilla/view/src/nsScrollingView.cpp,v  <--  nsScrollingView.cpp
new revision: 3.159; previous revision: 3.158
done
Checking in view/src/nsScrollingView.h;
/cvsroot/mozilla/view/src/nsScrollingView.h,v  <--  nsScrollingView.h
new revision: 3.67; previous revision: 3.66
done
Checking in view/src/nsViewManager.h;
/cvsroot/mozilla/view/src/nsViewManager.h,v  <--  nsViewManager.h
new revision: 3.101; previous revision: 3.100
done
Checking in webshell/tests/viewer/nsThrobber.cpp;
/cvsroot/mozilla/webshell/tests/viewer/nsThrobber.cpp,v  <--  nsThrobber.cpp
new revision: 1.20; previous revision: 1.19
done
Checking in webshell/tests/viewer/nsWebCrawler.cpp;
/cvsroot/mozilla/webshell/tests/viewer/nsWebCrawler.cpp,v  <--  nsWebCrawler.cpp
new revision: 1.112; previous revision: 1.111
done
Checking in widget/src/cocoa/nsSound.cpp;
/cvsroot/mozilla/widget/src/cocoa/nsSound.cpp,v  <--  nsSound.cpp
new revision: 1.7; previous revision: 1.6
done
Checking in widget/src/gtk/nsWidget.h;
/cvsroot/mozilla/widget/src/gtk/nsWidget.h,v  <--  nsWidget.h
new revision: 1.127; previous revision: 1.126
done
Checking in widget/src/gtk/nsWindow.cpp;
/cvsroot/mozilla/widget/src/gtk/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.383; previous revision: 1.382
done
Checking in widget/src/mac/nsSound.cpp;
/cvsroot/mozilla/widget/src/mac/nsSound.cpp,v  <--  nsSound.cpp
new revision: 1.29; previous revision: 1.28
done
Checking in xpcom/ds/nsRecyclingAllocator.cpp;
/cvsroot/mozilla/xpcom/ds/nsRecyclingAllocator.cpp,v  <--  nsRecyclingAllocator.cpp
new revision: 1.14; previous revision: 1.13
done
Checking in xpcom/macbuild/XPCOMIDL.xml;
/cvsroot/mozilla/xpcom/macbuild/XPCOMIDL.xml,v  <--  XPCOMIDL.xml
new revision: 1.16; previous revision: 1.15
done
Checking in xpcom/threads/MANIFEST;
/cvsroot/mozilla/xpcom/threads/MANIFEST,v  <--  MANIFEST
new revision: 3.8; previous revision: 3.7
done
Checking in xpcom/threads/MANIFEST_IDL;
/cvsroot/mozilla/xpcom/threads/MANIFEST_IDL,v  <--  MANIFEST_IDL
new revision: 3.6; previous revision: 3.5
done
Checking in xpcom/threads/Makefile.in;
/cvsroot/mozilla/xpcom/threads/Makefile.in,v  <--  Makefile.in
new revision: 1.32; previous revision: 1.31
done
Removing xpcom/threads/nsIScriptableTimer.idl;
/cvsroot/mozilla/xpcom/threads/nsIScriptableTimer.idl,v  <--  nsIScriptableTimer.idl
new revision: delete; previous revision: 3.2
done
Removing xpcom/threads/nsITimer.h;
/cvsroot/mozilla/xpcom/threads/nsITimer.h,v  <--  nsITimer.h
new revision: delete; previous revision: 1.5
done
RCS file: /cvsroot/mozilla/xpcom/threads/nsITimer.idl,v
done
Checking in xpcom/threads/nsITimer.idl;
/cvsroot/mozilla/xpcom/threads/nsITimer.idl,v  <--  nsITimer.idl
initial revision: 3.1
done
Removing xpcom/threads/nsITimerCallback.h;
/cvsroot/mozilla/xpcom/threads/nsITimerCallback.h,v  <--  nsITimerCallback.h
new revision: delete; previous revision: 1.1
done
RCS file: /cvsroot/mozilla/xpcom/threads/nsITimerInternal.idl,v
done
Checking in xpcom/threads/nsITimerInternal.idl;
/cvsroot/mozilla/xpcom/threads/nsITimerInternal.idl,v  <--  nsITimerInternal.idl
initial revision: 3.1
done
Checking in xpcom/threads/nsTimerImpl.cpp;
/cvsroot/mozilla/xpcom/threads/nsTimerImpl.cpp,v  <--  nsTimerImpl.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in xpcom/threads/nsTimerImpl.h;
/cvsroot/mozilla/xpcom/threads/nsTimerImpl.h,v  <--  nsTimerImpl.h
new revision: 1.16; previous revision: 1.15
done
Checking in xpfe/appshell/src/nsWebShellWindow.cpp;
/cvsroot/mozilla/xpfe/appshell/src/nsWebShellWindow.cpp,v  <--  nsWebShellWindow.cpp
new revision: 1.385; previous revision: 1.384
done
Checking in xpfe/browser/src/nsBrowserInstance.cpp;
/cvsroot/mozilla/xpfe/browser/src/nsBrowserInstance.cpp,v  <-- 
nsBrowserInstance.cpp
new revision: 1.245; previous revision: 1.244
done
Checking in xpfe/browser/src/nsBrowserStatusFilter.cpp;
/cvsroot/mozilla/xpfe/browser/src/nsBrowserStatusFilter.cpp,v  <-- 
nsBrowserStatusFilter.cpp
new revision: 1.2; previous revision: 1.1
done
Checking in xpfe/components/bookmarks/src/nsBookmarksService.cpp;
/cvsroot/mozilla/xpfe/components/bookmarks/src/nsBookmarksService.cpp,v  <-- 
nsBookmarksService.cpp
new revision: 1.255; previous revision: 1.254
done
Checking in xpfe/components/directory/nsDirectoryViewer.cpp;
/cvsroot/mozilla/xpfe/components/directory/nsDirectoryViewer.cpp,v  <-- 
nsDirectoryViewer.cpp
new revision: 1.96; previous revision: 1.95
done
Checking in xpfe/components/history/src/nsGlobalHistory.cpp;
/cvsroot/mozilla/xpfe/components/history/src/nsGlobalHistory.cpp,v  <-- 
nsGlobalHistory.cpp
new revision: 1.155; previous revision: 1.154
done
Checking in xpfe/components/search/src/nsInternetSearchService.cpp;
/cvsroot/mozilla/xpfe/components/search/src/nsInternetSearchService.cpp,v  <-- 
nsInternetSearchService.cpp
new revision: 1.179; previous revision: 1.178
done
Checking in xpfe/components/updates/src/nsUpdateNotifier.js;
/cvsroot/mozilla/xpfe/components/updates/src/nsUpdateNotifier.js,v  <-- 
nsUpdateNotifier.js
new revision: 1.6; previous revision: 1.5
done


I am going to mark this as fixed since we decided not to freeze this right now.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
This commit have added a "might be used unititialized" warning to brad Tbox (see
also bug 59652):

+xpfe/browser/src/nsBrowserStatusFilter.cpp:269
+ `nsresult rv' might be used uninitialized in this function

Indeed, nsBrowserStatusFilter::StartDelayTimer now will *always* check NS_FAILED
on an uninitialized rv!

http://lxr.mozilla.org/mozilla/source/xpfe/browser/src/nsBrowserStatusFilter.cpp#266
:

269     nsresult rv;
...     ... [no rv]
276     if (NS_FAILED(rv)) return rv;
277 
278     return NS_OK;
thanks.  fixed it.
dougt, reviewers: why didn't that code just do this:

   return mTimer->InitWithFuncCallback(TimeoutHandler, this, 400, 
                                       nsITimer::TYPE_ONE_SHOT);

There is no reason to worry about converting non-NS_OK success codes (e.g.
NS_COMFALSE) into NS_OK here.

/be
you are right.  I cleaned it up and checked it in.
Blocks: 167782
is nsITimer.idl being frozen with this bug? If so there are a few
inconsistencies between parameters and comments. Otherwise will verify this one
as fixed and submit other bug. The inconsistencies are:
1) notify() uses "timer" parameter, comments refer to "aTimer"
2) initWithFuncCallback(): 1st param is aCallback, comments use "aFunc"
3) initWithCallback(): "" ""
nsIScriptableInputStream and mozIJSSubScriptLoader patches verified. But
questions about mozIJSSubScriptLoader::loadSubScript(). In the comments it
mentions rv, but return type is void. Is that OK with JS? What about the
optional object (obj)? How would that be prototyped in idl?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
david, please do not reopen my bugs without first talking with me.  
Status: REOPENED → RESOLVED
Closed: 22 years ago22 years ago
Resolution: --- → FIXED
verified. Mozilla 1.3a Gecko/20021119
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.