remove nsIPluginInstancePeer[1/2/3] and nsPIPluginInstancePeer

RESOLVED FIXED

Status

()

Core
Plug-ins
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

({dev-doc-complete})

Trunk
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

9 years ago
We should remove nsIPluginInstancePeer[1/2/3] and nsPIPluginInstancePeer. Anything we actually use from them can be folded into nsIPluginInstance.
(Assignee)

Comment 1

9 years ago
Created attachment 385546 [details] [diff] [review]
part 1, v1.0

This gets rid of nsIPluginInstancePeer2 and nsIPluginInstancePeer3, which makes removing nsIPluginInstancePeer and nsPIPluginInstancePeer easier.
Attachment #385546 - Flags: superreview?(jst)
(Assignee)

Comment 2

9 years ago
Created attachment 385573 [details] [diff] [review]
part 1, v1.1

Windows build fix.
Attachment #385546 - Attachment is obsolete: true
Attachment #385573 - Flags: superreview?(jst)
Attachment #385546 - Flags: superreview?(jst)

Updated

9 years ago
Attachment #385573 - Flags: superreview?(jst)
Attachment #385573 - Flags: superreview+
Attachment #385573 - Flags: review+
Comment on attachment 385573 [details] [diff] [review]
part 1, v1.1

Wanna add removal of the type nsMIMEType to your list of things to do when cleaning up more around here? :)

r+sr=jst
(Assignee)

Comment 4

9 years ago
Created attachment 385669 [details] [diff] [review]
part 1, v1.2

Includes OS/2 fix, merge to tip.

pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/b68e78af5d88
Attachment #385573 - Attachment is obsolete: true
(Assignee)

Comment 5

9 years ago
Created attachment 385689 [details] [diff] [review]
part 2, v1.0

This is part two, completely removes plugin peers. This patch works, but needs cleanup.
(Assignee)

Comment 6

9 years ago
Created attachment 385705 [details] [diff] [review]
part 2, v1.1

Includes UNIX/GTK fixes.
Attachment #385689 - Attachment is obsolete: true
(Assignee)

Comment 7

9 years ago
Created attachment 385764 [details] [diff] [review]
part 2, v1.2

Includes Windows build fixes and general cleanup.

I realize there is more cleanup/optimization we could do as a result of this work but I'd rather do that later. For right now it is best to make the transition away from peers as simple as possible.
Attachment #385705 - Attachment is obsolete: true
Attachment #385764 - Flags: superreview?(jst)
Comment on attachment 385764 [details] [diff] [review]
part 2, v1.2

Looks good. Found a couple of things you can address before landing this, unless you have other work in the pipe that will make these issues go away...

+nsresult
+nsPluginStreamToFile::QueryInterface(const nsIID& aIID,
+                                     void** aInstancePtrResult)
+{

Wanna make this use a QI macro while you're here?

- In nsNPAPIPluginInstance::GetTagType():

+    nsIPluginTagInfo2 *tinfo;
+    nsresult rv = mOwner->QueryInterface(kIPluginTagInfo2IID, (void **)&tinfo);

Make this use an nsCOMPtr and remove the definition of kIPluginTagInfo2IID while you're moving this code. Same for GetParameters() and GetDOMElement().

- In nsNPAPIPluginInstance::GetAttributes():

+    nsIPluginTagInfo *tinfo;
+    nsresult rv = mOwner->QueryInterface(kIPluginTagInfoIID, (void **)&tinfo);

Same thing for kIPluginTagInfoIID.

+    if (NS_SUCCEEDED(rv))  {
+      rv = tinfo->GetAttributes(n, names, values);
+      NS_RELEASE(tinfo);
+    }
+    return rv;
+  } else {

Remove the else after return as well. Same thing in GetTagType() above and GetParameters below.

- In nsPluginHostImpl::HandleBadPlugin():

+  if (aInstance)
+      aInstance->GetOwner(getter_AddRefs(owner));

Over indented.

r+sr=jst
Attachment #385764 - Flags: superreview?(jst)
Attachment #385764 - Flags: superreview+
Attachment #385764 - Flags: review+
(Assignee)

Comment 9

9 years ago
Created attachment 385811 [details] [diff] [review]
part 2, v1.3

Address comments from jst.
Attachment #385764 - Attachment is obsolete: true
(Assignee)

Comment 10

9 years ago
part 2 pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/d51a8d3295d0
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

9 years ago
windows bustage fix pushed

http://hg.mozilla.org/mozilla-central/rev/dfcf90e5813d
Keywords: dev-doc-needed
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.