Consider removing Timer.jsm

NEW
Unassigned

Status

()

Firefox
General
a year ago
10 months ago

People

(Reporter: marco, Unassigned)

Tracking

(Blocks: 1 bug)

52 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

a year ago
Timer.jsm is a really simple wrapper around nsITimer, it could be removed.
Per the discussion in bug 1353731 comment 6 through 8, it seems exposing setTimeout and friends using importGlobalProperties would be the best way forward.

Needinfo on bholley and froydnj to figure out who would be the best person to handle this.
Flags: needinfo?(nfroyd)
Flags: needinfo?(bobbyholley)
(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> Per the discussion in bug 1353731 comment 6 through 8, it seems exposing
> setTimeout and friends using importGlobalProperties would be the best way
> forward.

One thing that's tricky about this is that the current setTimeout logic is all tangled up with nsGlobalWindow, and generalizing it to the point that it could be used in other globals is non-trivial to say the least. I should have been more explicit about this in bug 1353731 - my comment there was just to say that anything new exposed on sandboxes should be opt-in, orthogonal to how hard it would be to expose the given thing.

> 
> Needinfo on bholley and froydnj to figure out who would be the best person
> to handle this.

These are busy times, and I don't think that either I or Nathan will have cycles for this anytime soon. Unless this is extremely hard priority, you will likely find it difficult to find an XPConnect-capable hacker to work on this in the 57 timeframe.
Flags: needinfo?(nfroyd)
Flags: needinfo?(bobbyholley)

Comment 3

a year ago
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2)
> (In reply to Florian Quèze [:florian] [:flo] from comment #1)
> > Per the discussion in bug 1353731 comment 6 through 8, it seems exposing
> > setTimeout and friends using importGlobalProperties would be the best way
> > forward.
> 
> One thing that's tricky about this is that the current setTimeout logic is
> all tangled up with nsGlobalWindow, and generalizing it to the point that it
> could be used in other globals is non-trivial to say the least. I should
> have been more explicit about this in bug 1353731 - my comment there was
> just to say that anything new exposed on sandboxes should be opt-in,
> orthogonal to how hard it would be to expose the given thing.

This has actually recently been improved somewhat, most of this has been refactored into the TimeoutManager class now.  I didn't pay a lot of attention to make it completely independent of nsGlobalWindow.

None of this of course changes Bobby's comment about the complexities of dealing with the globals, but with some work we may actually be able to make TimeoutManager fully standalone and expose setTimout/setInterval to JSMs based on that.  Assuming we had a solution for the other issues involved.  :-)
Andreas, is this bug something you could help with? I'm mostly asking about the "exposing setTimeout and friends using importGlobalProperties" part. I'm happy to do the needed changes to our JS code if we can have a good replacement for the current Timer.jsm module.
Flags: needinfo?(afarre)

Comment 5

10 months ago
Not sure that I have the time either, but I had a look at what needs to be done.

Things TimeoutManager relies on nsGlobalWindow for:

* IsBackgroundInternal
* IsChromeWindow
* IsDocumentLoaded
* InnerObjectsFreed
* IsInnerWindow
* TimeoutManager

Used mainly to control throttling (start, resume, ...).

* AsInner

Used to be able to call IsPlayingAudio, InnerObjectsFreed,
IsDocumentLoaded, TimeoutManager. These all have to do with throttling.

* IsPlayingAudio
* WindowID

Called from IsActive only, also a throttling thing.

* GetExtantDoc

Check if there is a document at all before setting timeout.

* IsSuspended
* IsFrozen

Called all over the place, mainly to determine scheduling (timeouts and executor).

* GetPopupControlState

Used to block popups

* GetContextInternal

Get the script context to run the script with. Differs from nsIScriptGlobalObject::GetContext in that it also returns a script context for inner window.

* RunTimeoutHandler

Runs the timeout.

* EventTargetFor

Used to implement TimeoutManager::EventTarget.
Flags: needinfo?(afarre)
You need to log in before you can comment on or make changes to this bug.