Notify nsObjectLoadingContent when asynchronous plugin initialization fails

RESOLVED WONTFIX

Status

()

Core
Plug-ins
RESOLVED WONTFIX
3 years ago
8 months ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

According to https://bugzilla.mozilla.org/show_bug.cgi?id=998863#c62, we need to call into nsObjectLoadingContent if initialization fails.
Blocks: 1116806
No longer blocks: 998863
Created attachment 8560116 [details] [diff] [review]
Patch?

I really don't know what I'm doing here, so please feel free to consider this as a feedback request instead of a review! ;-)

Do I need to create a new fallback type, or is the 'crashed' type acceptable?
Attachment #8560116 - Flags: review?(john)
Comment on attachment 8560116 [details] [diff] [review]
Patch?

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

This looks fine with notes below: we only want PluginDestroyed *or* StopPluginInstance depending on what the plugin's state is at this point, and it should probably be in the InitFailed call.

::: dom/base/nsObjectLoadingContent.cpp
@@ +2770,5 @@
> +NS_IMETHODIMP
> +nsObjectLoadingContent::PluginAsyncInitFailed()
> +{
> +  LOG(("OBJLC [%p]: Plugin Async Init failed", this));
> +  NS_ASSERTION(mType == eType_Plugin, "PluginAsyncInitFailed at non-plugin type");

If StopPluginInstance() is called on the async-starting instance, it is impossible to then reach this state, right? It would be bad if we had a race and this happened:

- Start loading plugin
- Content mutates, LoadObject() on new plugin, StopPluginInstance() on old (still async-starting) plugin
- Old plugin fails to init, calls this, we stomp on newly-loading instance.

@@ +2772,5 @@
> +{
> +  LOG(("OBJLC [%p]: Plugin Async Init failed", this));
> +  NS_ASSERTION(mType == eType_Plugin, "PluginAsyncInitFailed at non-plugin type");
> +
> +  PluginDestroyed();

PluginDestroyed is for when the plugin instance dies from under us, and just cleans up the instance owner and nulls it.

What is the state of the plugin here? If there is still a plugin instance that needs Stop called to fully shut down/go away, that call should be moved here and PluginDestroyed becomes redundant. If that happens due to the init fail, then just PluginDestroyed is enough.

(If the plugin is in any way running, this becomes re-entrant, and is bad)

@@ +2775,5 @@
> +
> +  PluginDestroyed();
> +
> +  // Switch to fallback/crashed state, notify
> +  LoadFallback(eFallbackCrashed, true);

This should be fine, assuming we don't want different UI for init fail vs a "real" crash.

::: dom/plugins/base/nsPluginInstanceOwner.cpp
@@ +1452,5 @@
>  nsPluginInstanceOwner::NotifyHostAsyncInitFailed()
>  {
>    nsCOMPtr<nsIObjectLoadingContent> content = do_QueryInterface(mContent);
> +  if (content) {
> +    content->StopPluginInstance();

This call should probably be inside PluginAsyncInitFailed (StopPluginInstance should be made private at some point, it is re-entrant and can break OBJLC's state if misused).
Attachment #8560116 - Flags: review?(john) → review+
Async plugin init is dead and NPAPI is dying, so resolving.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.