Proposal: stop exporting JS symbols

[Benjamin Smedberg]

Currently, extensions are able to use the JSAPI via its exported
symbols. This pretty much invariably leads to stability or security
issues with extensions that actually do this:

* the JSAPI is complicated, and even within Mozilla we get reviews from
a select set of people on JSAPI code
* the JSAPI is moving quickly for such features as exact rooting as well
as things like compartments and the interaction with xpconnect.
Extensions aren't keeping up.
* the JSAPI is not a binary-stable interface but some people are trying
to use it that way

So I would like to propose that we link the JS libraries statically into
libxul and stop exporting JSAPI symbols entirely. This will effectively
prevent extensions from using it.

There are some technical issues to work out, such as what we do with
xpcshell. I think what I'd like to do is bake xpcshell directly into
libxul. As a side benefit this will aid a long-term goal of mine to
unify the startup paths across app/embedding/xpcshell launch.

Thoughts or objections?

--BDS

[Mike Hommey]

We're already statically linking js libraries info libxul. Except on
windows, but that's work in progress in bug 915735, although we don't
know yet if it's going to work at all: after dealing with the xpcshell
crash during the instrumentation phase, we'll have to see how much
memory the linker is going to use, and that might not be very good news
ahead.

Not exporting jsapi should be trivial, once we're there.

For xpcshell, I think there's a bug around to essentially fold it into
libxul, except for a thin layer. Ted might have been the one to file it
a long time ago.

With that being said, relatedly, while I'm hopeful we could kill the
--enable-shared-js option (for libxul) at some point, Boris raised an
interesting point in that it is useful to him with Instruments, so that
it can flatten everything from libmozjs in a profile. I hope we'll find
an alternative for him, because I wouldn't like to make things painful
for Boris. OTOH, supporting --enable-shared-js is already causing
headaches, not exporting symbols might add to the complexity.

Mike

[Benjamin Smedberg]

On 9/20/2013 9:23 AM, Mike Hommey wrote:
> We're already statically linking js libraries info libxul. Except on
> windows, but that's work in progress in bug 915735

I am primarily worried about doing this on Windows.

> , although we don't
> know yet if it's going to work at all: after dealing with the xpcshell
> crash during the instrumentation phase, we'll have to see how much
> memory the linker is going to use, and that might not be very good news
> ahead.

We will probably have a complete solution for linker memory issues when
VC2013 is released. Let's plan as if that were the case.

>
> For xpcshell, I think there's a bug around to essentially fold it into
> libxul, except for a thin layer. Ted might have been the one to file it
> a long time ago.

Does anyone own that currently? If not, I'm looking for volunteers!

>
> With that being said, relatedly, while I'm hopeful we could kill the
> --enable-shared-js option (for libxul) at some point, Boris raised an
> interesting point in that it is useful to him with Instruments, so that
> it can flatten everything from libmozjs in a profile. I hope we'll find
> an alternative for him, because I wouldn't like to make things painful
> for Boris. OTOH, supporting --enable-shared-js is already causing
> headaches, not exporting symbols might add to the complexity.

Hrm. Can profilers (Instruments in particular) profile by C++
namespace? I don't particularly mind have that kind of build option for
self-build profiling, as long as it's not on in release builds, but I
imagine that with the new ICU combinations it could get messy quickly.
Boris how strong is your opinion/need for this?

--BDS

[Boris Zbarsky]

On 9/20/13 10:02 AM, Benjamin Smedberg wrote:
> Hrm. Can profilers (Instruments in particular) profile by C++
> namespace?

I don't see a way to do it (keep in mind that it's probably primarily
targeting ObjC, not C++).

It can do things per-symbol and per-library, but those are theoptions.

> Boris how strong is your opinion/need for this?

It's helpful, but I can live without it if it's a significant
maintenance pain. Just makes profiles harder (so slower) to
read/interpret in some cases.

-Boris

[Bobby Holley]

