288626 - Switch from MOZILLA_STRICT_API to MOZILLA_INTERNAL_API

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect, P1)

Description

[Benjamin Smedberg]

To improve our embedding story and make it easier for embedders to use moz, we
are going to make the "strict API" the default, instead of requiring embedders
to define MOZILLA_STRICT_API. Gecko-internal code will use MOZILLA_INTERNAL_API
to flag that they want to use the internal/nonfrozen imports instead.

First, I'm going to go through the tree and add MOZILLA_INTERNAL_API = 1 to
pretty much all of our gecko makefiles (which will have no effect yet). This
excludes gtkmozembed (which uses the strict API IIUI) and the samples and
anything else which uses the glue. Then the final patch will switch around the
#define logic and add the necessary build system logic.

Camino will need to update its mac-specific build-system (xcode?) to define
MOZILLA_INTERNAL_API, since it's only a future dream that it should only use the
frozen imports. This can be done at any time.

Comment 1

[Benjamin Smedberg]

Note to self: I can make LIBXUL_LIBRARY trigger MOZILLA_INTERNAL_API, so I only
need to add it explicitly to non-libxul directories.

Comment 2

[Benjamin Smedberg]

Attachment: Switch to MOZILLA_INTERNAL_API

I'm still building various combinations, but I think this does it.

Comment 3

[Darin Fisher]

Comment on attachment 179736 [details] [diff] [review]
Switch to MOZILLA_INTERNAL_API

>Index: xpcom/base/nsIWeakReference.idl

> %{C++
>-#ifndef MOZILLA_STRICT_API
> #include "nsIWeakReferenceUtils.h" 
>-#endif
> %}

With the strict-api, IDL files are not supposed to #include other header
files.	So, this should be changed to an #ifdef MOZILLA_INTERNAL_API.


>Index: xpcom/components/nsIComponentManager.idl

> %{ C++
>-#ifndef MOZILLA_STRICT_API
> #include "nsComponentManagerUtils.h"
>-#endif
> %} C++

Same goes for this one.  These #ifdefs only exist because someone
was too lazy to go and fix all of the source files that depend on
this header file being included.


>Index: xpcom/components/nsIServiceManager.idl

> // Observing xpcom shutdown
> #define NS_XPCOM_SHUTDOWN_OBSERVER_ID "xpcom-shutdown"
> 
> // Observing xpcom autoregistration.  Topics will be 'start' and 'stop'.
> #define NS_XPCOM_AUTOREGISTRATION_OBSERVER_ID "xpcom-autoregistration"
> 
>-#ifndef MOZILLA_STRICT_API
> #include "nsXPCOM.h"
> #include "nsServiceManagerUtils.h"
>+
>+#ifdef MOZILLA_INTERNAL_API
> #include "nsIServiceManagerObsolete.h"
> #endif
> %}

We really need to document all of these contracts in some header file
like nsXPCOMCID.h or some other header file instead of in the IDL file.


>Index: xpcom/io/nsDirectoryServiceUtils.h

> inline nsresult
> NS_GetSpecialDirectory(const char* specialDirName, nsIFile* *result)
> {
>     nsresult rv;
>+    nsCOMPtr<nsIProperties> serv(do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv));
>     if (NS_FAILED(rv))
>         return rv;
> 
>     return serv->Get(specialDirName, NS_GET_IID(nsIFile),
>                      NS_REINTERPRET_CAST(void**, result));
> }

We should include this header file in the SDK.


>Index: xpcom/io/nsIFile.idl

> %{C++
>-#ifndef MOZILLA_STRICT_API
> #include "nsDirectoryServiceUtils.h" 
>-#endif
> %}

Ditto.


>Index: modules/libpref/src/nsPrefService.cpp

>+  static NS_DEFINE_CID(kDirectoryServiceCID, NS_DIRECTORY_SERVICE_CONTRACTID);

This looks wacky.  Does it compile?

Comment 4

[Benjamin Smedberg]

Attachment: Switch to MOZILLA_INTERNAL_API, rev. 1.1

