Closed Bug 1317822 Opened 3 years ago Closed 3 years ago

Move GMPCrashHelper into its own file

Categories

(Core :: Audio/Video: GMP, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(1 file)

Moving GMPCrashHelper into its own header file would mean we don't need to #include "GMPService.h" in a bunch of places, which would speed up compilation and make it easier to add stuff internal to GMPService to its own header.
FYI, I need this change as I need to #include "GMPContentParent.h" in GMPService.h for bug 1316215, and that fails to build in some dirs that don't setup the include paths for dom/media/gmp.
Comment on attachment 8811052 [details]
Bug 1317822 - Move GMPCrashHelper into its own file.

https://reviewboard.mozilla.org/r/93292/#review93292

r+ with or without a separate cpp, as you see fit:

::: dom/media/gmp/GMPCrashHelper.h:20
(Diff revision 1)
> +// For every GMP actor requested, the caller can specify a crash helper,
> +// which is an object which supplies the nsPIDOMWindowInner to which we'll
> +// dispatch the PluginCrashed event if the GMP crashes.
> +// GMPCrashHelper has threadsafe refcounting. Its release method ensures
> +// that instances are destroyed on the main thread.
> +class GMPCrashHelper

I see you have left the implementation in GMPService.cpp.
I would have a slight preference for a dedicated cpp to match this header, but since it's so small, I don't really mind if you just want to keep it this way.
(And I was going to suggest adding comments to point between header and implementation, but it's easy enough to find these through IDEs or DXR/searchfox, so no real need for comments either.)
Attachment #8811052 - Flags: review?(gsquelart) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e47666dbac8
Move GMPCrashHelper into its own file. r=gerald
https://hg.mozilla.org/mozilla-central/rev/3e47666dbac8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.