On Fri, Sep 20, 2013 at 6:06 AM, Benjamin Smedberg
<benj...@smedbergs.us> wrote:
> Currently, extensions are able to use the JSAPI via its exported symbols.
> This pretty much invariably leads to stability or security issues with
> extensions that actually do this:
>
> * the JSAPI is complicated, and even within Mozilla we get reviews from a
> select set of people on JSAPI code
> * the JSAPI is moving quickly for such features as exact rooting as well as
> things like compartments and the interaction with xpconnect. Extensions
> aren't keeping up.
> * the JSAPI is not a binary-stable interface but some people are trying to
> use it that way
>
> So I would like to propose that we link the JS libraries statically into
> libxul and stop exporting JSAPI symbols entirely. This will effectively
> prevent extensions from using it.

Yes please. Interacting with JSAPI without crashing now requires using
nsCxPusher (among other things), and the idea of addons manipulating
the JSContext stack gives me the creeps.

> There are some technical issues to work out, such as what we do with
> xpcshell. I think what I'd like to do is bake xpcshell directly into libxul.
> As a side benefit this will aid a long-term goal of mine to unify the
> startup paths across app/embedding/xpcshell launch.

That would be great. Also, there's a bunch of gunk we have to
NS_EXPORT_ in order to let xpcshell use it. Fixing this would remove
some cruft from XPConnect.

bholley

[Benjamin Smedberg]

On 9/20/13 9:06 AM, Benjamin Smedberg wrote:
> So I would like to propose that we link the JS libraries statically
> into libxul and stop exporting JSAPI symbols entirely. This will
> effectively prevent extensions from using it.

To answer questions I received privately about the impact of addon
compat. This *will* affect a few addons, but not very many. Searching
the addon MXR, I think the only affected addon is Prism, which is
long-gone. This will also affect the Norton Toolbar addon, which
currently uses JSAPI and will need to change; I already have contacts at
Norton about this.

--BDS

[Philipp Kewisch]

What about JS_NewDateObject, JS_NewDateObjectMsec, JS_ObjectIsDate,
js_DateIsValid? Most of these are in jsapi.h, and we need it in
Lightning for this code:

http://mxr.mozilla.org/comm-central/source/calendar/base/backend/libical/calDateTime.cpp#597

We are not quite ready to get rid of the libical backend right now, so
removing these functions without replacement will cause some
complications for Lightning.

Philipp

[Bobby Holley]

Can you manipulate Dates using the WebIDL bindings instead?

On Fri, Sep 20, 2013 at 12:12 PM, Philipp Kewisch <moz...@kewis.ch> wrote:

> On 9/20/13 8:22 PM, Benjamin Smedberg wrote:
>
>> On 9/20/13 9:06 AM, Benjamin Smedberg wrote:
>>
>>> So I would like to propose that we link the JS libraries statically
>>> into libxul and stop exporting JSAPI symbols entirely. This will
>>> effectively prevent extensions from using it.
>>>
>> To answer questions I received privately about the impact of addon
>>
>> compat. This *will* affect a few addons, but not very many. Searching
>> the addon MXR, I think the only affected addon is Prism, which is
>> long-gone. This will also affect the Norton Toolbar addon, which
>> currently uses JSAPI and will need to change; I already have contacts at
>> Norton about this.
>>
>> --BDS
>>
>>
> What about JS_NewDateObject, JS_NewDateObjectMsec, JS_ObjectIsDate,
> js_DateIsValid? Most of these are in jsapi.h, and we need it in Lightning
> for this code:
>

> http://mxr.mozilla.org/comm-**central/source/calendar/base/**
> backend/libical/calDateTime.**cpp#597<http://mxr.mozilla.org/comm-central/source/calendar/base/backend/libical/calDateTime.cpp#597>

>
> We are not quite ready to get rid of the libical backend right now, so
> removing these functions without replacement will cause some complications
> for Lightning.
>
> Philipp
>

