1115436 - Notify nsObjectLoadingContent when asynchronous plugin initialization fails

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

According to https://bugzilla.mozilla.org/show_bug.cgi?id=998863#c62, we need to call into nsObjectLoadingContent if initialization fails.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: 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?

Comment 2

[John Schoenick [:johns]]

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

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Async plugin init is dead and NPAPI is dying, so resolving.