Closed Bug 1093934 Opened 10 years ago Closed 9 years ago

Create a XPCOM library that can be used to support standalone WebRTC

Categories

(Core :: WebRTC, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: rbarker, Assigned: rbarker)

References

Details

Attachments

(1 file, 10 obsolete files)

71.61 KB, patch
Details | Diff | Splinter Review
It should be possible to utilize XPCOM without having to pull in all of XUL.
Assignee: nobody → rbarker
Blocks: 1079348
Attached patch libxpcomrt.patch (obsolete) — Splinter Review
First cut a getting a standalone library of XPCOM with minimal dependencies.
This patch has been tested against:

https://github.com/bluemarvin/xpcom-test/tree/16c7074bfb390e97ebe202ad517335426dc812ec

which is a simple application that creates a thread and a timer and dispatches events to the thread. It links against libxcomrt. The final binary size is 700k which should allow us to meet our binary size requirements.
Comment on attachment 8517062 [details] [diff] [review]
libxpcomrt.patch

Here is the first cut at a standalone xpcom library using the approach we discussed. I'm particularly looking for feedback on the thread implementation that removes the MessageLoop and the new init and shutdown functions. Also, if you have a better idea what to call it other than XPCOMRT (XPCOM Runtime), I'm open to suggestions. It was just the first thing I thought of.
Attachment #8517062 - Flags: feedback?(nfroyd)
Comment on attachment 8517062 [details] [diff] [review]
libxpcomrt.patch