> ______________________________**_________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/**listinfo/dev-platform<https://lists.mozilla.org/listinfo/dev-platform>
>

[Philipp Kewisch]

I've never used webidl bindings before. Sounds to me that I can somehow
use mozilla::dom::Date in a way that it is part of the xpcom object?

If there is a way to achieve this without jsapi, then I'm fine with it.
Implementing it just shouldn't become overly complicated.

Philipp

[Boris Zbarsky]

On 9/20/13 3:21 PM, Bobby Holley wrote:
> Can you manipulate Dates using the WebIDL bindings instead?

From C++? No.

-Boris

[Bobby Holley]

It seems like we're going to need a story for this kind of stuff. One major
goal of the bindings is to remove the need for C++ consumers to manually
muck with JSAPI. And as more web objects (TypedArrays, Promises, etc) move
into the JS engine, we're going to want a sane mechanism for manipulating
these objects from C++, ideally without hand-rolling each one individually
(as was the case for typed arrays).

We might be able to piggy-back on the Xray-to-Pure-JS work I'm doing in bug
914970.

bholley

[Boris Zbarsky]

On 9/20/13 6:23 PM, Bobby Holley wrote:
> It seems like we're going to need a story for this kind of stuff.

Sure. That story may not be WebIDL (e.g. there are proposals to not
have a Date type in IDL at all and just use raw numeric timestamps for
dates).

> One major goal of the bindings is to remove the need for C++ consumers to manually
> muck with JSAPI.

I agree.

> We might be able to piggy-back on the Xray-to-Pure-JS work I'm doing in bug
> 914970.

Possibly, yes. Then wrap it in a callback interface to an Xray
effectively or something....

-Boris

[xunxun]

于 2013/9/20 星期五 22:02, Benjamin Smedberg 写道:
> On 9/20/2013 9:23 AM, Mike Hommey wrote:
>> We're already statically linking js libraries info libxul. Except on
>> windows, but that's work in progress in bug 915735
> I am primarily worried about doing this on Windows.
>> , although we don't
>> know yet if it's going to work at all: after dealing with the xpcshell
>> crash during the instrumentation phase, we'll have to see how much
>> memory the linker is going to use, and that might not be very good news
>> ahead.
> We will probably have a complete solution for linker memory issues
> when VC2013 is released. Let's plan as if that were the case.

x64 cross to x86 toolchain?
Will you merge gkmedias.dll to xul.dll then?

And as *Makoto Kato* <https://twitter.com/makoto_kato> said, at present
VC2013 PGO has many bugs.

>
>>
>> For xpcshell, I think there's a bug around to essentially fold it into
>> libxul, except for a thin layer. Ted might have been the one to file it
>> a long time ago.
> Does anyone own that currently? If not, I'm looking for volunteers!
>
>>
>> With that being said, relatedly, while I'm hopeful we could kill the
>> --enable-shared-js option (for libxul) at some point, Boris raised an
>> interesting point in that it is useful to him with Instruments, so that
>> it can flatten everything from libmozjs in a profile. I hope we'll find
>> an alternative for him, because I wouldn't like to make things painful
>> for Boris. OTOH, supporting --enable-shared-js is already causing
>> headaches, not exporting symbols might add to the complexity.

> Hrm. Can profilers (Instruments in particular) profile by C++

> namespace? I don't particularly mind have that kind of build option
> for self-build profiling, as long as it's not on in release builds,
> but I imagine that with the new ICU combinations it could get messy

> quickly. Boris how strong is your opinion/need for this?
>
> --BDS
>

--
Best Regards,
xunxun

[Benjamin Smedberg]

On 9/20/2013 3:12 PM, Philipp Kewisch wrote:
>
> What about JS_NewDateObject, JS_NewDateObjectMsec, JS_ObjectIsDate,
> js_DateIsValid? Most of these are in jsapi.h, and we need it in
> Lightning for this code:
>
> http://mxr.mozilla.org/comm-central/source/calendar/base/backend/libical/calDateTime.cpp#597
>
>
> We are not quite ready to get rid of the libical backend right now, so
> removing these functions without replacement will cause some
> complications for Lightning.

Not knowing this code well, it seems that this code takes a double and
converts it to a JS date object with some timezone conversion. It seems
to me that you could write this code entirely in JS and just pass around
the integer or double values using IDL, to avoid the JSAPI usage
altogether. That might involve always replacing calDateTime with a
JS-implemented wrapper, but that's the kind of technique that we need to
be encouraging in general.

Is that technique a usable workaround for raw JSAPI usage?

--BDS

[Philipp Kewisch]

The code really just takes a PRTime type value and creates a javascript
date object out of it. If its a local timezone (floating), then use the
fields directly, otherwise create a jsdate from the PRTime.

Initially it seems it would be easy to replace calDateTime with a JS
component and I had started to do this, but unfortunately calDateTime is
instanciated directly (via constructor, not via xpcom) in a few
locations in our C code, so replacing only calDateTime with a JS
implementation wouldn't really work out well. Replacing all the other
components comes close to using the new backend, which is not ready yet.

If there would be some secret backdoor to continue using JSAPI that
would give us a little bit longer before we replace the whole backend, I
could live with this happening.

Philipp

[Benjamin Smedberg]

On 9/23/2013 8:45 AM, Philipp Kewisch wrote:
>
>
> Initially it seems it would be easy to replace calDateTime with a JS
> component and I had started to do this, but unfortunately calDateTime
> is instanciated directly (via constructor, not via xpcom) in a few
> locations in our C code, so replacing only calDateTime with a JS
> implementation wouldn't really work out well.

Maybe it's possible to replace the C++ constructor call with an XPCOM
service call which wraps?

> If there would be some secret backdoor to continue using JSAPI that
> would give us a little bit longer before we replace the whole backend,
> I could live with this happening.

This will be configurable, so we should be able to turn it off for
Thunderbird but turn it on for Firefox. But I don't think that any other
kind of backdoor would be better than the current situation.

--BDS

[Ehsan Akhgari]

On 2013-09-21 11:18 PM, xunxun wrote:
> 于 2013/9/20 星期五 22:02, Benjamin Smedberg 写道:
>> On 9/20/2013 9:23 AM, Mike Hommey wrote:
>>> We're already statically linking js libraries info libxul. Except on
>>> windows, but that's work in progress in bug 915735
>> I am primarily worried about doing this on Windows.
>>> , although we don't
>>> know yet if it's going to work at all: after dealing with the xpcshell
>>> crash during the instrumentation phase, we'll have to see how much
>>> memory the linker is going to use, and that might not be very good news
>>> ahead.
>> We will probably have a complete solution for linker memory issues
>> when VC2013 is released. Let's plan as if that were the case.
> x64 cross to x86 toolchain?
> Will you merge gkmedias.dll to xul.dll then?

We may do that once we're there.

Ehsan

[Philipp Kewisch]

On 9/23/13 8:20 PM, Benjamin Smedberg wrote:
> On 9/23/2013 8:45 AM, Philipp Kewisch wrote:
>>
>>
>> Initially it seems it would be easy to replace calDateTime with a JS
>> component and I had started to do this, but unfortunately calDateTime
>> is instanciated directly (via constructor, not via xpcom) in a few
>> locations in our C code, so replacing only calDateTime with a JS
>> implementation wouldn't really work out well.
> Maybe it's possible to replace the C++ constructor call with an XPCOM
> service call which wraps?

So to get/set a jsdate from a js caller, there is a call from JS -> C++
-> JS that returns a value that traverses back through that path? I
guess it would be possible, I don't know how much this will affect
performance as its used quite often.

If not exporting JS symbols is the way to go, its at least good to know
there is a workaround.

Philipp

[Benjamin Smedberg]

On 9/20/2013 9:06 AM, Benjamin Smedberg wrote:
> So I would like to propose that we link the JS libraries statically
> into libxul and stop exporting JSAPI symbols entirely. This will
> effectively prevent extensions from using it.

This has been checked in for Firefox 27 in bug 920731, in the following
cases:

* For desktop Firefox, Firefox for Android, and Firefox OS
* When JS is linked statically. Currently that's all platforms *except*
Windows. Windows will automatically stop exporting JS symbols if/when
bug 915735 lands.

--BDS