33099 - XPCOM Correctness: webclient depends on ns* classes rather than nsI interfaces.

Status: RESOLVED FIXED

(Core Graveyard :: Java APIs to WebShell, defect, P3)

Description

[edburns]

Currently, we link with xpcom.lib, and we use non nsI interfaces. 
        This has to change so we only link with xpcom.dll.

Comment 1

[edburns]

Here's a summary of the non nsI symbols on which webclient depends:

CurrentPageImpl.cpp:        nsCRT::free(charResult);
DocumentLoaderObserverImpl.cpp:			     nsString& aMsg) 
HistoryImpl.cpp:      nsCRT::free(charResult);
NativeEventThread.cpp:    rv = nsServiceManager::GetService
(kEventQueueServiceCID,
RDFEnumeration.cpp:    rv = nsComponentManager::CreateInstance
(NS_IRDFCONTAINER_PROGID,
WrapperFactoryImpl.cpp:static nsFileSpec gBinDir; 


Thankfully there aren't that many.

Comment 2

[edburns]

Attachment: Fix for this bug. Other attachments to follow.

Comment 3

[edburns]

Attachment: New file

Comment 4

[edburns]

Here is some descriptive text for the above attachments:

Native code changes:

This fix eradicates all occurrences of the following symbols

nsComponentManager 
nsServiceManeger

And replaces them with their nsI counterparts.  

The following ns* classes still are used in webclient, and no plans
exist to replace them with nsI counterparts:

nsresult
nsCOMPtr
nsCRT
nsnull 

* nsServiceManager occurrences were replaced with do_GetService(), using a
  PROGID.

* nsComponentManager occurrences were replaced with a call on the global class 
  gComponentManager, declared in the new file ns_globals.h, and defined
  in WrapperFactoryImpl.cpp.  ns_globals.h is included in jni_util.h.
  See the attachment to bug 33099 for ns_globals.h

* Added deallocation code to WindowControlImpl.cpp nativeTerminate.  I
  know it doesn't do much, but it's correct.

Java code changes:

* Added static method BrowserControlFactory.appTerminate().  This method
  simply calls the existing BrowserControlImpl.appTerminate(), which
  calls WrapperFactoryImpl.cpp nativeTerminate().
  BrowserControlFactory.appTerminate() is called from EmbeddedMozilla's
  WindowListener, which gets fired when the user signals she wants the
  app to terminate.

* Moved call to BrowserControlFactory.setAppData() from per-window
  EMWindow contructor to per-app EmbeddedMozilla.  This is more correct.

Comment 5

[edburns]

Changed Summary.

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

- Should purge the rest of the static kInterfaceIID stuff, in favour of
NS_GET_IID.
- do you really want a copy of the PROGIDs in every file that includes
rdf_utils.h?  Why not declare them extern there and define them in one .cpp
file?

Other than that, it looks pretty good. r=shaver.

Comment 7

[edburns]

Thanks Mike,

The real solution on the RDF PROGIDs is fixing 
http://bugzilla.mozilla.org/show_bug.cgi?id=33192

Ed

Comment 8

[edburns]

Shaver reviewed.  Checked in.