The default bug view has changed. See this FAQ.

Sometimes network requests do not display all information

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Console
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: msucan, Assigned: msucan)

Tracking

Trunk
Firefox 16
Points:
---

Firefox Tracking Flags

(firefox15+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Under certain conditions network requests only display the request headers - no response information.

STR: Load a page with lots of network requests.


I have a fix almost ready. Will submit a patch with fix and explanation of the problem.
(Assignee)

Comment 1

5 years ago
Created attachment 629814 [details] [diff] [review]
proposed fix

Problem: when many requests happen at once the code that delays output causes handleNetworkActivity() to not find previous messages to update their network request information.

Fix: use a networkRequests object to hold information about requests that are displayed or about to be displayed. Using the DOM node as the object to attach request information to was not ideal in the first place. Plus, the querySelector() was potentially slow with a lot of output in the Web Console.

Potential of negative impact: minimal to none.

Try run:
https://tbpl.mozilla.org/?tree=Try&rev=c0255fe22a09

Once the try run is green and I get r+ I would like to ask for aurora approval. Thanks!
Attachment #629814 - Flags: review?(rcampbell)
(Assignee)

Comment 2

5 years ago
Asking for tracking-firefox15 because I'd like this bug fixed in Aurora.
status-firefox15: --- → affected
tracking-firefox15: --- → ?
(Assignee)

Comment 3

5 years ago
Comment on attachment 629814 [details] [diff] [review]
proposed fix

Re-assigning the review request to Dave.
Attachment #629814 - Flags: review?(rcampbell) → review?(dcamp)
(Assignee)

Updated

5 years ago
Blocks: 761257

Comment 4

5 years ago
Comment on attachment 629814 [details] [diff] [review]
proposed fix

Review of attachment 629814 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/webconsole/HUDService.jsm
@@ +2743,5 @@
>          location = aNode.childNodes[4].getAttribute("title");
>        }
>        delete this.cssNodes[desc + location];
>      }
> +    else if (aNode.classList.contains("webconsole-msg-network")) {

Is removeOutputMessage() always guaranteed to be called for every outputMessageNode() call?  Is there any case where a message might get queued and never displayed?  For example, if 1000 nodes are added while the scrollback limit is 200, do you throw away 800 nodes without every displaying them?

If we're not guaranteed that, we'll leak network requests here.
(Assignee)

Comment 5

5 years ago
(In reply to Dave Camp (:dcamp) from comment #4)
> Comment on attachment 629814 [details] [diff] [review]
> proposed fix
> 
> Review of attachment 629814 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/webconsole/HUDService.jsm
> @@ +2743,5 @@
> >          location = aNode.childNodes[4].getAttribute("title");
> >        }
> >        delete this.cssNodes[desc + location];
> >      }
> > +    else if (aNode.classList.contains("webconsole-msg-network")) {
> 
> Is removeOutputMessage() always guaranteed to be called for every
> outputMessageNode() call?  Is there any case where a message might get
> queued and never displayed?  For example, if 1000 nodes are added while the
> scrollback limit is 200, do you throw away 800 nodes without every
> displaying them?

Yes, messages that are queued are not guaranteed to be displayed - they could get thrown away.


> If we're not guaranteed that, we'll leak network requests here.

The HUDService-content script should not leak network requests - their processing and cleanup is independent of UI. The UI can't leak the actual network requests as such (no access to nsIRequests, nsIHttpChannels, etc). However you do rightly point out that we can leak JS objects that hold network information. We do clear them when clearOutput() is called and when the Web Console is closed. (just checked and I see I forgot to add _networkRequests = {} in clearOutput()).

To avoid such cases I can make sure that when the queue pruning happens we clean up the associated network info object.

Do you want me to make these changes here? I kept this patch minimal so we can land it in Aurora. To me it's fine to do this here.

Do note that bug 761257 makes more invasive changes and JS objects with network information in the HUD object are not even created if the messages are pruned before being displayed. So those objects no longer leak when pruning happens.

Thank you!
(Assignee)

Comment 6

5 years ago
Created attachment 631973 [details] [diff] [review]
[in-fx-team] address review comment

Made the code to reuse the removeOutputMessage() function when output queue pruning happens. The removeOutputMessage() handles the clearing of any recorded information for each message. This hopefully addresses your comment.

Looking forward to your review. Thank you!
Attachment #629814 - Attachment is obsolete: true
Attachment #629814 - Flags: review?(dcamp)
Attachment #631973 - Flags: review?(dcamp)

Updated

5 years ago
Attachment #631973 - Flags: review?(dcamp) → review+
(Assignee)

Comment 7

5 years ago
Comment on attachment 631973 [details] [diff] [review]
[in-fx-team] address review comment

https://hg.mozilla.org/integration/fx-team/rev/9fff1479db88
Attachment #631973 - Attachment description: address review comment → [in-fx-team] address review comment
(Assignee)

Comment 8

5 years ago
Thanks for the review!
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/9fff1479db88
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 10

5 years ago
Comment on attachment 631973 [details] [diff] [review]
[in-fx-team] address review comment

[Approval Request Comment]
Bug caused by (feature/regressing bug #): caused by bug 722685 - the quick succession of many network requests combined with delayed output of messages triggers the issue.
User impact if declined: some network requests in the Web Console will have no UI updates / no details about the request and response.
Testing completed (on m-c, etc.): several try pushes, testing in fx-team and m-c.
Risk to taking this patch (and alternatives if risky): Should be no risk - this code only makes sure that network requests being logged have their details and UI properly updated.
String or UUID changes made by this patch: None.
Attachment #631973 - Flags: approval-mozilla-aurora?
Comment on attachment 631973 [details] [diff] [review]
[in-fx-team] address review comment

Approving for aurora landing, will set tracking as well in case we need to backout or followup.
Attachment #631973 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
tracking-firefox15: ? → +
(Assignee)

Comment 12

5 years ago
Comment on attachment 631973 [details] [diff] [review]
[in-fx-team] address review comment

Thank you!

Landed:
https://hg.mozilla.org/releases/mozilla-aurora/rev/72c27b21702e
(Assignee)

Updated

5 years ago
status-firefox15: affected → fixed
You need to log in before you can comment on or make changes to this bug.