Closed Bug 867579 Opened 9 years ago Closed 2 months ago

move DOMWINDOW/DOCSHELL messages under control of a pref, enabled for testing

Categories

(Core :: DOM: Core & HTML, defect, P5)

defect

Tracking

()

RESOLVED DUPLICATE of bug 1592297

People

(Reporter: froydnj, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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)
...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)
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....
(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...
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 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+
Blocks: fx-noise
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
Component: DOM → DOM: Core & HTML
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1592297
You need to log in before you can comment on or make changes to this bug.