Pipe additional output to the Windows debugger console

RESOLVED FIXED in mozilla17

Status

()

Core
General
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
mozilla17
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 649136 [details] [diff] [review]
patch

console output, component load, and Dump in frame manager.
(Assignee)

Updated

5 years ago
Attachment #649136 - Flags: review?(neil)

Comment 1

5 years ago
Comment on attachment 649136 [details] [diff] [review]
patch

>+    OutputDebugStringW(aStr.BeginReading());
BeginReading doesn't guarantee null-termination, so use PromiseFlatString(aStr).get() instead. r=me with that fixed. It's a shame that nsGlobalWindow::Dump uses OutputDebugStringA though :-(

>+            nsXPIDLString msg;
I have a slight preference for nsString here because nsXPIDLString::get can return nullptr.

>+            message->GetMessageMoz(getter_Copies(msg));
>+            OutputDebugStringW(msg.get());
>+            OutputDebugStringW(L"\n");
I'd prefer if you appended the newline to the string before outputting it, this makes the debugger output pane update more smoothly (it tries to keep the last character on screen which is hard when the line of text exceeds the window width).
Attachment #649136 - Flags: review?(neil) → review+
(Assignee)

Comment 2

5 years ago
(In reply to neil@parkwaycc.co.uk from comment #1)
> Comment on attachment 649136 [details] [diff] [review]
> patch
> 
> >+    OutputDebugStringW(aStr.BeginReading());
> BeginReading doesn't guarantee null-termination, so use
> PromiseFlatString(aStr).get() instead. r=me with that fixed. It's a shame
> that nsGlobalWindow::Dump uses OutputDebugStringA though :-(

Has that caused problems for non-ascii output? I could probably update it if the wide char data is available there.
(Assignee)

Comment 3

5 years ago
Created attachment 649340 [details] [diff] [review]
updated patch
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac4ee04df5e2

Comment 5

5 years ago
https://hg.mozilla.org/mozilla-central/rev/ac4ee04df5e2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17

Comment 6

5 years ago
(In reply to Jim Mathies from comment #2)
> (In reply to neil@parkwaycc.co.uk from comment #1)
> > >+    OutputDebugStringW(aStr.BeginReading());
> > BeginReading doesn't guarantee null-termination, so use
> > PromiseFlatString(aStr).get() instead. r=me with that fixed. It's a shame
> > that nsGlobalWindow::Dump uses OutputDebugStringA though :-(
> Has that caused problems for non-ascii output? I could probably update it if
> the wide char data is available there.
I don't know, I don't get to dump much non-ASCII output, but the wide char data is certainly available via PromiseFlatString(aStr).get() again ;-)
This makes running Firefox in a debugger super-slow. I'm using a Nightly with WinDBG attached to try to catch an intermittent OOM, and every time I load a page the whole browser is unresponsive because it's feeding tons of error console output to the debugger.
(Assignee)

Comment 8

4 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7)
> This makes running Firefox in a debugger super-slow. I'm using a Nightly
> with WinDBG attached to try to catch an intermittent OOM, and every time I
> load a page the whole browser is unresponsive because it's feeding tons of
> error console output to the debugger.

We could add a build config flag that disables calls to OutputDebugString.
I think I'd be happy if we could just revert the bit that feeds the error console to the debugger. That seems to be what's causing me the most grief.
(Assignee)

Comment 10

4 years ago
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> I think I'd be happy if we could just revert the bit that feeds the error
> console to the debugger. That seems to be what's causing me the most grief.

Hmm, that output is really useful when you're remote debugging metrofx on a Win8 tablet.
You need to log in before you can comment on or make changes to this bug.