OK... I don't really see why the %{C++ #includes are such a bad thing, but it
doesn't really matter to me. Let's save moving the IDL docs for another bug.

Comment 5

[Benjamin Smedberg]

Attachment: Addition patch for profile/dirserviceprovider

Comment 6

[Darin Fisher]

Comment on attachment 179741 [details] [diff] [review]
Switch to MOZILLA_INTERNAL_API, rev. 1.1

r=darin

This is a bit of a stretch, but...

The idea is to keep the IDL files clean, so that a consumer who does not want
to import all of those header files and hence various inline functions can
do so.	Some people do not use the XPCOM glue when writing components b/c
they do not want to "taint" their DLLs with code from Mozilla CVS.  They may
not want to be responsible for the authenticity of that code.

If we want to make our APIs easier to use by having fewer header files for
people to include, then we should create additional header files for that
purpose.  E.g., it's nice being able to #include "nsNetUtil.h" to get most
of the necko interfaces.  The Gecko SDK could benefit from headers like that.

The other thing to consider is that sometimes people add %{C++ sections to
IDL files that add stuff that only makes sense in the context of a particular
implementation of that interface.  For example, I could implement a version
of nsIServiceManager that exposed services available in another process.  Do
the xpcom-autoregistration notification topics make sense in that context?
No, they only make sense in the context of the @mozilla.org/service-manager;1
implementation of nsIServiceManager.

Comment 7

[Darin Fisher]

> No, they only make sense in the context of the @mozilla.org/service-manager;1
> implementation of nsIServiceManager.

Sorry, I made up that contractid.  What I meant was to indicate the built-in
service manager that we all know and uhm love.

Comment 8

[Doug Halamay]

This might be little more than a stab in the dark, but here goes anyway.  I do
static MSVC2003.NET builds, and they've been working fine up until 4/4/05.  I
tried doing a build on Tuesday (4/5/05), and I keep getting this error (see
below) at the very last stage of the build, as the .obj files are linked.  The
only thing that I could see that had anything to do with this was the checkins
related to xpcom and this bug on 4/5/05.  Could this error be caused by
something to do with this change from STRICT_API to INTERNAL_API?  Any help
would be appreciated.  I'm doing a build right now with the changes backed out
to see if that fixes the problem.  I will advise further as soon as it finishes
(or not).  Thanks in advance.

LINK : warning LNK4044: unrecognized option '/L../../dist/lib/components'; ignored
mork.lib(morkObject.obj) : error LNK2005: "public: __thiscall
nsQueryInterface::nsQueryInterface(class nsISupports *)"
(??0nsQueryInterface@@QAE@PAVnsISupports@@@Z) already defined in
xpcom_core.lib(xpcom_core.dll)
mork.lib(morkObject.obj) : warning LNK4006: "public: __thiscall
nsQueryInterface::nsQueryInterface(class nsISupports *)"
(??0nsQueryInterface@@QAE@PAVnsISupports@@@Z) already defined in
xpcom_core.lib(xpcom_core.dll); second definition ignored
   Creating library firefox.lib and object firefox.exp
firefox.exe : fatal error LNK1169: one or more multiply defined symbols found
make[4]: *** [firefox.exe] Error 145
make[4]: Leaving directory `/cygdrive/i/mozilla/objdir/browser/app'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/i/mozilla/objdir/browser'
make[2]: *** [tier_99] Error 2
make[2]: Leaving directory `/cygdrive/i/mozilla/objdir'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/cygdrive/i/mozilla/objdir'
make: *** [build] Error 2

Comment 9

[Doug Halamay]

With regard to my previous comment (comment #8), I can now confirm that
performing the following backout remedied the situation (the build worked just
fine).  I'm unable to say exactly which file is the problem here, though.  Any
ideas?

cvs update -j1.17 -j1.16 mozilla/xpcom/string/public/nsTAString.h
cvs update -j1.14 -j1.13 mozilla/xpcom/string/public/nsStringAPI.h
cvs update -j1.38 -j1.37 mozilla/xpcom/tools/registry/Makefile.in
cvs update -j1.10 -j1.9 mozilla/xpcom/tests/TestMinStringAPI.cpp
cvs update -j1.23 -j1.22 mozilla/xpcom/build/nsXPCOMPrivate.h
cvs update -j1.16 -j1.15 mozilla/xpcom/build/nsXPCOM.h
cvs update -j3.5 -j3.4 mozilla/xpcom/components/nsIServiceManager.idl
cvs update -j3.21 -j3.20 mozilla/xpcom/components/nsIComponentManager.idl
cvs update -j1.8 -j1.7 mozilla/xpfe/bootstrap/nsStringSupport.h
cvs update -j1.436 -j1.435 mozilla/xpfe/bootstrap/nsAppRunner.cpp
cvs update -j1.277 -j1.276 mozilla/xpfe/bootstrap/Makefile.in
cvs update -j1.20 -j1.19 mozilla/xpcom/glue/standalone/Makefile.in
cvs update -j1.37 -j1.36 mozilla/xpcom/sample/Makefile.in
cvs update -j3.8 -j3.7 mozilla/xpcom/glue/nsServiceManagerUtils.h
cvs update -j1.11 -j1.10 mozilla/xpcom/glue/nsComponentManagerUtils.cpp
cvs update -j1.21 -j1.20 mozilla/xpcom/glue/Makefile.in
cvs update -j1.62 -j1.61 mozilla/xpcom/io/nsIFile.idl
cvs update -j3.4 -j3.3 mozilla/xpcom/io/nsDirectoryServiceUtils.h
cvs update -j1.30 -j1.29 mozilla/xpcom/io/nsDirectoryService.h
cvs update -j1.80 -j1.79 mozilla/xpcom/io/Makefile.in
cvs update -j1.2 -j1.1 mozilla/profile/dirserviceprovider/standalone/Makefile.in
cvs update -j1.6 -j1.5
mozilla/profile/dirserviceprovider/public/nsProfileDirServiceProvider.h
cvs update -j1.4 -j1.3 mozilla/profile/dirserviceprovider/src/nsProfileStringTypes.h
cvs update -j1.7 -j1.6 mozilla/profile/dirserviceprovider/src/Makefile.in
cvs update -j1.76 -j1.75 mozilla/xpcom/base/nscore.h
cvs update -j1.19 -j1.18 mozilla/xpcom/base/nsIWeakReference.idl
cvs update -j1.89 -j1.88 mozilla/modules/libpref/src/nsPrefService.cpp
cvs update -j3.310 -j3.309 mozilla/config/config.mk

Comment 10

[Benjamin Smedberg]

Attachment: Fix msvc.net2003 static builds [checked in]

This is a trivial build change to fix static builds on msvc.net2003, really low
risk.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 179886 [details] [diff] [review]
Fix msvc.net2003 static builds [checked in]

Since things depending on mork are LIBXUL_LIBRARY (e.g.,
toolkit/components/history/), shouldn't this be LIBXUL_LIBRARY=1 instead?

Comment 12

[Asa Dotzler [:asa]]

Comment on attachment 179886 [details] [diff] [review]
Fix msvc.net2003 static builds [checked in]

a=asa for 1.8b2 checkin.

Comment 13

[Doug Halamay]

(In reply to comment #10)
> Created an attachment (id=179886) [edit]
> Fix msvc.net2003 static builds
> 
> This is a trivial build change to fix static builds on msvc.net2003, really low
> risk.

I can confirm that this fix solves the problem I reported in comment 8.  The
build works just fine in MSVC2003.NET.

Comment 14

[Benjamin Smedberg]

Comment on attachment 179886 [details] [diff] [review]
Fix msvc.net2003 static builds [checked in]

LIBXUL_LIBRARY is correct, checked in

Comment 15

[Benjamin Smedberg]

I'm pretty sure this is all fixed now.

Comment 16

[Mitch]

Attachment: Build Failure from MOZILLA_INTERNAL_API change

Looks like the calendering tree hasn't been fixed to use the new API. See
attachment for build failure (on Linux) of latest cvs source.