Closed Bug 845571 Opened 7 years ago Closed 7 years ago

Turn on console debug logging when running in Metro

Categories

(Firefox for Metro Graveyard :: General, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 22

People

(Reporter: bbondy, Assigned: bbondy)

Details

Attachments

(1 file)

Metro debug builds are unusable for anyone who doesn't know they need to set XPCOM_DEBUG_BREAK=warn.

We should turn that on by default when running in Metro mode only.
IMO, we should make warn the default on windows.
I think there's another bug for that somewhere, but I want to do this at least for metro for now. I think there will be some people who don't agree with warn on desktop but no one will care for only metro.
Should I do this only for Metro or for desktop + metro?
On metro it results in crashing the browser without XPCOM_DEBUG_BREAK=warn, hence this bug.
Flags: needinfo?(benjamin)
It results in crashing because the windbgdlg doesn't work in metro mode? Definitely do this for metro mode. Let's hold off for desktop (at least do it in another bug) so that we can have the proper discussion without holding you up.
Flags: needinfo?(benjamin)
> It results in crashing because the windbgdlg doesn't work in metro mode? 

Yup

OK agreed. Will do, thanks.
Attached patch Patch v1.Splinter Review
Just took the code I had previously written for metro detection in gfx and moved it into xpcom's nsWindowsHelpers. Then used that to determine which default to pick when missing the env variable.  Also only do the check in gfx now when it's a metro build.
Attachment #719015 - Flags: review?(jmathies)
Comment on attachment 719015 [details] [diff] [review]
Patch v1.

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

::: xpcom/base/nsDebugImpl.cpp
@@ +42,5 @@
>  #include <tchar.h>
>  #include "nsString.h"
> +#ifdef MOZ_METRO
> +#include "nsWindowsHelpers.h"
> +#endif

I think this will break since we allow building metro code bits on other platforms. Unless nsWindowsHelpers is wrapped in win32 checks. (I think I need to update my crash reporter patches due to this as well. :)

::: xpcom/base/nsWindowsHelpers.h
@@ +95,5 @@
>      return info.dwMajorVersion >= 6;
>    }
> +
> +  bool
> +  IsRunningInWindows8Metro()

nit - nix the '8'
Attachment #719015 - Flags: review?(jmathies) → review+
> I think this will break since we allow building metro code bits on other
>  platforms. Unless nsWindowsHelpers is wrapped in win32 checks. (I think I need 
> to update my crash reporter patches due to this as well. :)

It's already inside of a #if defined(XP_WIN) section above it so should be fine.

> nit - nix the '8'

Agreed ,will do.
https://hg.mozilla.org/mozilla-central/rev/d64093cf0595
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.