151920 - nsPluginHostImpl::AddInstanceToActiveList does not return failure if 'new nsActivePlugin' failed

Status: RESOLVED FIXED

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

Description

[Peter Lubczynski]

From bug 140931:
------- Additional Comment #45 From Patrick C. Beard  2002-06-13 16:37 -------

(From update of attachment 86297 [details] [diff] [review])
nsPluginHostImpl::AddInstanceToActiveList() should return an nsresult code.

...the caller of this has no way of knowing memory allocation may have failed.

Comment 1

[rubydoo123]

pushing to future for now

Comment 2

[av (gone)]

Taking. Let's get rid of easy-to-fix-but-annoying-to-have bugs.

Comment 3

[av (gone)]

Attachment: patch v.1

Peter, I also put back a flag saying that the instance is a default plugin in
|nsPluginHostImpl::SetUpDefaultPluginInstance|. I think it was cut-and-paste
type of mistake when you checked in a fix for bug 140931, here:

-  AddInstanceToActiveList(plugin, instance, aURL, PR_TRUE);
+  AddInstanceToActiveList(plugin, instance, aURL, PR_FALSE, pIpeer);

If this has been done on purpose please speak up and I'll correct for it.

Comment 4

[serge (gone)]

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

r=serge

Comment 5

[Darin Fisher]

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


>Index: nsPluginHostImpl.cpp

>   (void)aURL->GetSpec(url);

just a side question: do you care if this result is non-ASCII?	if you
expect ASCII, then you should call GetAsciiSpec instead.

what does this object expect?

>   nsActivePlugin * plugin = new nsActivePlugin(pluginTag, aInstance, url.get(), aDefaultPlugin, peer);

>+  if (NS_FAILED(result))
>+    return result;
> 
>   return NS_OK;
> }

how about "return result" instead of these 3 lines of code?

Comment 6

[av (gone)]

Darin: no, we don't care about url spec being non-ASCII. We will likely rewrite
this code soon anyway to use nsIURI.
As to |return result| -- I also noticed this leftover after I posted the patch,
I will change it before I check in.

Comment 7

[Darin Fisher]

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

sr=darin

Comment 8

[av (gone)]

In the trunk.