Review of attachment 8517062 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/nsDebugImpl.cpp
@@ +405,4 @@
>        return;
>  
>      case NS_DEBUG_ABORT: {
> +#if defined(MOZ_CRASHREPORTER) && !defined(MOZILLA_XPCOMRT_API)

Is it feasible to just disable the crash reporter, MOZ_DMD, DUMP_ASSERTION_STACK etc when doing the XPCOMRT build?
Comment on attachment 8517062 [details] [diff] [review]
libxpcomrt.patch

Review of attachment 8517062 [details] [diff] [review]:
-----------------------------------------------------------------

Ugh, wanting to build this alongside a normal libxul makes things really messy.  I guess this is why :gcp's suggested solution won't work, because we want to build a crashreporter-less xpcomrt alongside a crashreporter-enabled libxul (and Firefox)?

Build peers are going to need to get involved at some point, discussing how we want to package/test/distribute this...What is your plan for those things?

I like this more than the last standalone XPCOM patch, but I'd hardly call it pretty.  Duplicating the XPCOM{,RT}Init.cpp bits is not the greatest, either, but maybe that's OK, given that we don't modify XPCOM initialization often and XPCOMRT probably wants to set things up its own way anyway?

I'm going to flag Benjamin on this, since he was around when XPCOM was more-or-less standalone and has watched it become more entangled, maybe he has some additional high-level (superfeedback?) commentary on this.

::: xpcom/base/nsDebugImpl.cpp
@@ +357,1 @@
>    PrintToBuffer("%d] ", base::GetCurrentProcId());

We should probably just define the moral equivalent of GetCurrentProcId in xpcom, enough places hack around it with getpid().

::: xpcom/build/FileLocation.cpp
@@ +34,5 @@
>      if (aFile.mBaseFile) {
>        Init(aFile.mBaseFile, aFile.mPath.get());
> +    }
> +#if !defined(MOZILLA_XPCOMRT_API)
> +    else {

These additions seem like they're disabling actual functionality.  What's wrong with nsZipArchive.h/nsURLHelper.h?  We don't want xpcomrt linking against zlib?

::: xpcom/components/nsComponentManager.cpp
@@ +74,1 @@
>  #include "nsNetUtil.h"

o.O  This include seems like something we should fix regardless.  File a separate bug, please?

::: xpcom/glue/BlockingResourceBase.cpp
@@ +23,4 @@
>  #include "mozilla/ReentrantMonitor.h"
>  #include "mozilla/Mutex.h"
>  
> +#if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API)

This seems like an incorrect modification.  GeckoProfiler.h should be fixed, perhaps to not include the JS bits when MOZ_ENABLE_PROFILER_SPS is undefined?

I guess the counterargument is that there's value in making --enable-profiling (i.e. Nightly) builds produce the same libxpcomrt bits as non- --enable-profiling builds?

::: xpcom/libxpcomrt/XPCOMRTInit.cpp
@@ +42,5 @@
> +
> +#define COMPONENT(NAME, Ctor) { &kNS_##NAME##_CID, false, nullptr, Ctor },
> +const mozilla::Module::CIDEntry kXPCOMCIDEntries[] = {
> +  { &kComponentManagerCID, true, nullptr, nsComponentManagerImpl::Create },
> +#include "XPCOMRTModule.inc"

I guess you forgot to |$VCS add| this file?  Would probably be good if this was explicitly included from XPCOMModule.inc, too.

@@ +103,5 @@
> +  }
> +
> +  nsRefPtr<nsObserverService> observerService;
> +  CallGetService("@mozilla.org/observer-service;1",
> +                   (nsObserverService**)getter_AddRefs(observerService));

This seems very dodgy.

::: xpcom/libxpcomrt/moz.build
@@ +1,1 @@
> +Library('xpcomrt')

A build peer is going to need to look through all this.  Should we be flipping something on to build a static library always, or do we want a shared library here?

@@ +6,5 @@
> +]
> +
> +xpcom_base_src = [
> +  'nsDebugImpl.cpp',
> +  'nsMemoryImpl.cpp',

I can't help but think a lot of these lists would be better placed into their source directories, and then include'd into the directory's moz.build, and this moz.build.  See xpcom/glue/objs.mozbuild for an example of how this works.

@@ +36,5 @@
> +  'nsSupportsPrimitives.cpp',
> +]
> +if CONFIG['OS_ARCH'] == 'WINNT':
> +  xpcom_ds_src += [
> +    'TimeStamp_windows.cpp',

Having the lists in their own directories means we only have this conditional once, for instance.

::: xpcom/string/nsTSubstring.cpp
@@ +22,1 @@
>      NS_LogCtor(mData, "StringAdopt", 1);

Maybe we should have NS_LOG_CTOR macro wrappers that can be defined in nsISupportsImpl.h for cases like these?

::: xpcom/threads/nsThread.cpp
@@ +25,5 @@
>  #include "pratom.h"
>  #include "prlog.h"
>  #include "nsIObserverService.h"
> +#if defined(MOZILLA_XPCOMRT_API)
> +#include "TimeStamp.h"

?  If we're bootlegging TimeStamp.h from someplace else, we shouldn't be.  Just #include TimeStamp.h regardless?
Attachment #8517062 - Flags: feedback?(nfroyd)
Attachment #8517062 - Flags: feedback?(benjamin)
Attachment #8517062 - Flags: feedback+
(In reply to Nathan Froyd (:froydnj) from comment #5)
> Comment on attachment 8517062 [details] [diff] [review]
> libxpcomrt.patch
> 
> Review of attachment 8517062 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ugh, wanting to build this alongside a normal libxul makes things really
> messy.  I guess this is why :gcp's suggested solution won't work, because we
> want to build a crashreporter-less xpcomrt alongside a crashreporter-enabled
> libxul (and Firefox)?
> 

Yes, it will be built alongside libxul.

> Build peers are going to need to get involved at some point, discussing how
> we want to package/test/distribute this...What is your plan for those things?
> 

Currently the WebRTC unit test uses this combination of xpcom glue and statically linked in libraries that have their symbols hidden in XUL. We would like to convert the unit tests to use this xpcom rt library. This would make the webRTC code cleaner as right now it is riddled with #ifdef MOZILLA_INTERNAL_API as well as help detect breakage since we will have a unit test that depends on the library. Since one of the end goals is to have a WebRTC library that 3rd parties can us, ideal packaging would be a single (or small collection) of libraries that can be used to create a WebRTC enabled application. Someone should be able to checkout and build gecko-dev and use the build artifacts directly out of the obj directory.

> I like this more than the last standalone XPCOM patch, but I'd hardly call
> it pretty.  Duplicating the XPCOM{,RT}Init.cpp bits is not the greatest,
> either, but maybe that's OK, given that we don't modify XPCOM initialization
> often and XPCOMRT probably wants to set things up its own way anyway?
>

I thought about #ifdef'ing XPCOMInit.cpp and just using the default init and shutdown functions, but it seemed that #ifdef'ing that file would have created a undesirable mess. I didn't expect the resultant rt init file to grow as large as it did however. 

> I'm going to flag Benjamin on this, since he was around when XPCOM was
> more-or-less standalone and has watched it become more entangled, maybe he
> has some additional high-level (superfeedback?) commentary on this.
> 
> ::: xpcom/base/nsDebugImpl.cpp
> @@ +357,1 @@
> >    PrintToBuffer("%d] ", base::GetCurrentProcId());
> 
> We should probably just define the moral equivalent of GetCurrentProcId in
> xpcom, enough places hack around it with getpid().
> 

I thought about it but decided to just ifdef it out since it was the only occurrence of it and I was more interested in getting things working. Easy to fix, probably as a separate patch? 

> ::: xpcom/build/FileLocation.cpp
> @@ +34,5 @@
> >      if (aFile.mBaseFile) {
> >        Init(aFile.mBaseFile, aFile.mPath.get());
> > +    }
> > +#if !defined(MOZILLA_XPCOMRT_API)
> > +    else {
> 
> These additions seem like they're disabling actual functionality.  What's
> wrong with nsZipArchive.h/nsURLHelper.h?  We don't want xpcomrt linking
> against zlib?
> 

The problem wasn't zlib, it was that nsZipArchive lives in /modules/libjar and nsURLHelper lives in /netwerk/base/src/. From the previous attempt, I knew we didn't even need FileLocation, however there is an idl file that depends on it so I couldn't just cut it out completely. So I cut out the external dependencies. I guess I could stump it out but this seemed more obvious that the functionality is not included.

> ::: xpcom/components/nsComponentManager.cpp
> @@ +74,1 @@
> >  #include "nsNetUtil.h"
> 
> o.O  This include seems like something we should fix regardless.  File a
> separate bug, please?
> 

This is included for NS_NewURI. I'm guessing if nsNetUtil.h should not be include, neither should nsURLHelper.h since they both live in /netwerk?

> ::: xpcom/glue/BlockingResourceBase.cpp
> @@ +23,4 @@
> >  #include "mozilla/ReentrantMonitor.h"
> >  #include "mozilla/Mutex.h"
> >  
> > +#if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API)
> 
> This seems like an incorrect modification.  GeckoProfiler.h should be fixed,
> perhaps to not include the JS bits when MOZ_ENABLE_PROFILER_SPS is undefined?
> 
> I guess the counterargument is that there's value in making
> --enable-profiling (i.e. Nightly) builds produce the same libxpcomrt bits as
> non- --enable-profiling builds?
> 

Including this would require pulling in /tools/profiler/. 

> ::: xpcom/libxpcomrt/XPCOMRTInit.cpp
> @@ +42,5 @@
> > +
> > +#define COMPONENT(NAME, Ctor) { &kNS_##NAME##_CID, false, nullptr, Ctor },
> > +const mozilla::Module::CIDEntry kXPCOMCIDEntries[] = {
> > +  { &kComponentManagerCID, true, nullptr, nsComponentManagerImpl::Create },
> > +#include "XPCOMRTModule.inc"
> 
> I guess you forgot to |$VCS add| this file?  Would probably be good if this
> was explicitly included from XPCOMModule.inc, too.
> 

Sorry, looks like .gitignore has a rule for .inc so I didn't see it needed to be added.

> @@ +103,5 @@
> > +  }
> > +
> > +  nsRefPtr<nsObserverService> observerService;
> > +  CallGetService("@mozilla.org/observer-service;1",
> > +                   (nsObserverService**)getter_AddRefs(observerService));
> 
> This seems very dodgy.
> 

I copied it from here: http://mxr.mozilla.org/mozilla-central/source/xpcom/build/XPCOMInit.cpp#821

> ::: xpcom/libxpcomrt/moz.build
> @@ +1,1 @@
> > +Library('xpcomrt')
> 
> A build peer is going to need to look through all this.  Should we be
> flipping something on to build a static library always, or do we want a
> shared library here?
> 

We need it to be static although currently a libxpcomrt.a.desc is what is produced. Others may want a shared lib. I don't know what the correct answer is for this.

> @@ +6,5 @@
> > +]
> > +
> > +xpcom_base_src = [
> > +  'nsDebugImpl.cpp',
> > +  'nsMemoryImpl.cpp',
> 
> I can't help but think a lot of these lists would be better placed into
> their source directories, and then include'd into the directory's moz.build,
> and this moz.build.  See xpcom/glue/objs.mozbuild for an example of how this
> works.
> 

I understand the need to reduce duplication. My thinking was that it is easier to see what actually is going into the library than if the files being pulled in were scattered through the various sub dirs in xpcom. Also I ran into a problem early on with SOURCES vs UNIFIED_SOURCES and rather than wasting time unraveling it, just putting every thing into SOURCES. I guess I could still put everything into SOURCES regardless of how it is done for the regular library.

> @@ +36,5 @@
> > +  'nsSupportsPrimitives.cpp',
> > +]
> > +if CONFIG['OS_ARCH'] == 'WINNT':
> > +  xpcom_ds_src += [
> > +    'TimeStamp_windows.cpp',
> 
> Having the lists in their own directories means we only have this
> conditional once, for instance.
> 
> ::: xpcom/string/nsTSubstring.cpp
> @@ +22,1 @@
> >      NS_LogCtor(mData, "StringAdopt", 1);
> 
> Maybe we should have NS_LOG_CTOR macro wrappers that can be defined in
> nsISupportsImpl.h for cases like these?
> 

Sure, I'll add it.

> ::: xpcom/threads/nsThread.cpp
> @@ +25,5 @@
> >  #include "pratom.h"
> >  #include "prlog.h"
> >  #include "nsIObserverService.h"
> > +#if defined(MOZILLA_XPCOMRT_API)
> > +#include "TimeStamp.h"
> 
> ?  If we're bootlegging TimeStamp.h from someplace else, we shouldn't be. 
> Just #include TimeStamp.h regardless?

Yes, I'll fix that.
Comment on attachment 8517062 [details] [diff] [review]
libxpcomrt.patch

I really can't provide useful feedback. Standalone XPCOM was a maintenance nightmare the first time. The bootstrap sequence inherently couples a lot of things that maybe shouldn't be coupled.

But somewhere you need to document what's "in" and what's "out" of the standalone version, if we're going to have a standalone version.
Attachment #8517062 - Flags: feedback?(benjamin)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> Comment on attachment 8517062 [details] [diff] [review]
> libxpcomrt.patch
> 
> I really can't provide useful feedback. Standalone XPCOM was a maintenance
> nightmare the first time. The bootstrap sequence inherently couples a lot of
> things that maybe shouldn't be coupled.
> 
> But somewhere you need to document what's "in" and what's "out" of the
> standalone version, if we're going to have a standalone version.

Do you think making the standalone xpcom an internal component of the standalone WebRTC library that we want to create would help with the maintenance nightmare? Since the end goal is to create a standalone WebRTC library, not to create a standalone xpcom library, it doesn't matter to me if we don't package this up so that it is usable for external consumption.
Blocks: 1097804
(In reply to Randall Barker [:rbarker] from comment #8)
> Do you think making the standalone xpcom an internal component of the
> standalone WebRTC library that we want to create would help with the
> maintenance nightmare?

What do you mean exactly, here?
Flags: needinfo?(benjamin)
Flags: needinfo?(benjamin)
(In reply to Nathan Froyd (:froydnj) from comment #5)

> Ugh, wanting to build this alongside a normal libxul makes things really
> messy.  I guess this is why :gcp's suggested solution won't work, because we
> want to build a crashreporter-less xpcomrt alongside a crashreporter-enabled
> libxul (and Firefox)?

I'm not sure what you mean by "solution won't work" because I implemented this already :-)

You can simply undefine those defines during the libxpcomrt build. Doing it centrally reduces the size of this patch and makes things cleaner.

As for building alongside (i.e. building the relevant files twice), this has the advantages of not having to untangle everything at the cost of having to sprinkle those #idefs, of course. Given that normal builds have currently no want of using an outside-libxul libxpcom you need to build them twice anyway.
I'm interested in this from the standpoint of having a separate lib for using Necko without having to load and init the rest of Gecko. I mention that to plant a seed in gcp's and rbarker's heads :D
Blocks: 1101651
Attached patch XPCOM Standalone v3 (obsolete) — Splinter Review
Rebased to latest code base.
Attachment #8517062 - Attachment is obsolete: true
Attached patch XPCOM Standalone v4 (obsolete) — Splinter Review
Attachment #8544899 - Attachment is obsolete: true
Attached patch XPCOM Standalone v5 (obsolete) — Splinter Review
Attachment #8545449 - Attachment is obsolete: true
Comment on attachment 8545531 [details] [diff] [review]
XPCOM Standalone v5

I believe this is finally ready for review.
Attachment #8545531 - Flags: review?(nfroyd)
Attached patch XPCOM Standalone v6 (obsolete) — Splinter Review
Attachment #8545531 - Attachment is obsolete: true
Attachment #8545531 - Flags: review?(nfroyd)
Comment on attachment 8545565 [details] [diff] [review]
XPCOM Standalone v6

Moved formatting change from standalone necko to this patch.
Attachment #8545565 - Flags: review?(nfroyd)
Attached patch XPCOM Standalone v7 (obsolete) — Splinter Review
Attachment #8545565 - Attachment is obsolete: true
Attachment #8545565 - Flags: review?(nfroyd)
Comment on attachment 8545634 [details] [diff] [review]
XPCOM Standalone v7

Discovered adding Telemetry stubs broke optimized build.
Attachment #8545634 - Flags: review?(nfroyd)
Comment on attachment 8545634 [details] [diff] [review]
XPCOM Standalone v7

Review of attachment 8545634 [details] [diff] [review]:
-----------------------------------------------------------------

The moz.build parts need review from a build peer; lobbying the review for that to mshal.  I think the xpcom bits are OK.

Benjamin's point about documenting what goes where from comment 7 is well-taken.  We can do this with in-tree Sphinx docs.  Please ask if you need pointers.

f+ only because I'd like to at least glance at the docs.

::: xpcom/build/Services.cpp
@@ +12,5 @@
>  #ifdef ACCESSIBILITY
>  #include "nsIAccessibilityService.h"
>  #endif
>  #include "nsIChromeRegistry.h"
> +#endif // !defined(MOZILLA_XPCOMRT_API)

Please rearrange headers in this block and the next to consolidate into a single #if !defined block.

::: xpcom/components/nsComponentManager.cpp
@@ +62,2 @@
>  #include "ManifestParser.h"
> +#endif // !defined(MOZILLA_XPCOMRT_API)

Please rearrange headers in this block and the next to consolidate into a single #if !defined block.

::: xpcom/libxpcomrt/XPCOMRTInit.cpp
@@ +1,1 @@
> +#include "mozilla/Module.h"

This file needs a license header and editor modelines.

::: xpcom/libxpcomrt/XREStub.cpp
@@ +1,1 @@
> +#include "nsXULAppAPI.h"

This needs a license header and editor modelines.

::: xpcom/string/nsSubstring.cpp
@@ +118,3 @@
>      // Treat this as destruction of a "StringAdopt" object for leak
>      // tracking purposes.
> +    MOZ_LOG_DTOR(aData, "StringAdopt", 1);

Shouldn't this macro be defined appropriately for !NS_BUILD_REFCNT_LOGGING, in which case we could remove the #if?

::: xpcom/threads/nsThread.cpp
@@ +32,5 @@
>  #include "mozilla/Services.h"
>  #include "nsXPCOMPrivate.h"
>  #include "mozilla/ChaosMode.h"
> +#include "mozilla/TimeStamp.h"
> +#if !defined(MOZILLA_XPCOMRT_API)

Please bundle up the MOZILLA_XPCOMRT_API header blocks.  base/message_loop.h can stay where it is up above, given problems with the LOG macro, though.
Attachment #8545634 - Flags: review?(nfroyd)
Attachment #8545634 - Flags: review?(mshal)
Attachment #8545634 - Flags: feedback+
Comment on attachment 8545634 [details] [diff] [review]
XPCOM Standalone v7

>diff --git a/xpcom/libxpcomrt/moz.build b/xpcom/libxpcomrt/moz.build
>new file mode 100644
>--- /dev/null
>+++ b/xpcom/libxpcomrt/moz.build
>@@ -0,0 +1,143 @@

You'll want to add the standard moz.build comment header here.

>+Library('xpcomrt')
>+
>+src_list = [
>+  'XPCOMRTInit.cpp',
>+  'XREStub.cpp',
>+]

Please use 4-space indents for moz.build files.

>+xpcom_base_src = [
>+  'nsDebugImpl.cpp',
>+  'nsMemoryImpl.cpp',
>+  'nsUUIDGenerator.cpp',
>+]
>+src_list += [
>+  '%s/xpcom/base/%s' % (TOPSRCDIR, s) for s in xpcom_base_src
>+]

These constructions are ugly, but unfortunately I don't have a better suggestion until we get better path handling in moz.build files (like bug 964302 and/or bug 1065434).

Is any of this data shared with other moz.build files? If so then it may make sense to pull them out to included files as froydnj suggested. Otherwise I don't see a problem with having it all here.
Attachment #8545634 - Flags: review?(mshal) → feedback+
No longer blocks: 1079348
Blocks: 1079348
Attached patch XPCOM Standalone v8 (obsolete) — Splinter Review
Added first cut at doc.
Attachment #8545634 - Attachment is obsolete: true
Comment on attachment 8552128 [details] [diff] [review]
XPCOM Standalone v8

I've added my attempt to document the xpcomrt library. I'm still working to get this patch to build on try for all platforms but I figured you could look at the doc I created and give feedback on what you feel needs more information (or less?). I guess I should have made the doc a separate patch to make it easy to review. Search for index.rst to find it.
Attachment #8552128 - Flags: feedback?(nfroyd)
Attached patch XPCOM Standalone v9 (obsolete) — Splinter Review
Attachment #8552128 - Attachment is obsolete: true
Attachment #8552128 - Flags: feedback?(nfroyd)
Comment on attachment 8552717 [details] [diff] [review]
XPCOM Standalone v9

Now able to pass try. Patch should be ready for review.
Attachment #8552717 - Flags: review?(nfroyd)
Comment on attachment 8552717 [details] [diff] [review]
XPCOM Standalone v9

Review of attachment 8552717 [details] [diff] [review]:
-----------------------------------------------------------------

Minor wordsmithing on the newly added docs.  I didn't look at the other changes; I'm assuming you made whatever fixups were necessary from previous reviews.

::: xpcom/libxpcomrt/docs/index.rst
@@ +3,5 @@
> +==========================
> +
> +What it is for
> +--------------
> +The XPCOM standalone library called libxpcomrt was created to support building the WebRTC

Maybe "The XPCOM standalone library, libxpcomrt, ..."

@@ +5,5 @@
> +What it is for
> +--------------
> +The XPCOM standalone library called libxpcomrt was created to support building the WebRTC
> +standalone library. The libxpcomrt library contains only the parts of XPCOM that are required
> +to run WebRTC without the need to include XUL. A library containing a small subset of Necko was also

"...without the need to include XUL." sounds imprecise.  Perhaps "...to run WebRTC; parts such as the cycle collector and the startup cache required only by Gecko are not included."

@@ +12,5 @@
> +The libxcomrt library was created specifically to support the WebRTC standalone library.
> +It is not intended to be used as a general purpose library to add XPCOM functionality to
> +an application. It is likely that some of the code contained in the libxpcomrt library
> +has unresolved symbols that may be exposed if used for purposes other than linking in
> +the WebRTC standalone library.

I think "...other than being linked into the WebRTC standalone library" is more precise here.

@@ +16,5 @@
> +the WebRTC standalone library.
> +
> +How to use it
> +-------------
> +When compiling code utilizing libxpcomrt, both MOZILLA_INTERNAL_API and MOZILLA_XPCOMRT_API must be defined in

``MOZILLA_INTERNAL_API`` and ``MOZILLA_XPCOMRT_API`` to make things appropriately monospaced, please.

@@ +17,5 @@
> +
> +How to use it
> +-------------
> +When compiling code utilizing libxpcomrt, both MOZILLA_INTERNAL_API and MOZILLA_XPCOMRT_API must be defined in
> +addition to whatever standard flags were defined at Gecko's compile time.

Maybe "...whatever standard flags are used to compile Gecko"?

@@ +18,5 @@
> +How to use it
> +-------------
> +When compiling code utilizing libxpcomrt, both MOZILLA_INTERNAL_API and MOZILLA_XPCOMRT_API must be defined in
> +addition to whatever standard flags were defined at Gecko's compile time.
> +The library is initialized with NS_InitXPCOMRT() and shutdown with  NS_ShutdownXPCOMRT(). Both functions are

``NS_InitXPCOMRT()`` and ``NS_ShutdownXPCOMRT()``, please.

@@ +19,5 @@
> +-------------
> +When compiling code utilizing libxpcomrt, both MOZILLA_INTERNAL_API and MOZILLA_XPCOMRT_API must be defined in
> +addition to whatever standard flags were defined at Gecko's compile time.
> +The library is initialized with NS_InitXPCOMRT() and shutdown with  NS_ShutdownXPCOMRT(). Both functions are
> +defined in xpcom/libxpcomrt/XPCOMRTInit.h. Only a small number of services which are required for the WebRTC

"Both functions are declared in..."

@@ +22,5 @@
> +The library is initialized with NS_InitXPCOMRT() and shutdown with  NS_ShutdownXPCOMRT(). Both functions are
> +defined in xpcom/libxpcomrt/XPCOMRTInit.h. Only a small number of services which are required for the WebRTC
> +standalone library to function are included with libxpcomrt. The dynamic loading of services is not
> +supported. Including a service through NSMODULE_DEFN and static linking is also not supported. The only way to add
> +a service to libxpcomrt is explicitly inside of nsComponentManagerImpl::Init in xpcom/components/nsComponentManager.cpp.

"...to libxpcomrt is to explicitly start the service during nsComponentManagerImpl::Init..."  Should probably be ``nsComponentManagerImpl::Init`` so it shows up in a monospaced font.

@@ +25,5 @@
> +supported. Including a service through NSMODULE_DEFN and static linking is also not supported. The only way to add
> +a service to libxpcomrt is explicitly inside of nsComponentManagerImpl::Init in xpcom/components/nsComponentManager.cpp.
> +The best method to determine what parts of XPCOM are included in libxpcomrt is to examine the
> +xpcom/libxpcomrt/moz.build file. It contains all of the XPCOM source files used to build libxpcomrt.
> +A few of the services that are include are:

"...that are included..."

@@ +32,5 @@
> +* DNS Service
> +* Socket Transport Service
> +* IDN Service
> +
> +All dependencies on ipc/chromium have been removed. IO and preference services are not include making this library of limited utility.

"...are not included..."
Attachment #8552717 - Flags: review?(nfroyd) → review+
Rebased to current inbound.
Carry forward r+ from nfroyd
Attachment #8552717 - Attachment is obsolete: true
Rebased to current inbound.
Carry forward r+ from nfroyd
Attachment #8558854 - Attachment is obsolete: true
Keywords: checkin-needed
Rebased to current inbound.
Carry forward r+ from nfroyd
Flags: needinfo?(rbarker)
Attachment #8587522 - Attachment is obsolete: true
Keywords: checkin-needed
Depends on: 1151829
https://hg.mozilla.org/mozilla-central/rev/9e105dd45820
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla40
See Also: → 1237409
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: