1350436 - Remove Dispatcher and renaming ValidatingDispatcher to SchedulerGroup

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This is a refactoring I've been considering since bug 1337537. I would like to eliminate the Dispatcher class. It serves as a base class to DocGroup and ValidatingDispatcher. However, DocGroup doesn't really use its functionality--it just delegates to its TabGroup. So ValidatingDispatcher is really only one subclass. I might as well fold Dispatcher into ValidatingDispatcher. Then I can rename that class to SchedulerGroup, which is a better name.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: DocGroup no longer inherits from Dispatcher

This patch makes DocGroup no longer inherit from Dispatcher.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: Get rid of Dispatcher

This patch gets rid of Dispatcher so that we only have ValidatingDispatcher.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: Rename ValidatingDispatcher to SchedulerGroup

This patch does the renaming to SchedulerGroup.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8851102 [details] [diff] [review]
DocGroup no longer inherits from Dispatcher

Review of attachment 8851102 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/DocGroup.h
@@ +75,5 @@
>  
> +  nsIEventTarget* EventTargetFor(TaskCategory aCategory) const;
> +
> +  AbstractThread*
> +  AbstractMainThreadFor(TaskCategory aCategory);

I'm unsure if we're concerned that we now/will have a set of functions in DocGroup and a set of functions in Dispatcher/SystemGroup that share names and functionality/contracts, but there's no way of ensuring that the two implementations stay "in sync".  I guess if TabGroup doesn't implement the Dispatcher interface, then this is kind of a moot point, because we're already in that sort of situation today.

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> I guess if TabGroup doesn't implement the
> Dispatcher interface, then this is kind of a moot point, because we're
> already in that sort of situation today.

Oh, I guess TabGroup inherits from ValidatingDispatcher, so TabGroup will at least have a consistent interface by virtue of that.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: networking changes

While rebasing this patch over bug 1343750, I had to make significant changes. I think it deserves another review.

I'm trying to change the code so that Dispatcher is no longer a shared base class for DocGroup and TabGroup. Instead, I would like to use nsIEventTarget as a generic type to be used to dispatch to a DocGroup or TabGroup. This patch does that for the HTTP and FTP code.

I had to remove an assertion in the HTTP code. It's hard to see how to keep it with the code refactored the way it is. The property being asserted isn't really necessary for the code to be correct. It's just a nice thing to have.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7509b5c557ee
Use nsIEventTarget instead of Dispatcher in netwerk code (r=mayhemer)
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e1cf2787080
DocGroup should not inherit from Dispatcher (r=froydnj)
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ff47d6ae913
Collapse Dispatcher into ValidatingDispatcher (r=froydnj)
https://hg.mozilla.org/integration/mozilla-inbound/rev/68dcc900c5bd
Rename ValidatingDispatcher to SchedulerGroup (r=froydnj)

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7509b5c557ee
https://hg.mozilla.org/mozilla-central/rev/2e1cf2787080
https://hg.mozilla.org/mozilla-central/rev/1ff47d6ae913
https://hg.mozilla.org/mozilla-central/rev/68dcc900c5bd