nsIPluginInstancePeer needs a GetOwner() method

VERIFIED FIXED in mozilla1.2alpha

Status

()

P3
normal
VERIFIED FIXED
19 years ago
16 years ago

People

(Reporter: sean, Assigned: serhunt)

Tracking

Trunk
mozilla1.2alpha
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [PL2:NA])

Attachments

(1 attachment, 2 obsolete attachments)

11.87 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

19 years ago
see the reinterpret casts in nsPluginHostImpl::NewPluginURLStream,
nsPluginHostImpl:GetURL and nsPluginHostImpl::PostURL at

http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.cpp
Not a Netscape 6 RTM blocker. FUTURE. This bug has been marked Future because
the Netscape engineer it is assigned to is overburdened.
Target Milestone: --- → Future

Comment 2

18 years ago
I need this in another place too. Perhaps I'll implement it. Is the API frozen?
(Reporter)

Comment 3

18 years ago
Plugin API frozen?  sort of.  they didn't go throught the API review process 
due to Sun having distributed a plugin before last summer based on the API.  
The API was subsequently declared frozen after an attempt to IDL'ify it failed.

If you only add to the end of the interface definitions that exist, then 
you're 'safe'.  The alternative is to create a new interface that has the new 
stuff which are derived from the frozen interfaces, then replace all the uses 
of the old interface with the new one where needed.
(Assignee)

Comment 4

18 years ago
Right. I think the ugly but safe (without quotation marks) way is to define 
nsIPluginInstancePeer2 derived from nsIPluginInstancePeer.

Comment 5

18 years ago
> way is to define nsIPluginInstancePeer2 derived from nsIPluginInstancePeer.

nsIPluginInstancePeer2 was defined wile ago. (1998/07/12)
do you want to define nsIPluginInstancePeer3?

Comment 6

17 years ago
implement new private interface, such as nspiPluginInstancePeer
Target Milestone: Future → mozilla1.0.2

Updated

17 years ago
Whiteboard: [PL2:NA]
(Assignee)

Comment 7

17 years ago
Created attachment 93234 [details] [diff] [review]
patch v.1

Added nsPIPluginInstancePeer interface, some code clean up. Please review.
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: [PL2:NA] → [PL2:NA][review needed]

Comment 8

17 years ago
Comment on attachment 93234 [details] [diff] [review]
patch v.1

r=peterl
Attachment #93234 - Flags: review+
(Assignee)

Updated

17 years ago
Whiteboard: [PL2:NA][review needed] → [PL2:NA][superreview needed]

Comment 9

17 years ago
adding nsbeta1+ for buffy
Keywords: nsbeta1+

Updated

17 years ago
Target Milestone: mozilla1.0.2 → mozilla1.2alpha

Comment 10

17 years ago
Comment on attachment 93234 [details] [diff] [review]
patch v.1

sr=beard, nit about using an nsCOMPtr<> parameter on an interface method, but
it is a private interface. BTW, why did you need to do a QueryInterface() in
GetOwner() instead of a simple assignment?
Attachment #93234 - Flags: superreview+
(Assignee)

Comment 11

17 years ago
Right. There is no need to QI since the pointer is of the same type. I will
change the function to be the following:

NS_IMETHODIMP 
nsPluginInstancePeerImpl::GetOwner(nsCOMPtr<nsIPluginInstanceOwner> &aOwner)
{
  aOwner = mOwner; // this assignment will addref
  return (mOwner) ? NS_OK : NS_ERROR_FAILURE;
}
(Assignee)

Comment 12

17 years ago
In the trunk now.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: [PL2:NA][superreview needed] → [PL2:NA]
(Assignee)

Comment 13

17 years ago
Reopening bug. It caused regression with Java and has been backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 14

17 years ago
Stupid mistake. I missed one interface in
NS_IMPL_ISUPPORTS6(nsPluginInstancePeerImpl,...) macro. The new patch is coming.
(Assignee)

Comment 15

17 years ago
Created attachment 96333 [details] [diff] [review]
patch v.1.1

This is a new patch for nsPluginInstancePeer.cpp, all other files are intact
from the previous patch. I added missing |nsIPluginTagInfo| to
NS_IMPL_ISUPPOTS.
(Assignee)

Comment 16

17 years ago
Created attachment 96403 [details] [diff] [review]
patch v.2

Some files from the patch v.1 has already been checked in (those in
base/public) so this patch is against the current tree.

Things changed:

1. corrected typo having caused Java plugin regression -- added forgotten
|nsIPluginTagInfo| interface

2. using nsCOMPtr<> as arguments in interface methods is not what COMPtr
guidelines suggest -- changed |nsPIPluginInstancePeer::GetOwner| to taking a
raw pointer

3. fixed a warning complaint about ";" empty control statement in
nsPluginHostImpl.cpp -- NS_TIMELINE_START_TIMER got { } around it.
Attachment #93234 - Attachment is obsolete: true
Attachment #96333 - Attachment is obsolete: true

Comment 17

17 years ago
Comment on attachment 96403 [details] [diff] [review]
patch v.2

sr=beard
Attachment #96403 - Flags: superreview+
(Assignee)

Comment 18

17 years ago
Checked in.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 19

16 years ago
.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.