Bug 54067 - nrwt multiprocessing: minor cleanup prior to implementing the new worker
Summary: nrwt multiprocessing: minor cleanup prior to implementing the new worker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks: 49566 54068
  Show dependency tree / graph
 
Reported: 2011-02-08 22:32 PST by Dirk Pranke
Modified: 2011-02-09 16:09 PST (History)
4 users (show)

See Also:

Attachments
Patch (4.73 KB, patch)
2011-02-08 22:34 PST, Dirk Pranke 		no flags Details | Formatted Diff | Diff
make ManagerConnection inherit from BrokerConnection (4.83 KB, patch)
2011-02-08 22:48 PST, Dirk Pranke 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2011-02-08 22:32:54 PST 
nrwt multiprocessing: minor cleanup prior to implementing the new worker
Comment 1 Dirk Pranke 2011-02-08 22:34:39 PST 
Created attachment 81753 [details]
Patch
Comment 2 Dirk Pranke 2011-02-08 22:48:30 PST 
Created attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection
Comment 3 Tony Chang 2011-02-09 11:00:39 PST 
Comment on attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

View in context: https://bugs.webkit.org/attachment.cgi?id=81754&action=review

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker.py:139
> +class _ManagerConnection(message_broker2.BrokerConnection):

It might be good to somehow make it clear in the diagram in message_broker2.py that BrokerConnection is always a Manager.   Something like:

50	    BrokerClient   ------>    ManagerConnection (implements BrokerConnection)
51	         ^                         |
52	         |                         v
53	         \----------------      Broker
Comment 4 Dirk Pranke 2011-02-09 12:18:53 PST 
WorkerConnection (see bug 54067) will also implement BrokerConnection, just as the Worker class will implement AbstractWorker (and hence BrokerClient). Also, from the broker's point of view, there's no such thing as a "manager", only clients. It might make sense to create a similar drawing in manager_worker_broker showing both sides.

Thoughts?
Comment 5 Tony Chang 2011-02-09 12:28:18 PST 
(In reply to comment #4)
> WorkerConnection (see bug 54067) will also implement BrokerConnection, just as the Worker class will implement AbstractWorker (and hence BrokerClient). Also, from the broker's point of view, there's no such thing as a "manager", only clients. It might make sense to create a similar drawing in manager_worker_broker showing both sides.
> 
> Thoughts?

Ah, right, I see.  Seems like in manager_worker_broker.py, we could have a diagram with the concrete classes on the manager side and in whatever file has the Worker implementations, it could have a diagram with the worker side.
Comment 6 Dirk Pranke 2011-02-09 16:07:03 PST 
er, that was supposed to be bug 54068, not 54067 (which is this bug).

At any rate, I will add a diagram in that bug, since most of the classes don't exist in this patch.
Comment 7 Dirk Pranke 2011-02-09 16:09:41 PST 
Comment on attachment 81754 [details]
make ManagerConnection inherit from BrokerConnection

Clearing flags on attachment: 81754

Committed r78159: <http://trac.webkit.org/changeset/78159>
Comment 8 Dirk Pranke 2011-02-09 16:09:45 PST 
All reviewed patches have been landed.  Closing bug.