Closed Bug 48972 Opened 25 years ago Closed 23 years ago

nsIPluginInstancePeer needs a GetOwner() method

Categories

(Core Graveyard :: Plug-ins, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.2alpha

People

(Reporter: sean, Assigned: serhunt)

Details

(Whiteboard: [PL2:NA])

Attachments

(1 file, 2 obsolete files)

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
I need this in another place too. Perhaps I'll implement it. Is the API frozen?
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.
Right. I think the ugly but safe (without quotation marks) way is to define nsIPluginInstancePeer2 derived from nsIPluginInstancePeer.
> way is to define nsIPluginInstancePeer2 derived from nsIPluginInstancePeer. nsIPluginInstancePeer2 was defined wile ago. (1998/07/12) do you want to define nsIPluginInstancePeer3?
implement new private interface, such as nspiPluginInstancePeer
Target Milestone: Future → mozilla1.0.2
Whiteboard: [PL2:NA]
Attached patch patch v.1 (obsolete) — Splinter Review
Added nsPIPluginInstancePeer interface, some code clean up. Please review.
Status: NEW → ASSIGNED
Whiteboard: [PL2:NA] → [PL2:NA][review needed]
Comment on attachment 93234 [details] [diff] [review] patch v.1 r=peterl
Attachment #93234 - Flags: review+
Whiteboard: [PL2:NA][review needed] → [PL2:NA][superreview needed]
adding nsbeta1+ for buffy
Keywords: nsbeta1+
Target Milestone: mozilla1.0.2 → mozilla1.2alpha
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+
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; }
In the trunk now.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: [PL2:NA][superreview needed] → [PL2:NA]
Reopening bug. It caused regression with Java and has been backed out.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Stupid mistake. I missed one interface in NS_IMPL_ISUPPORTS6(nsPluginInstancePeerImpl,...) macro. The new patch is coming.
Attached patch patch v.1.1 (obsolete) — Splinter Review
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.
Attached patch patch v.2Splinter Review
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 on attachment 96403 [details] [diff] [review] patch v.2 sr=beard
Attachment #96403 - Flags: superreview+
Checked in.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: