526580 - IPDL should generate documentation and subprotocol diagrams automatically

Status: NEW

(Core :: IPC, enhancement, P5)

Description

[Benjamin Smedberg]

IPDL has all the information necessary to generate some fairly nice documentation: I'd really like a graph showing the subprotocol hierarchies for PPluginModule and PContentProcess. It also seems really easy to generate docs for protocols, although I'm less sure what you'd actually do with them, other than perhaps browse them locally.

Low-priority, but I thought I'd file it since I'm redoing the IPDL docs today.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

+1.  ISTR a bug about generating state machine diagrams as well, but can't find it.  Maybe it was just bounced around as a project idea.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Note that with bug 564086, we now also have complete information about the process graph and which IPDL parent/child actors live in which process.

Comment 3

[Kerem Kat]

Attachment: allprotocols.png

Please assign this bug to me as I would like to work on diagram generation and then documentation generation.

I have a general idea for subprotocol hierarchies which include parent-child protocol management relationships. Attached allprotocols.png has three node colors corresponding to protocol strength (async: blue, sync: pink, intr: orange). Edges can be forward(in), back(out) and both(both).

For example, sync PBrowser protocol handles message async PDatePicker, calls async PDocumentRenderer and calls and receives async PRenderFrame.

Could you take a look and comment on correctness and completeness, considering what other information be useful to show?

Comment 4

[Benjamin Smedberg]

Kerem, I am less involved with IPC stuff today, and so your best bet as a mentor and reviewer would be Bill McCloskey (:billm). Typically we don't assign bugs to new contributors until they've attached their patch.

Comment 5

[Benjamin Smedberg]

Are you asking about the correctness/completeness of this particular graph? I have some very general comments that Bill might add to:

* The most important bit is to identify the parent/child relationships and call out the root protocols such as PBrowser/PPluginModule. Perhaps a more hierarchical representation would be good.
* Do the nodes link to more details, or to the IPDL source files (either from searchfox or DXR)?
* There should definitely be a legend explaning the color scheme: I'm not sure the color scheme is that important, personally.

Comment 6

[Kerem Kat]

I intend to make changes to the patch according to correctness and completeness feedbacks about this particular graph before attaching the patch to this bug, as the code is not patch quality yet.

This particular graph is generated via a writing a dot file from ipdl compiler and laid out using graphviz fdp algorithm.

* I will look into a more hierarchical representation.
* As is, the graph is not interactive, just a png image.
* Legend is a good idea

Also, I am open to suggestions of a graph file format, just picked graphviz dot OTOH.

Thank you for the update and redirection.

Comment 7

[Kerem Kat]

Attachment: allprotocols.LR.png

Here is a left-to-right hierarchical view of the protocols.

Comment 8

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

Hi Kerem,
This is a neat diagram. I think you could make it a little easier to read by turning it into a tree. You would have to clone nodes that have multiple parents. That would obscure places where a node is shared, but I think that's fine.

I'm not sure what the "async in" stuff is about. I suspect that's based on how the constructor message is marked? That's not really very important, so I think it should be removed (if I'm right about what it is).

Whether the protocol is async, sync, intr, or high priority is very important. I think it would be great to make that clearer in a key or something.

Honestly, though, I wonder if this would be easier to read in text form. You could present it as:

PContent (sync nested(inside_cpow))
.. PPrinting
.... PPrintSettingsDialog
.... PRemotePrintJob
.. PFlyWebPublishedServer
etc

You could still use colors, but this way it would be easier for people to copy and paste, search, etc.

I'm not sure exactly how we would use this. The protocols change often enough that simply putting it in a wiki wouldn't help much. Perhaps if we could publish it somewhere where it could be updated automatically?

Comment 9

[Benjamin Smedberg]

I would very much like this to be hooked up with the in-tree doc system which is built with `./mach doc`. That is auto-published to https://gecko.readthedocs.io/en/latest/