1081353 - Eliminate PPluginIdentifier

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: refactor-identifiers

In order to make plugin IPC the way we want it for e10s (bug 641685), we need to break up or replace PPluginModule. That protocol manages PPluginIdentifier instances, which are essentially jsids/NPIdentifiers. Initially I considered moving the management of this protocol to PPluginInstance. However, it seems better to just remove this protocol entirely--there doesn't seem to be any reason for it to exist.

I looked at this history for why this protocol exists and I found bug 547359. Before that bug, it looks like the child send an IPC message to the parent every time it wanted to use an identifier. An NPIdentifier in the child process was essentially the same bit representation as in the parent, except that it didn't actually point to anything. Bug 547359 made this more efficient by caching IDs in the child. However, it still sends messages whenever a new identifier was first created on either side.

Fundamentally, it seems that there doesn't need to be any link between identifiers in the plugin process and in the chrome process. The only important invariant is that the plugin process should see only one NPIdentifier for a given string/integer. To do that, we just need some sort of hash consing.

Originally I was planning on hash consing every string turned into an NPIdentifier in the plugin process forever. However, it seems that we assume that the plugin doesn't store NPIdentifiers in the heap (or, if it does, it calls NPN_GetStringIdentifier first). So it should be okay to remove the table entries once a given RPC call has finished, except when NPN_GetStringIdentifier is called. That's pretty much what this patch does.

I also tightened up the rooting of NPIdentifiers in the chrome process. This is still pretty error-prone, and it will get more dangerous when the JS engine starts moving strings. I outlined a plan for handling that in a comment.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cb1c03335d0

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5cb1c03335d0