1482070 - Move devtools/shared/webconsole/network-monitor next to its actor

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

devtools/shared/webconsole/network-monitor.js is only used from the actors and should rather be next to them rather than in shared folder:
  https://searchfox.org/mozilla-central/search?q=network-monitor%22&case=false&regexp=false&path=

Now the naming will be complex as everything is named network-something:
 * devtools/server/actors/network-monitor.js
   NetworkMonitorActor
 * devtools/server/actors/network-event.js
   NetworkEventActor
 * devtools/shared/webconsole/network-monitor.js
   StackTraceCollector, NetworkMonitor, ConsoleProgressListener

First thing would be to split network-monitor.js in three and move ConsoleProgressListener to actors/webconsole/listeners.js (we may split listeners.js if that helps).
Then, I think it would be helful to create a new folder: actors/network-monitor, and move StackTraceCollector in it, as well as NetworkMonitor. But we would have to rename it to make the distinction between NetworkMonitorActor and NetworkMonitor clearer. What about NetworkObserver, or NetworkListener, or... ?

Comment 1

[Alexandre Poirot [:ochameau]]

honza, what do you think about this renaming plan?

Comment 2

[Jan Honza Odvarko [:Honza] (always need-info? me)]

This is nice clean-up, thanks for working on it!

(In reply to Alexandre Poirot [:ochameau] from comment #0)
> devtools/shared/webconsole/network-monitor.js is only used from the actors
> and should rather be next to them rather than in shared folder:
>  
> https://searchfox.org/mozilla-central/search?q=network-
> monitor%22&case=false&regexp=false&path=
> 
> Now the naming will be complex as everything is named network-something:
>  * devtools/server/actors/network-monitor.js
>    NetworkMonitorActor
>  * devtools/server/actors/network-event.js
>    NetworkEventActor
>  * devtools/shared/webconsole/network-monitor.js
>    StackTraceCollector, NetworkMonitor, ConsoleProgressListener
> 
> First thing would be to split network-monitor.js in three and move
> ConsoleProgressListener to actors/webconsole/listeners.js
All make total sense. We might split it into more files, see below.

> (we may split listeners.js if that helps).
I am usually in favor or splitting files (having one object/component per file), so I am saying yes. No need to have an extra dir for these new listener files of course.

> Then, I think it would be helpful to create a new folder:
> actors/network-monitor, and move StackTraceCollector in it, as well as
Yes, precisely

> NetworkMonitor. But we would have to rename it to make the distinction
> between NetworkMonitorActor and NetworkMonitor clearer. What about
> NetworkObserver, or NetworkListener, or... ?

So, I can image the following:

* ConsoleProgressListener -> actors/webconsole/console-progress-listener.js
* ConsoleServiceListener -> actors/webconsole/console-service-listener.js
* ConsoleAPIListener -> actors/webconsole/console-api-listener.js
* ConsoleReflowListener -> actors/webconsole/console-reflow-listener.js
* ContentProcessListener -> actors/webconsole/console-process-listener.js
* DocumentEventsListener -> actors/webconsole/document-events-listener.js
* StackTraceCollector -> server/actors/network-monitor/stack-trace-collector.js
* NetworkMonitorActor -> server/actors/network-monitor/network-monitor-actor.js

When splitting `devtools/shared/webconsole/network-monitor.js` we might also introduce:
* matchRequest() -> server/actors/network-monitor/utils.js
* ChannelEventSink -> server/actors/network-monitor/channel-event-sink.js
* NetworkResponseListener -> server/actors/network-monitor/network-response-listener.js
* NetworkMonitor -> NetworkObserver -> server/actors/network-monitor/network-observer.js
     * I am picking 'observer' since the impl is based on `Services.obs.addObserver` 

-

I think that file names usually follow the name of the object/component implemented inside.
So, this is why I am suggesting:
* NetworkMonitorActor -> server/actors/network-monitor/network-monitor-actor.js

... and *not* a shortcut like:
* NetworkMonitorActor -> server/actors/network-monitor/actor.js

Even if the dir and file name might look a bit repetitive.

Honza

Comment 4

[Alexandre Poirot [:ochameau]]

I may have expected a slight change regarding perf, due to eventual lazy loading of some modules, but it looks like it doesn't change anything:
  https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=25b9514854630292e93fa5b8a6c412b77ecdcd6b&newProject=try&newRevision=06b411e497b19f34043c1b2b6ef3babff070a3a3&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&framework=12

Green try:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=06b411e497b19f34043c1b2b6ef3babff070a3a3&selectedJob=194494912

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ConsoleProgressListener to its own file. r=Honza

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ConsoleServiceListener to its own file. r=Honza

Depends On D3597

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ConsoleAPIListener to its own file. r=Honza

Depends On D3598

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ConsoleReflowListener to its own file. r=Honza

Depends On D3599

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ContentProcessListener to its own file. r=Honza

Depends On D3600

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move DocumentEventsListener to its own file. r=Honza

Depends On D3601

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move ChannelEventSink to its own file. r=Honza

Depends On D3602

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move StackTraceCollector to its own file. r=Honza

Depends On D3603

Comment 13

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Rename NetworkMonitor to NetworkObserver and move it next to the NetworkMonitorActor. r=Honza

Depends On D3604

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1482070 - Move NetworkResponseListener to its own file. r=Honza

Depends On D3605

Comment 15

[Alexandre Poirot [:ochameau]]

I changed two things compared to comment 2:
* I've let matchRequest in network-observer as I found it weird to have just this method in utils.js
* I've let NetworkMonitorActor in server/actors/network-monitor.js. All actors are following this pattern (except inspector.js). See highlighters, webconsole, emulation, object.

Comment 16

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001917 [details]
Bug 1482070 - Move ConsoleProgressListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 17

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001918 [details]
Bug 1482070 - Move ConsoleServiceListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001919 [details]
Bug 1482070 - Move ConsoleAPIListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001920 [details]
Bug 1482070 - Move ConsoleReflowListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001921 [details]
Bug 1482070 - Move ContentProcessListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 21

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001922 [details]
Bug 1482070 - Move DocumentEventsListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 22

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001924 [details]
Bug 1482070 - Move ChannelEventSink to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 23

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001925 [details]
Bug 1482070 - Move StackTraceCollector to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 24

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001926 [details]
Bug 1482070 - Rename NetworkMonitor to NetworkObserver and move it next to the NetworkMonitorActor. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 25

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 9001927 [details]
Bug 1482070 - Move NetworkResponseListener to its own file. r=Honza

Jan Honza Odvarko [:Honza] has approved the revision.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6c757dc5f0b
Move ConsoleProgressListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/71e21bc8aa29
Move ConsoleServiceListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/985b16e17356
Move ConsoleAPIListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6cb4956e53f
Move ConsoleReflowListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b6d22251680
Move ContentProcessListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/f269b019428d
Move DocumentEventsListener to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc2fffaa4633
Move ChannelEventSink to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4b6ce31de8d
Move StackTraceCollector to its own file. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/6871a98b8b0b
Rename NetworkMonitor to NetworkObserver and move it next to the NetworkMonitorActor. r=Honza
https://hg.mozilla.org/integration/mozilla-inbound/rev/68bff34cbddd
Move NetworkResponseListener to its own file. r=Honza

Comment 27

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/c6c757dc5f0b
https://hg.mozilla.org/mozilla-central/rev/71e21bc8aa29
https://hg.mozilla.org/mozilla-central/rev/985b16e17356
https://hg.mozilla.org/mozilla-central/rev/e6cb4956e53f
https://hg.mozilla.org/mozilla-central/rev/5b6d22251680
https://hg.mozilla.org/mozilla-central/rev/f269b019428d
https://hg.mozilla.org/mozilla-central/rev/cc2fffaa4633
https://hg.mozilla.org/mozilla-central/rev/e4b6ce31de8d
https://hg.mozilla.org/mozilla-central/rev/6871a98b8b0b
https://hg.mozilla.org/mozilla-central/rev/68bff34cbddd