Sync plugin instantiation should not pass a runnable to PluginProcessParent::Launch

RESOLVED INVALID

Status

()

Core
Plug-ins
RESOLVED INVALID
3 years ago
3 years ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This isn't a big deal for correctness right now the way things are written, but currently the runnable is executed for both sync and async init, which isn't future proof (and isn't actually needed in the sync case, so it's a waste of time to execute it).
Created attachment 8544272 [details] [diff] [review]
Don't set launch runnable unless we're starting async
Attachment #8544272 - Flags: review?(jmathies)

Comment 2

3 years ago
Comment on attachment 8544272 [details] [diff] [review]
Don't set launch runnable unless we're starting async

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

::: dom/plugins/ipc/PluginModuleParent.cpp
@@ +366,5 @@
>  {
>      PLUGIN_LOG_DEBUG_FUNCTION;
>  
>      nsAutoPtr<PluginModuleChromeParent> parent(new PluginModuleChromeParent(aFilePath, aPluginId));
> +    UniquePtr<LaunchCompleteTask> onLaunchedRunnable;

can we name this something different, it really threw me at first since it looks like some sort of 'onXYZ' event method. Maybe just 'launchRunnable'?
Attachment #8544272 - Flags: review?(jmathies) → review+
Created attachment 8545399 [details] [diff] [review]
Don't set launch runnable unless we're starting async (r2)

Addressed comment. Carrying forward r+.
Attachment #8544272 - Attachment is obsolete: true
Attachment #8545399 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/852f51cb251e
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/8fa274cce616 for various test failures:

https://treeherder.mozilla.org/logviewer.html#?job_id=5181302&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=5182176&repo=mozilla-inbound
https://treeherder.mozilla.org/logviewer.html#?job_id=5181308&repo=mozilla-inbound
Flags: needinfo?(aklotz)
On further inspection of this code, I think that it should be left as-is. My bad.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(aklotz)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.