48972 - nsIPluginInstancePeer needs a GetOwner() method

Status: VERIFIED FIXED

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

Description

[sean echevarria]

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

http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.cpp

Comment 1

[ekrock's old account (dead)]

Not a Netscape 6 RTM blocker. FUTURE. This bug has been marked Future because
the Netscape engineer it is assigned to is overburdened.

Comment 2

[Peter Lubczynski]

I need this in another place too. Perhaps I'll implement it. Is the API frozen?

Comment 3

[sean echevarria]

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.

Comment 4

[av (gone)]

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

Comment 5

[Igor Kushnirskiy]

> way is to define nsIPluginInstancePeer2 derived from nsIPluginInstancePeer.

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

Comment 6

[rubydoo123]

implement new private interface, such as nspiPluginInstancePeer

Comment 7

[av (gone)]

Attachment: patch v.1

Added nsPIPluginInstancePeer interface, some code clean up. Please review.

Comment 8

[Peter Lubczynski]

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

r=peterl

Comment 9

[rubydoo123]

adding nsbeta1+ for buffy

Comment 10

[Patrick C. Beard]

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?

Comment 11

[av (gone)]

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;
}

Comment 12

[av (gone)]

In the trunk now.

Comment 13

[av (gone)]

Reopening bug. It caused regression with Java and has been backed out.

Comment 14

[av (gone)]

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

Comment 15

[av (gone)]

Attachment: 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.

Comment 16

[av (gone)]

Attachment: 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.

Comment 17

[Patrick C. Beard]

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

sr=beard

Comment 18

[av (gone)]

Checked in.

Comment 19

[shrirang khanzode]

.