1420291 - RequestListContent could instanciate RequestListContextMenu lazily

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

Today we instanciate RequestListContextMenu on componentWillMount:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/RequestListContent.js#65-71
It forces loading its module immediately.

But I think we can delay its load and instanciation in onContextMenu function:
https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/RequestListContent.js#200-203

So that we speed up netmonitor opening.

Comment 1

[Tom Glowka]

Hey, this seems pretty clear, just replacing pure require with lazyGetter, right? I could do that, if it makes sense.

Comment 2

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

(In reply to glowka.tom from comment #1)
> Hey, this seems pretty clear, just replacing pure require with lazyGetter,
> right? I could do that, if it makes sense.

lazyGetter is not available in this scope. But, AFAIU this is about moving the `new RequestListContextMenu({...})` code from componentWillMount to onContextMenu.

Honza

Comment 3

[Tom Glowka]

OK, so please assign it to me.

Comment 4

[Tom Glowka]

Attachment: Bug 1420291 - instanciate RequestListContextMenu lazily

Review commit: https://reviewboard.mozilla.org/r/228362/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228362/

Comment 5

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

Comment on attachment 8959561 [details]
Bug 1420291 - instanciate RequestListContextMenu lazily

https://reviewboard.mozilla.org/r/228362/#review234686

Looks good, thanks for the patch!

R+ assuming try is green

Honza

Comment 6

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

Also, please rebase on top of m-c before landing.
Honza

Comment 7

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

Btw. you already know you need to set checkin-needed, right?

Honza

Comment 8

[Tom Glowka]

Comment on attachment 8959561 [details]
Bug 1420291 - instanciate RequestListContextMenu lazily

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228362/diff/1-2/

Comment 9

[Tom Glowka]

(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> Also, please rebase on top of m-c before landing.
> Honza

Rebased.

(In reply to Jan Honza Odvarko [:Honza] from comment #7)
> Btw. you already know you need to set checkin-needed, right?
> 
> Honza

Yeah, but good to be reminded, thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e63bab1349dd
instanciate RequestListContextMenu lazily r=Honza

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e63bab1349dd