Closed
Bug 1115436
Opened 10 years ago
Closed 8 years ago
Notify nsObjectLoadingContent when asynchronous plugin initialization fails
Categories
(Core Graveyard :: Plug-ins, defect)
Core Graveyard
Plug-ins
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: bugzilla, Assigned: bugzilla)
References
Details
Attachments
(1 file)
3.54 KB,
patch
|
johns
:
review+
|
Details | Diff | Splinter Review |
According to https://bugzilla.mozilla.org/show_bug.cgi?id=998863#c62, we need to call into nsObjectLoadingContent if initialization fails.
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
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 2•10 years ago
|
||
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+
Assignee | ||
Comment 3•8 years ago
|
||
Async plugin init is dead and NPAPI is dying, so resolving.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•