1490927 - Network events are received twice from the backend

Status: VERIFIED FIXED

(DevTools :: Console, defect, P2)

Description

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

All network events (both `networkEvent` and `networkEventUpdate` are received twice.

This can be tested by putting `console.log` into:
https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/devtools/shared/webconsole/client.js#138


if (!networkInfo) {
  return;
}

console.log("Client._onNetworkEventUpdate " +
  packet.updateType +
  packet.from, packet);

networkInfo.updates.push(packet.updateType);

STR:
1) Load: http://janodvarko.cz/firebug/tests/601/Issue601.htm
2) Open the Toolbox, select the Console panel
3) Open the Browser console.
4) Press the "POST" button on the page to generate XHR

Note, that the problem does happen only if the Browser console is opened.

Honza

Comment 1

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

@ochameau: Could this be caused by bug 1444132?

Honza

Comment 2

[Marco Castelluccio [:marco]]

Is this a regression?

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1490927 - Stop sharing maps in NetworkMonitor between all instances. r=Honza

When opening a web console and a browser console, we instantiate two NetworkMonitor
instances in the same process and they need independant states.

Comment 4

[Alexandre Poirot [:ochameau]]

(In reply to Marco Castelluccio [:marco] from comment #2)
> Is this a regression?

Yes, from bug 1444132.

(In reply to Jan Honza Odvarko [:Honza] from comment #0)
> All network events (both `networkEvent` and `networkEventUpdate` are
> received twice.

This is a really stupid mistake... That's because we end up sharing the maps on the prototype here:
https://searchfox.org/mozilla-central/source/devtools/server/actors/network-monitor.js#14-15
  _netEvents: new Map(),
  _networkEventActorsByURL: new Map(),

I submitted a patch to fix that but it is quite complex to write a test against that as it involves two toolboxes.

Comment 5

[Alexandre Poirot [:ochameau]]

Note that because of that, the browser console client wasn't receiving the network-event-update events while the web console was receiving duplicated.
Duplicated events doesn't seem to break anything, while the miss of events in the browser console end up making all request panels (headers, cookies, params....) empty.
It should not affect the web console unless you open the browser console or the browser toolbox.

Comment 6

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

Comment on attachment 9009662 [details]
Bug 1490927 - Stop sharing maps in NetworkMonitor between all instances. r=Honza

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

Comment 7

[:Harald Kirschner :digitarald]

Honza, did this have a performance impact that we should track?

Comment 8

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

It only happened if the Browser Console was opened, so less impact on average web-def user I guess.

Honza

Comment 9

[Alexandre Poirot [:ochameau]]

Green try (before I addressed eslint):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=709746466436fd06d4de94b29f3793461aa4641e

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cce1c1845bbc
Stop sharing maps in NetworkMonitor between all instances. r=Honza

Comment 11

[Andrei Ciure[:aciure]]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Did you want to nominate this for Beta backport?

Comment 13

[Alexandre Poirot [:ochameau]]

Comment on attachment 9009662 [details]
Bug 1490927 - Stop sharing maps in NetworkMonitor between all instances. r=Honza

Approval Request Comment
[Feature/Bug causing the regression]:
  fix regression from bug 1444132
[User impact if declined]:
  Browser console has empty panels while inspecting network events.
[Is this code covered by automated tests?]:
  The browser console feature isn't covered by tests, while the one in regular web console is.
[Has the fix been verified in Nightly?]:
  Not by QA.
[Needs manual test from QE? If yes, steps to reproduce]: 
  There is STR in comment 0.
[List of other uplifts needed for the feature/fix]:
  None.
[Is the change risky?]:
  No.
[Why is the change risky/not risky?]:
  Fixed a naive issue and is covered by many tests in the regular web console.
[String changes made/needed]:

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9009662 [details]
Bug 1490927 - Stop sharing maps in NetworkMonitor between all instances. r=Honza

Low risk patch for a P2 devtools bug, baked on nightly for a week, uplift approved for 63 beta 12, thanks.

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d442c34c8ef7

Comment 16

[Stefan [:StefanG_QA]]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

I was able to reproduce the mentioned behavior on Windows 10 x64 and Windows 7 using Nightly (20180919003800) and Beta 63.0b11 builds with the steps from comment 0. When opening the Response tab of the appropriate XHR entry additional request is received.


The issue is verified as fixed on the latest Nightly build (20181010100123) and Beta 63.0b13. When opening the Response tab of the appropriate XHR entry additional request is not received any more.

Comment 17

[Stefan [:StefanG_QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0 20181010100123

This report had been verified on OS X 10.13.6 and Ubuntu 18.04 as well using the latest Nightly and Beta 63.0b13 builds.