542971 - kill/replace nsPluginInstanceTag

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Josh Aas]

We should kill/replace nsPluginInstanceTag. We don't need it, it adds unnecessary complexity, and it makes our code more error-prone.

Comment 1

[Josh Aas]

Attachment: fix v1.0

This makes our code much nicer. The object hierarchy is more clear and it is easier to get from one object to another. We avoid unnecessary null checks and strong references due to a clarified lifetimes dependencies between objects.

This also fixes a bug where we store plugin instances in nsAutoPtr objects. They should be stored in nsRefPtr objects.

I realize that there is more cleanup to do - this patch makes some of that even more clear. This patch is complex and long enough already, I'll get more stuff later.

Comment 2

[Josh Aas]

Attachment: fix v1.1

Includes Linux build fix.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 424207 [details] [diff] [review]
fix v1.1

If you haven't already, you should push this through try and watch for leaks etc.

Changes look good.

Comment 4

[Josh Aas]

I ran it on the try servers, no problems.

Comment 5

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/f61e06c31086

Comment 6

[Daniel Holbert [:dholbert]]

Backed out per bsmedberg's request:
  http://hg.mozilla.org/mozilla-central/rev/981efc4d2ad2

'hg backout' ended up having merge conflicts, and I don't have a great merge tool, so I used "hg export f61e06c31086 | patch -p1 -R -F10" and then hand-fixed the chunks that patch failed on. It looks like the conflicts were all from bug 543339's patch, "Clean up a bunch of things in nsNPAPIPlugin", which included a number of variable-renamings in code touched by this bug's patch.

Comment 7

[Josh Aas]

Attachment: fix v1.2

Updated for current trunk. When this lands again it'll need to land with the patch on bug 543376.

Comment 8

[Josh Aas]

Attachment: fix v1.3

Updated to current trunk but has known memory leaks. I have fixes for the leaks but I want to archive this version of the patch here anyway.

Comment 9

[Josh Aas]

Attachment: fix v1.5

Comment 10

[Josh Aas]

Attachment: fix v1.6

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 458256 [details] [diff] [review]
fix v1.6

r=jst

As a followup it might be nice to figure out if we could guarantee that there's never a live instance with a plugin that doesn't have valid plugin funcs, so that we wouldn't need to check if the plugin has a library before accessing the plugin funcs etc. But this is a good step forward in and of itself!

Comment 12

[Josh Aas]

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/b242765c4ba8