Remove support for async plugin init

ASSIGNED
Assigned to

Status

()

Core
Plug-ins
ASSIGNED
a month ago
17 days ago

People

(Reporter: bsmedberg, Assigned: njn)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

a month ago
Async plugin init was a feature we tried to enable where the Flash plugin was not synchronously initialized with respect to the page that loaded it. It was bug 998863.

This feature was risky and we never got it into a shippable state. Now that we're moving Flash to click-to-play by default, that risk is unnecessary.

The feature is currently landed in-tree but off by default. In order to simplify a bunch of other code, I'd like us to try and back it out completely. This will e.g. unblock bug 1352573 but also make some things more readable.

aklotz, I'm not asking you to do this, but could you provide advice and maybe mentoring about whether it's possible to actually "back out" individual things from bug 998863 or whether we should read those patches as inspiration for hand-removing code, or some other process.
Flags: needinfo?(aklotz)
(Reporter)

Updated

a month ago
Depends on: 998863
While those patches could definitely provide a roadmap, it's probably best to illustrate the fundamentals here:

* dom/plugins/ipc/PluginAsyncSurrogate* can go. Anything that references PluginAsyncSurrogate or AsyncNPObject can go. Note that there are some references to the affected classes in dom/plugins/base too.

* dom/plugins/ipc/PluginDataResolver.h can optionally go, but you'll need to clean up anything that casts to a PluginDataResolver and replace that with casts to PluginInstanceParent. Look for PluginInstanceParent::Cast and PluginModuleParent::StreamCast.

* nsPluginTag::mSupportsAsyncInit can go

* The declaration of kAsyncInitPref in PluginModuleParent can go, as well as its declaration in all.js

* In PluginModuleParent code, mAsyncInitRv mAsyncInitError mInitOnAsyncConnect mIsStartingAsync mContentParent can all go. Any code that is conditional on those values existing and being true can go.

* AssociatePluginId and LoadPluginResult may be removed from dom/ipc/PContent.ipdl

* AsyncNP_Initialize, AsyncNPP_New, NP_InitializeResult may be removed from PPluginModule.ipdl

* AsyncNPP_NewStreamResult may be removed from PBrowserStream.ipdl

* AsyncNPP_NewResult and AsyncNPP_NewStream may be removed from PPluginInstance.ipdl

I'd tend to think that once that stuff and anything that depends on it is gone, you should be left with sync-only plugin init.
Flags: needinfo?(aklotz)
Also:

* PluginInstanceParent::mUseSurrogate can go.
* PluginModuleParent.cpp: PluginModuleMapping can go; it was only for e10s + async init
(Assignee)

Comment 4

20 days ago
Created attachment 8856370 [details] [diff] [review]
(part 1) - Remove unused supportsAsyncInit arguments and fields

The aAllowAsyncInit argument to PluginModuleParent's constructor is unused.
This patch removes it and various other things that are no longer needed once
it is gone.
Attachment #8856370 - Flags: review?(aklotz)
(Assignee)

Updated

20 days ago
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
(Assignee)

Comment 5

20 days ago
Created attachment 8856371 [details] [diff] [review]
(part 2) - Remove nsPluginTag::mSupportsAsyncInit

It's not used in any useful way.
Attachment #8856371 - Flags: review?(aklotz)
(Assignee)

Comment 6

17 days ago
FYI: I'm most of the way through this. I will likely finish it in the first half of next week.
You need to log in before you can comment on or make changes to this bug.