285751 - Move nsSchemaLoader from nsSupportsHashtable to nsInterfaceHashtable

Status: RESOLVED FIXED

(Core Graveyard :: Web Services, defect)

Description

[Doron Rosenberg (IBM)]

Comment 1

[Doron Rosenberg (IBM)]

Attachment: patch, tested and seems to work fine.

Comment 2

[Doron Rosenberg (IBM)]

*** Bug 285625 has been marked as a duplicate of this bug. ***

Comment 3

[Peter Van der Beken [:peterv]]

Comment on attachment 177141 [details] [diff] [review]
patch, tested and seems to work fine.

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

> nsBuiltinSchemaCollection::nsBuiltinSchemaCollection()
> {
>+  mBuiltinTypesHash.Init();
>+  mSOAPTypeHash.Init();

These can fail. You need to add an Init method to nsBuiltinSchemaCollection
with these two calls and switch to NS_GENERIC_FACTORY_CONSTRUCTOR_INIT in the
module file.

> nsBuiltinSchemaCollection::~nsBuiltinSchemaCollection()
> {

>+  mBuiltinTypesHash.Clear();
>+  mSOAPTypeHash.Clear();

No need for this, nsInterfaceHashtable's destructor should clean up for you.

>     nsSchemaBuiltinType* builtin = new nsSchemaBuiltinType(typeVal);
>     if (!builtin) {
>       return NS_ERROR_OUT_OF_MEMORY;
>     }
>-    sup = builtin;
>-    mBuiltinTypesHash.Put(&key, sup);
>-    
>+
>+    mBuiltinTypesHash.Put(aName, builtin);
>+
>     *aType = builtin;
>     NS_ADDREF(*aType);

Make builtin an nsCOMPtr<nsISchemaType> and instead of addref'ing do
builtin.swap(*aType)


> nsSchemaLoader::nsSchemaLoader()
> {
>   mBuiltinCollection = do_GetService(NS_BUILTINSCHEMACOLLECTION_CONTRACTID);
>+  mSchemas.Init();

See above.

> }
> 
> nsSchemaLoader::~nsSchemaLoader()
> {
>+  mSchemas.Clear();

See above.

> nsSchemaLoader::GetSchema(const nsAString & targetNamespace, 
>                           nsISchema **_retval)
> {
>   NS_ENSURE_ARG_POINTER(_retval);
> 
>-  nsStringKey key(targetNamespace);
>-  nsCOMPtr<nsISupports> sup = dont_AddRef(mSchemas.Get(&key));
>-  nsCOMPtr<nsISchema> schema(do_QueryInterface(sup));
>-
>-  if (!schema) {
>+  if (!mSchemas.Get(targetNamespace, _retval)) {
>     return NS_ERROR_SCHEMA_UNKNOWN_TARGET_NAMESPACE;  // no schema for specified targetname
>   }
> 
>-  *_retval = schema;
>-  NS_ADDREF(*_retval);
>-
>   return NS_OK;

I'd just do

  return mSchemas.Get(targetNamespace, _retval) ? NS_OK :
NS_ERROR_SCHEMA_UNKNOWN_TARGET_NAMESPACE;

Wanna change _retval to aResult while you're here?

>+  nsISchema * os;
>+  if (mSchemas.Get(targetNamespace, &os)) {
>     *_retval = os;
>     NS_ADDREF(*_retval);
>     
>     return NS_OK;
>   }

This will leak, remove the NS_ADDREF.

Comment 4

[Doron Rosenberg (IBM)]

Attachment: with comments

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 177423 [details] [diff] [review]
with comments

>Index: extensions/webservices/schema/src/nsSchemaLoader.cpp
>===================================================================

>@@ -875,42 +852,42 @@ nsSchemaLoader::ProcessSchemaElement(nsI

>-  *_retval = schemaInst;
>-  NS_ADDREF(*_retval);
>+  *aResult = schemaInst;
>+  NS_ADDREF(*aResult);

Since you're touching this, make it:

  NS_ADDREF(*aResult = schemaInst);

Comment 6

[Doron Rosenberg (IBM)]

checked in.