GCLI starts network listeners for seemingly no reason

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Graphic Commandline and Toolbar
P1
normal
RESOLVED FIXED
a month ago
4 days ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [qf:p1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

From bug 1297525 comment 10, GCLI is starting network listeners, which gobble up memory.

We should disable this feature when only GCLI is used.
(Assignee)

Updated

a month ago
Priority: -- → P1
(Assignee)

Updated

a month ago
Blocks: 1314815
(Assignee)

Comment 1

a month ago
I can confirm that GCLI enables networking listening for all tabs.  So, this is in effect much worse than bug 1297525, because it affects all tabs instead of a single one.
(Assignee)

Updated

a month ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 3

a month ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=50a6791a3db913a47a8fd125df62df3c2b2425ad

Comment 4

28 days ago
mozreview-review
Comment on attachment 8850656 [details]
Bug 1346854 - Delay network listening until toolbox open.

https://reviewboard.mozilla.org/r/123204/#review126230

Good catch! Thanks for the fix.

::: devtools/client/framework/toolbox.js:416
(Diff revision 1)
>        yield this._target.makeRemote();
>  
> +      // Start tracking network activity on toolbox open
> +      yield this._target.client.attachConsole(this._target.form.consoleActor, [
> +        "NetworkActivity",
> +      ]);

It looks wrong to call attachConsole multiple times, we are going to create two instances of WebConsoleClient.
I think it would be better to call:
target.activeConsole.startListeners instead of client.attachConsole.
Attachment #8850656 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 5

20 days ago
mozreview-review-reply
Comment on attachment 8850656 [details]
Bug 1346854 - Delay network listening until toolbox open.

https://reviewboard.mozilla.org/r/123204/#review126230

> It looks wrong to call attachConsole multiple times, we are going to create two instances of WebConsoleClient.
> I think it would be better to call:
> target.activeConsole.startListeners instead of client.attachConsole.

Thanks, I actually wanted to do that first, but then I thought the method didn't exist...  Turns out the `WebConsoleClient` was just in a separate module. :S

I'll make the change.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 8

19 days ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=99a338d6a50005f18dbbbb90a1defac14e2e21f1
Comment hidden (mozreview-request)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=37404c9be64d3ff03d900fad265217aa39ffcaa9
I am seeing some collisions with patch from bug 1352699 that just landed (integration/autoland)

Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #11)
> I am seeing some collisions with patch from bug 1352699 that just landed
> (integration/autoland)

Okay, I'll rebase after this merges.
(Assignee)

Updated

17 days ago
Depends on: 1352699
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #12)
> (In reply to Jan Honza Odvarko [:Honza] from comment #11)
> > I am seeing some collisions with patch from bug 1352699 that just landed
> > (integration/autoland)
> 
> Okay, I'll rebase after this merges.
Just a note that patches from bug 1352699 landed in m-c.

Honza
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f832a95ba774781cf91aeb1661aeedbfa638f9b6
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

11 days ago
mozreview-review
Comment on attachment 8854927 [details]
Bug 1346854 - Adjust network monitor test expectations.

https://reviewboard.mozilla.org/r/126852/#review132626

Looks good to me!

Thanks Ryan!

Honza
Attachment #8854927 - Flags: review?(odvarko) → review+
(Assignee)

Updated

10 days ago
Whiteboard: [qf]
Comment hidden (mozreview-request)

Comment 19

10 days ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s dc639ea60ada -d a8a257ba1903: rebasing 389953:dc639ea60ada "Bug 1346854 - Delay network listening until toolbox open. r=ochameau"
rebasing 389954:52565493af19 "Bug 1346854 - Adjust network monitor test expectations. r=Honza" (tip)
merging devtools/client/netmonitor/test/head.js
warning: conflicts while merging devtools/client/netmonitor/test/head.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 22

10 days ago
Pushed by jryans@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/87c2e570f1c6
Delay network listening until toolbox open. r=ochameau
https://hg.mozilla.org/integration/autoland/rev/7857690c1399
Adjust network monitor test expectations. r=Honza
https://hg.mozilla.org/mozilla-central/rev/87c2e570f1c6
https://hg.mozilla.org/mozilla-central/rev/7857690c1399
Status: ASSIGNED → RESOLVED
Last Resolved: 10 days ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Updated

4 days ago
Whiteboard: [qf] → [qf:p1]
You need to log in before you can comment on or make changes to this bug.