Closed Bug 1316222 Opened 4 years ago Closed 4 years ago

Disable the crash guard in the GPU process

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

Attachments

(2 files, 1 obsolete file)

This hasn't been made to work yet (it's disabled in Nightly builds so hasn't caused problems yet) and is basically obseleted when we have the GPU process.

Let's just disable it for now.
Attachment #8808898 - Flags: review?(dvander)
Attached patch dont-sync-dispatch-gpu-process (obsolete) — Splinter Review
If the crash guard isn't enabled, then we don't need to sync dispatch to the main thread to initialize DXVA.

This fixes a bug that tryserver found, where NS_DISPATCH_SYNC spins the event loop of the current thread while it waits, and we end up trying to handle Shutdown() (on VideoDecoderParent) while we're still in the middle of Init.
Attachment #8808899 - Flags: review?(jyavenard)
Attachment #8808898 - Flags: review?(dvander) → review+
Comment on attachment 8808899 [details] [diff] [review]
dont-sync-dispatch-gpu-process

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

I can understand that we don't need to do anything on the main thread (as that the crashguard is the only reason we need to run on the main thread anyway).

But why not use SyncRunnable instead? this doesn't start an event loop.

Otherwise r=me with the telemetry stuff put back

::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp
@@ +125,5 @@
>                         mGotValidOutputAfterNullOutput ? 3 :
>                         4;
>  
> +  LOG(nsPrintfCString("Reporting telemetry VIDEO_MFT_OUTPUT_NULL_SAMPLES=%d", telemetry).get());
> +  Telemetry::Accumulate(Telemetry::ID::VIDEO_MFT_OUTPUT_NULL_SAMPLES, telemetry);

this seems unrelated... this isn't a sync dispatch.

And I believe telemetry needs to be run on the main thread regardless.
Attachment #8808899 - Flags: review?(jyavenard) → review+
Attached patch sync-runnableSplinter Review
Trying again using SyncRunnable.

Unfortunately the DXVA blacklist code uses ClearOnShutdown which is main thread only, so the previous patch had issues on try.
Attachment #8808899 - Attachment is obsolete: true
Attachment #8809199 - Flags: review?(jyavenard)
Attachment #8809199 - Flags: review?(jyavenard) → review+
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccc2831433b6
Use SyncRunnable instead of NS_DISPATCH_SYNC in WMFVideoMFTManager since the latter can spin the event loop. r=jya
https://hg.mozilla.org/integration/mozilla-inbound/rev/f210f6b4c5cd
Disable the crash guard in the GPU process since it won't be useful. r=dvander
https://hg.mozilla.org/mozilla-central/rev/ccc2831433b6
https://hg.mozilla.org/mozilla-central/rev/f210f6b4c5cd
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.