Closed Bug 1173972 Opened 4 years ago Closed 4 years ago

Improve logging in gfxWindowPlatform

Categories

(Core :: Graphics, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Details

(Whiteboard: gfx-noted)

Attachments

(1 file)

No description provided.
Attached patch patchSplinter Review
Attachment #8621239 - Attachment is patch: true
Attachment #8621239 - Flags: review?(milan)
Comment on attachment 8621239 [details] [diff] [review]
patch

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

The code is good, let me check the intention.  With the app notes, there was a guarantee we would see these messages.  With critical error, they may get rotated out (except for the very first one), if there are enough critical errors following it.  Are the messages we are replacing such that this is OK?
(In reply to Milan Sreckovic [:milan] from comment #2)
> Comment on attachment 8621239 [details] [diff] [review]
> patch
> 
> Review of attachment 8621239 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The code is good, let me check the intention.  With the app notes, there was
> a guarantee we would see these messages.  With critical error, they may get
> rotated out (except for the very first one), if there are enough critical
> errors following it.  Are the messages we are replacing such that this is OK?

I think we're fine. The first one should be sufficient.
Comment on attachment 8621239 [details] [diff] [review]
patch

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

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1943,5 @@
>        adapter = nullptr;
>      }
>  
>      if (FAILED(hr) || !DoesD3D11DeviceWork(mD3D11Device)) {
> +      gfxCriticalError() << "D3D11 device creation failed" << hr;

Not sure if this will compile, but using hexa(hr) will.
Attachment #8621239 - Flags: review?(milan) → review+
OS: Unspecified → Windows
Whiteboard: gfx-noted
https://hg.mozilla.org/mozilla-central/rev/41936f46f3cb
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8621239 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: None
[User impact if declined]: Harder to diagnose graphics problems/crashes
[Describe test coverage new/current, TreeHerder]: On m-c
[Risks and why]: Basically none.
Attachment #8621239 - Flags: approval-mozilla-aurora?
Comment on attachment 8621239 [details] [diff] [review]
patch

Anything to help you with that guys. Taking it.
Attachment #8621239 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.