Closed
Bug 867579
Opened 11 years ago
Closed 2 years ago
move DOMWINDOW/DOCSHELL messages under control of a pref, enabled for testing
Categories
(Core :: DOM: Core & HTML, defect, P5)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
DUPLICATE
of bug 1592297
People
(Reporter: froydnj, Unassigned)
References
Details
Attachments
(1 file, 1 obsolete file)
8.18 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
I've looked at the bits for MOZ_QUIET a number of times but still forget about it when it comes time to do Mozilla development on a different machine. Having it enabled by default just increases debug spew to no real purpose for developers. And if there does happen to be an important message (e.g. JavaScript error), it risks getting drowned out by the DOMWINDOW/DOCSHELL spew. Since it's only needed for tests, let's just enable it for tests. I left the MOZ_QUIET control in content/media/MediaDecoderStateMachine.cpp since it controls something quite different.
Reporter | ||
Comment 1•11 years ago
|
||
I've looked at the bits for MOZ_QUIET a number of times but still forget about it when it comes time to do Mozilla development on a different machine. (I don't even have it enabled on my main machines because, well, it's one more knob to twiddle.) Having it enabled by default just increases debug spew to no real purpose for developers. And if there does happen to be an important message (e.g. JavaScript error), it risks getting drowned out by the DOMWINDOW/DOCSHELL spew. Since it's only needed for tests, let's just enable it for tests. I left the MOZ_QUIET control in content/media/MediaDecoderStateMachine.cpp since it controls something quite different. Yes, I have read the discussion in bug 673252. I'm skeptical that anybody really enjoys having these messages on by default. Tagging bz for review since he reviewed jlebar's change in bug 673252; the testing bits will probably need separate review if/when bz r+'s.
Attachment #744118 -
Flags: review?(bzbarsky)
Reporter | ||
Comment 2•11 years ago
|
||
...and a refresh because I wasn't paying close attention to compiler warnings.
Attachment #744118 -
Attachment is obsolete: true
Attachment #744118 -
Flags: review?(bzbarsky)
Attachment #744120 -
Flags: review?(bzbarsky)
Comment 3•11 years ago
|
||
I'm not sure I'm the best person to ask about this, since I think the messages _should_ in fact be enabled so people will notice leaks....
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #3) > I'm not sure I'm the best person to ask about this, since I think the > messages _should_ in fact be enabled so people will notice leaks.... OK, but do you think that people (DOM/content folks excluded, or even included) really translate from the output to "hey, I caused a leak"? I am skeptical that they do. I'm even more skeptical that every developer needs to be able to do this and thus needs this output on by default. It'd be interesting to know the number of backouts caused by "hey, we leaked a window or docshell" over the last year or so...
Comment 5•11 years ago
|
||
It's possible that my opinion that people _should_ care is not matched by them actually caring, yes.... :( David, thoughts?
Flags: needinfo?(dbaron)
I have mixed feelings: given that we have about:memory now, I'm inclined to be more ok with this than I was before. But it still is one of the few things that might lead people to notice a leak-until-shutdown that gets cleaned up at shutdown, which is a serious problem that we don't have good real testing for (other than loading pages in the Talos Tp harness, which doesn't exercise much UI).
Flags: needinfo?(dbaron)
Comment 7•11 years ago
|
||
Comment on attachment 744120 [details] [diff] [review] move DOMWINDOW/DOCSHELL messages under control of a pref, enabled for testing I guess I can live with this....
Attachment #744120 -
Flags: review?(bzbarsky) → review+
Comment 8•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Assignee | ||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•