After the SOAP rewrite, and before the new SOAP is added to the installers, it needs to be in daily builds for Tinderbox for a while.

Tentatively scheduling for 0.9.7.

Target milestone changed.

Things to do here, as we discussed in our team meeting, added with a couple of points I just realized: 1. Put in the same restrictive model XMLHttpRequest uses. 2. Make the open access a pref (no UI needed at this point). 3. Get code reviews and super reviews. 4. Turn on in default builds. 5. Write some documentation about our SOAP implementation, mentioning the pref etc. (I can put this on mozilla.org but you'd need to write the (short) doc). One side effect of turning tyhis on in the default builds is that you will then be required to go through the review and super review process, which would probably slow you down a little. But it is starting to look like most of the features are there, so it probably wouldn't matter that much. Also, it would be a good time to get this on to get the much needed test coverage. After these steps are done and this bug is closed, bug 71394 would probably take care of the correct security model.

This is from peterv.

Windows and Linux makefile changes to separate soap and schema from wsdl and proxy. MOZ_SOAP remains the environment variable to define to get low-level SOAP. MOZ_WSP gets you the high-level stuff.

Some things to fix, compiler warns about these too: nsSchemaComplexType.cpp: Reorder initializers in nsSchemaComplexType constructor to match declaration order. nsDOMUtils.h: Reorder initializeres in nsChildElementIterator constructor to match declaration order. nsSchemaLoader.cpp: LoadListener class needs virtual destructor. There are also other warnings in wspproxytest.cpp, nsWSDLLoader.cpp and nsSOAPBlock.cpp. These were with MOZ_SOAP and MOZ_WSP on, so some warnings we can ignore for the purposes of this bug.

haven't tested yet

haven't tested yet, but I already noticed there is some junk generated in the exported makefiles.

Mac build system 10, me 1. _xmlsoap.mcp always fails in build; can't find xpidl generated header files. _xmlwsp.mcp works fine, though. Peter, could you help out here...?

Review comments regarding the interfaces: 1. Change the license template in all files to NPL/LGPL (see for example nsIXMLHttpRequest.idl). 2. In IDL files, this kind of pattern could use some whitespace: interface nsISOAPTransportListener:nsISupports There are some other instancies where more whitespace would be nice. 3. It would be nice if you could indent the interface method comments. 4. Instead of using AString in interfaces, could you perhaps use DOMString? I think the xpidl compiler will output nsAString in its place too. That way you would not need to use yucky the C++ section in the IDL file. 5. nsISOAPParameter is an empty interface - do we really need it? 6. What is this magic number, is it defined to be like this somewhere? const unsigned short VERSION_UNKNOWN = 32767; 7. I do not like the use NS_ERROR_MODULE_GENERAL when creating your own error values (for example in nsISchemaLoader.idl), because it is reserved for 3rd party developers who have only read access to our code. Not sure what would be the best way though, since this code lives in mozilla/extensions... 8. The schema interfaces need some comments. 9. I dislike using PRInt32 etc. in interfaces, long (in this case) could be used as well.

Btw, some of the interface comments, like 1., apply to all files, including source, so please keep that in mind. Comments reading the source: 10. Do not use tabs. Please check all files. At least nsSOAPUtils.h uses tabs. 11. You might want to make the constructor & destructor private for nsSOAPUtils because it looks like you are only using the static members. 12. I am not sure the static data members will work correctly on all platforms with this pattern. But I guess we could try this now and see who complains... 13. I did not yet check the usage, but is it safe that realXMLNamespacePrefix (for example) is defined as "xmlns:" (notice the colon). What about default namespaces? 14. You might want to guard certain functions so that even if they return error nsresult, you reset the input/output variables to something. For example, nsSOAPUtils::GetElementTextContent() can return an error, but in that case it will not truncate the output string parameter. 15. There is no need to fix this now, but I think you should file a new bug: string use optimizations in SOAP. Usually when you are using Left(), Mid(), Right() etc. functions to extract and create a new string, you could make do with a nsDependentString (which would avoid the copy). 16. Call nsString::Truncate() with no parameters if you want to make the string empty. This is probably just a style thing, I don't think there is actually any harm in calling Truncate(0). Will continue...

17. Unused protected section in nsSOAPResponse class definition. 18. Be consistent with how you start a function. i.e. on which line do you put the opening {. I think the normal convention is to put it on a line of its own. There are some mixups in nsSOAPPropertyBag.cpp for example. 19. nsSOAPPropertyBagMutator() does not check if mSOAPBag exists, the new could have failed in the constructor. Perhaps introduce an Init() function to catch the case where new failed?

20. It can be dangerous to declare variables in for loops like this: for (PRUint32 i = 0; because some compilers scope i differentely (either inside or outside the for loop). If you are not using i again in the function it does not matter, but it would be safer to always declare the variable before the loop than doing it like above. 21. Some variables could be defined closer to where they are used, for example mustUnderstand in nsSOAPMessage::Encode() could be moved into the if-clause where it is used. 22. This is bad: nsCOMPtr < nsIDOMNode > node1 = (nsIDOMElement *) element; Do this instead: nsCOMPtr < nsIDOMNode > node1 = do_QueryInterface(element); Also in general, if you notice you are using casts try to at least replace them with NS_STATIC_CASTS. You should almost never need to cast a nsCOMPtr to another nsCOMPtr. 23. There are cases where you do: nsresult nsSOAPMessage::GetEncodingWithVersion(nsIDOMElement * aFirst, PRUint16 * aVersion, nsISOAPEncoding ** aEncoding) { nsCOMPtr < nsIDOMElement > element = aFirst; I think that will invoke QI and AddRef. In these kinds of cases you could write the last line as: nsCOMPtr < nsIDOMElement > element(dont_QueryInterface(aFirst)); which will avoid the needless QI.

24. In nsSOAPMessage::GetHeaderBlocks() you do not check if you have a valid 'memory' variable before using it. Check also nsSOAPMessage::GetParameters(). 25. Also in above methods you may have allocated something into the output parameter and return error without clearing it - this is mlk.

26. Instead of having several PRBools as data members, make the PRPackedBools and place them next to each other so we'll save a little bit of memory. See nsSOAPBlock.h for instance. 27. I did not yet check how these are used, but it seemed strange that nsDefaultSOAPEncoder_1_1 and nsDefaultSOAPEncoder_1_2 classes have no members.

28. It looks like EncodeSimpleValue() may return an error, but still have partially valid data in the output parameter (potential mlk). Maybe introduce a temporary variable and assign from that to output if everything went correctly? 29. It seems strange that in HasSimpleValue() you do GetSchemaType() and then QI to the type - shouldn't you just be able to QI directly without asking for the type first? This pattern is repeated in other methods as well. 30. This line: *aResult = typevalue == nsISchemaComplexType::CONTENT_MODEL_SIMPLE; would be a bit easier to read with some parenthesis: *aResult = (typevalue == nsISchemaComplexType::CONTENT_MODEL_SIMPLE); 31. nsDefaultEncoder::Encode() should probably set the return value to nsnull in case of error. This may happen in other methods as well. 32. Rather than calling AddRef() & Release() directly, it would be better to use the NS_ADDREF() and NS_RELEASE() macros because they provide us refcount info for debugging. Also, if you do NS_ADDREF followed by Release() on a pointer, our tools tell us we leaked something... See for example GetArrayType() where this happens (there are other places as well). 33. You might need to guard against null parameters, for example in nsDefaultEncoder::Decode(). 34. You may use variable 'simple' unitialized in nsAnyTypeEncoder::Decode(). 35. In the same function you have: simple = child == nsnull; where simple is PRBool and child is nsCOMPtr, which is probably not safe because you should not compare nsCOMPtr to nsnull. How about simple = !child; 36. Should there be {}'s somewhere in here, the indentation seems to suggest that but I am not sure... if (minOccurs == 0 && rc == NS_ERROR_NOT_AVAILABLE) // If we failed recoverably, but we were permitted to, then return success *_retElement = aElement; NS_IF_ADDREF(*_retElement); rc = NS_OK;

37. Need out of memory check here: nsCOMPtr<nsISupportsArray> all = new nsSupportsArray(); // Create something we can mutate all->SizeTo(particleCount); 38. Actually regarding point 36 I am pretty sure there are {}'s missing, because this same pattern apperars later WITH {}'s! 39. In general, do not compare nsCOMPtr to nsnull. For example, replace while (child != nsnull) { with while (child) { 40. In DecodeArrayDimensions() the loop past whitespace does not seem complete, perhaps you should use IsSpace() method on the current iterator instead of that strange looking costruction there... What is the deal with "anything < or > than ws"? 41. DecodeArrayPosition()might generate compiler warnings for function maybe not returning values in all execution paths (although it does). Maybe replace the -1 return in the for loop with break, and return -1 at the end of the function? 42. Need out of mem check in CreateArray() for nsIVariant** a = new nsIVariant*[count]; // Create variant array. 43. There are some magic numbers (2147483647) that would be better to define somewhere. 44. Need out of mem check here (several instances of this): nsCOMPtr < nsIWritableVariant > p = do_CreateInstance(NS_VARIANT_CONTRACTID); p->SetAsDouble(f);

45. Unused variable in nsSOAPFault::GetDetail(). 46. Need out of mem checks for nsSOAPEncodingRegistry::GetAssociatedEncoding(), and should also add an Init() method in case new fails in constructor. 47. nsSOAPEncoding class also needs Init() since now it does new in constructor.

48. I noticed you are using sync loading with XMLHttpRequest - I am a bit puzzled how it works (if indeed it does) since I have a bug open to enable sync load from C++ code! See bug 56518. 49. In nsSchemaPrivate, arrange the data members so that all classes can be as small as possible. At least nsSchemaFacet can be made smaller. As a rule of thumb, move all large data members to the beginning and then smaller and smaller etc.

Testing with soapunscramble.html I get this assertion: ###!!! ASSERTION: nsVoidArray::ElementAt(index past end array) - note on bug 961 08: 'aIndex < Count()', file ..\ds\nsVoidArray.h, line 78 ###!!! Break: at file ..\ds\nsVoidArray.h, line 78 The stack is: nsDebug::Assertion(const char * 0x10113598 `string', const char * 0x101135e8 `string', const char * 0x10113654 `string', int 78) line 291 + 13 bytes nsVoidArray::ElementAt(int 2) line 78 + 35 bytes nsGenericContainerElement::GetAttrNameAt(const nsGenericContainerElement * const 0x04de7c80, int 2, int & 1232532, nsIAtom * & 0x00000000, nsIAtom * & 0x00000000) line 3248 + 16 bytes nsDOMAttributeMap::Item(nsDOMAttributeMap * const 0x04de60b0, unsigned int 2, nsIDOMNode * * 0x0012ceb4) line 229 + 93 bytes nsSOAPUtils::MakeNamespacePrefix(nsISOAPEncoding * 0x00000000, nsIDOMElement * 0x04de7ca8, const nsAString & {???}, nsAString & {???}) line 440 + 99 bytes GetTransportURI(nsISOAPCall * 0x04d7c684, nsAString & {???}) line 220 + 33 bytes nsHTTPSOAPTransport::SyncCall(nsHTTPSOAPTransport * const 0x04dd3b50, nsISOAPCall * 0x04d7c684, nsISOAPResponse * 0x04de34b4) line 268 + 16 bytes nsSOAPCall::Invoke(nsSOAPCall * const 0x04d7c684, nsISOAPResponse * * 0x0012d9f0) line 148 + 61 bytes XPTC_InvokeByIndex(nsISupports * 0x04d7c684, unsigned int 22, unsigned int 1, nsXPTCVariant * 0x0012d9f0) line 106 XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode CALL_METHOD) line 1998 + 42 bytes XPC_WN_CallMethod(JSContext * 0x035a7100, JSObject * 0x02d26160, unsigned int 1, long * 0x02d44a64, long * 0x0012dccc) line 1266 + 14 bytes js_Invoke(JSContext * 0x035a7100, unsigned int 1, unsigned int 0) line 832 + 23 bytes js_Interpret(JSContext * 0x035a7100, long * 0x0012eabc) line 2802 + 15 bytes js_Invoke(JSContext * 0x035a7100, unsigned int 1, unsigned int 2) line 849 + 13 bytes js_InternalInvoke(JSContext * 0x035a7100, JSObject * 0x01351628, long 47338368, unsigned int 0, unsigned int 1, long * 0x0012ed14, long * 0x0012ebe4) line 924 + 20 bytes JS_CallFunctionValue(JSContext * 0x035a7100, JSObject * 0x01351628, long 47338368, unsigned int 1, long * 0x0012ed14, long * 0x0012ebe4) line 3415 + 31 bytes nsJSContext::CallEventHandler(nsJSContext * const 0x035a3c20, void * 0x01351628, void * 0x02d25380, unsigned int 1, void * 0x0012ed14, int * 0x0012ed18, int 0) line 1016 + 33 bytes nsJSEventListener::HandleEvent(nsJSEventListener * const 0x04d7c500, nsIDOMEvent * 0x04ddd418) line 180 + 77 bytes nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x04d7c0c0, nsIDOMEvent * 0x04ddd418, nsIDOMEventTarget * 0x035a4d50, unsigned int 1, unsigned int 7) line 1217 + 20 bytes nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x04d7c550, nsIPresContext * 0x04d634f0, nsEvent * 0x0012f43c, nsIDOMEvent * * 0x0012f3e4, nsIDOMEventTarget * 0x035a4d50, unsigned int 7, nsEventStatus * 0x0012f464) line 1890 + 36 bytes GlobalWindowImpl::HandleDOMEvent(GlobalWindowImpl * const 0x035a4d40, nsIPresContext * 0x04d634f0, nsEvent * 0x0012f43c, nsIDOMEvent * * 0x0012f3e4, unsigned int 1, nsEventStatus * 0x0012f464) line 648 DocumentViewerImpl::LoadComplete(DocumentViewerImpl * const 0x04d594d0, unsigned int 0) line 1276 + 47 bytes nsDocShell::EndPageLoad(nsIWebProgress * 0x03d08a24, nsIChannel * 0x04d02ab0, unsigned int 0) line 3745 nsWebShell::EndPageLoad(nsIWebProgress * 0x03d08a24, nsIChannel * 0x04d02ab0, unsigned int 0) line 724 nsDocShell::OnStateChange(nsDocShell * const 0x03d08564, nsIWebProgress * 0x03d08a24, nsIRequest * 0x04d02ab0, int 131088, unsigned int 0) line 3653 nsDocLoaderImpl::FireOnStateChange(nsIWebProgress * 0x03d08a24, nsIRequest * 0x04d02ab0, int 131088, unsigned int 0) line 1110 nsDocLoaderImpl::doStopDocumentLoad(nsIRequest * 0x04d02ab0, unsigned int 0) line 749 nsDocLoaderImpl::DocLoaderIsEmpty() line 647 nsDocLoaderImpl::OnStopRequest(nsDocLoaderImpl * const 0x03d08a14, nsIRequest * 0x04ddddf0, nsISupports * 0x00000000, unsigned int 0) line 578 nsLoadGroup::RemoveRequest(nsLoadGroup * const 0x03d089a0, nsIRequest * 0x04ddddf0, nsISupports * 0x00000000, unsigned int 0) line 526 + 44 bytes PresShell::RemoveDummyLayoutRequest() line 6553 + 42 bytes PresShell::DoneRemovingReflowCommands() line 6509 PresShell::ProcessReflowCommands(int 1) line 6332 ReflowEvent::HandleEvent() line 6116 HandlePLEvent(ReflowEvent * 0x04dd9fc0) line 6130 PL_HandleEvent(PLEvent * 0x04dd9fc0) line 590 + 10 bytes PL_ProcessPendingEvents(PLEventQueue * 0x004b9ee0) line 520 + 9 bytes _md_EventReceiverProc(HWND__ * 0x0011059e, unsigned int 49507, unsigned int 0, long 4955872) line 1071 + 9 bytese).

I have checked in fixes to most of these issues. I have files sitting in my directory to fix the rest (except the strings substring optimization which I will log a bug on). I also reviewed and tested the WSDL separation code. r=rayw. I will check all these in if I get a chance if the tree opens -- trying to talk to Vidur first. That leaves the following items: 1. Mac fixes, which I cannot test. 2. Change default state or remove MOZ_SOAP define. 3. Start actions such as any additions to installer (I don't know if we need something here). I also have not heard back whether jst was able to run purify tests on this. I seriously doubt there could be leaks if people don't call it :-) and I will try to get to this sort of test soon, as well as writing docs, writing more tests, and updating Vidur's WSDL tests so I can run them.

These makefile changes make us build SOAP by default, but it is still possible to disable SOAP by declaring env variable DISABLE_SOAP. I am starting to think SOAP should perhaps be its own extension so it would be easier to configure... we can think about that later.

I am also working on the Mac installer issues. In my opinion we don't need to worry about installers until after this has been baking on the trunk in default builds for about a week. That is bug 107796. The installer changes are trivial, just one line per dll, xpt, etc. to packages files in mozilla/xpinstall/packager

In response to a few of Heikki's comments: Item 4: AString is preferred in IDL over DOMString. Item 7: The point of NS_ERROR_MODULE_GENERAL was to provide a mechanism that avoided having to add more modules to xpcom/base/nsError.h. Since these are extensions, it's perfectly reasonable to have them use the generic error mechanism (taking into account its caveats). Item 9: It's perfectly acceptable to use PRInt32 over long and, according to Jband, is even preferrable.

I'm happy to lend my name to a sr= (or rather a rs=). It seems like Ray's code has been vetted for XPCOM correctness. I've had some exposure to it while working with it. I believe that more exhaustive testing is necessary, but I know that's next on Ray's list. I will attempt to review more closely in the coming week.

r=heikki for the SOAP stuff in general. We'll get individual r & sr for the build patches here....

Simon, could you look at the mac build issues? I think I have made eveything that is needed, but for some reason xmlsoap makefile does not work (xmlwsp is the other one I created and it works). I think it can't find the XPIDL generated headers. You may need to apply 3 patches (1st, 3rd and 4th in the attachment) list to try.

One last set of changes for the sources in the soap directory that fixes the last issues listed here by Heikki, as well as adding more descriptive messages and fixing a few other things. Unfortunately, I have been unable to get a build that compiles today to test the last couple of changes on, which should work, but who knows. There is also one compile problem on Windows (but not on Linux) fixed (I recently introduced) fixed by this patch. This patch should be used in addition / independently of the others.

Comment on attachment 69795 [details] [diff] [review] Fix to last issues of Heikki. (applied) I was able to do more testing with a working tree, and so I checked this latest few issues in. This fixed a platform-specific problem and the last issues mentioned except for the WSP seperation and Mac build problems that are still listed here.

I have added my first stab at documentation for these APIs at: mozilla/extensions/xmlextras/docs/Soap_Scripts_in_Mozilla.html

I just realized that we should add SOAP to installers at the same time as we turn it on in default builds because the code is part of xmlextras which is enabled by default. Without the xpt files you still wouldn't be able to call SOAP from scripts, but it would seem kind of silly... If SOAP is not on by default I do not know what should be done with the packager... maybe look how some other extensions are doing it?

Can peterv help with the mac build stuff?

Ray: Here are the compiler warning I see on Win32. It would be good if you fixed the code to quiet them.

I believe I understood the cause of each warning, and have fixed it properly. In two cases, there were bogus extra castings occurring right beside the correct one. In one case, a PRUint16 had been replaced by a PRBool (which works for the present since we only have two values but would have failed on SOAP 1.3 or some such thing). IN another case, an argument check had been moved to a method that did not return a status (and did not need to check because it was internal). Anyone want to review these and verify that they eliminate the warnings on Win32 (I don't have a Win32 environment and Linux doesn't complain)? Thanks

Comment on attachment 70123 [details] [diff] [review] Fixes the warnings that JBand reported This was inadvertantly checked in, but it is harmless and works and is not part of default build.

Comment on attachment 68637 [details] [diff] [review] Windows and Linux makefile changes (applied) Not part of default build, done inside of MOZ_SOAP defines. Heikki's later patch changed part, but not all to DISABLE_SOAP, but we have no permission yet, and that does touch the border between default and non-default build when we do it.

Comment on attachment 69738 [details] [diff] [review] Windows & Linux makefiles changes, proposal 2 This will have to be redone if we get permission to enable SOAP in default build because we landed the earlier patch by Vidur. It should be easy to rereverse the defines. But this patch appeared to neglect to reverse the defines in the build .cpp file.

Heikki, could you try this one out? Remove the soap or wsp options from your build prefs and build, SOAP should be built but wsp/WSDL should not. Thanks.

Comment on attachment 70529 [details] [diff] [review] Mac build changes sr=sfraser on mac build changes

This, with the Mac enabling patch, are all that are needed to enable SOAP, I believe. I will be testing the patch on a completely-clean tree to check for broken SOAP dependencies before checking it in.

Sorry, the last diff was missing -u. Here it is.

Comment on attachment 70620 [details] [diff] [review] Final soap enabling patch (with the Mac build patch. sr=jband

Comment on attachment 70620 [details] [diff] [review] Final soap enabling patch (with the Mac build patch. a=shaver on behalf of drivers. Welcome to the new era of SOAP!

I'd rather check this one in, it avoids littering the build script with warnings for bad build options.

Comment on attachment 70645 [details] [diff] [review] Slightly better Mac build patch sr=sfraser

Comment on attachment 70645 [details] [diff] [review] Slightly better Mac build patch Most of this patch was checked in. This is the remaining part that we'll check in when turning on SOAP on all platforms. Index: mozilla/build/mac/build_scripts/MozillaBuildFlags.txt =================================================================== RCS file: /cvsroot/mozilla/build/mac/build_scripts/MozillaBuildFlags.txt,v retrieving revision 1.49 diff -u -3 -r1.49 MozillaBuildFlags.txt --- mozilla/build/mac/build_scripts/MozillaBuildFlags.txt 21 Feb 2002 14:15:50 -0000 1.49 +++ mozilla/build/mac/build_scripts/MozillaBuildFlags.txt 21 Feb 2002 14:33:06 -0000 @@ -50,7 +50,7 @@ ldap 1 MOZ_LDAP_XPCOM ldap_experimental 0 MOZ_LDAP_XPCOM_EXPERIMENTAL xmlextras 1 -soap 0 MOZ_SOAP +soap 1 wsp 0 MOZ_WSP inspector 1 mailextras 1

FYI, I see 14 new "xxx might be used uninitialized" warnings on TBox: +extensions/xmlextras/schema/src/nsSchemaLoader.cpp:2672 + `PRUint16 facetType' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2842 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2845 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2847 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2849 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2851 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2853 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2855 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2857 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2859 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2861 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2863 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2865 + `PRInt32 p' might be used uninitialized in this function + +extensions/xmlextras/soap/src/nsDefaultSOAPEncoder.cpp:2873 + `PRInt32 p' might be used uninitialized in this function + See also bug 59652 - the tracking bug for "xxx might be used uninitialized" warnings.

nsDefaultSOAPEncoder.cpp warnings seem to all be coming from the DECODE_ARRAY macro and something is definitely weird in there: PRInt32 p;\ if (nsSOAPUtils::GetAttribute(aEncoding, aSource, nsSOAPUtils::kSOAPEncURI,\ kSOAPArrayPositionAttribute, pos)) {\ PRInt32 p = DecodeArrayPosition(pos, dimensionCount, dimensionSizes);\ if (p == -1) {\ rc = NS_ERROR_ILLEGAL_VALUE;\ break;\ }\ }\ else {\ p = next++;\ }\ Should't the "PRInt32 p = ..." be just "p = ..."??? The nsSchemaLoader.cpp:2672 warning comes from the fact that if aTagName is "none of the above", facetInst->SetFacetType will be called on an uninitialized facetType.

This is finished, after one last unfortunate struggle with the Mac build.

> This is finished, after one last unfortunate struggle with the Mac build. Finished? So, should I file a separate bug for these problems with DECODE_ARRAY in nsDefaultSOAPEncoder.cpp and facetType nsSchemaLoader.cpp, then? They would probably require a one-line fix each, so not sure whether they "deserve" a separate bug.

Do we have data on the effects of enabling SOAP on startup and pageload performance?

Is there any evidence that the xmlextras dll even gets loaded in the normal case? I'm not seeing it get loaded in the windows release build I'm running.

I believe that encoding and decoding of arrays, as well as the schema types tuff certainly deserves a new bug. I like to make sure the work SOAP appears somewhere in the subject. The code for these is complex enough, a one line fix still involves substantial work to make sure it is the correct one line, unless it is a very shallow bug indeed.

OK, filed new bugs for the compiler warnings: bug 127489 for nsDefaultSOAPEncoder.cpp and bug 127490 for nsSchemaLoader.cpp

Changing Q/A contact

verified the patch on the trunk build 2002-06-07-04-trunk. marking verified