Closed Bug 1115436 Opened 10 years ago Closed 8 years ago

Notify nsObjectLoadingContent when asynchronous plugin initialization fails


(Core Graveyard :: Plug-ins, defect)

Not set


(Not tracked)



(Reporter: bugzilla, Assigned: bugzilla)




(1 file)

According to, we need to call into nsObjectLoadingContent if initialization fails.
Blocks: asyncplugininit
No longer blocks: 998863
Attached patch Patch?Splinter Review
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]

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 @@
> +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.
Closed: 8 years ago
Resolution: --- → WONTFIX
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.