Closed Bug 1317822 Opened 3 years ago Closed 3 years ago

Move GMPCrashHelper into its own file


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




Tracking Status
firefox53 --- fixed


(Reporter: cpearce, Assigned: cpearce)




(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.

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
Move GMPCrashHelper into its own file. r=gerald
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.