interface stability policy on 1.8 branch

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

interface stability policy on 1.8 branch

L. David Baron
I have some questions about the interface stability policy on the 1.8
branch:

 1. Who decided on this policy?

 2. What is the goal of this policy?  (i.e., why do we have it?)

 3. What exactly does it mean?  (i.e., what counts as an interface)

I see it mentioned a lot in review comments, but the only definition of
the policy is in
http://developer.mozilla.org/devnews/index.php/2006/01/30/tree-rules-for-the-18-branch/
which says "should", which means (according to RFC 2119) that it can be
broken given a good reason.

I'm asking because I'm wondering whether it's worthwhile for me to do
the merging in https://bugzilla.mozilla.org/show_bug.cgi?id=336791 to
land the leak fixes listed as dependencies of that bug.  That's probably
~2 days work even without worring about changing the patch to avoid
changing interfaces.  Then not changing interfaces would require
additional work (and thus additional risk).  And doing so without
hurting memory use would probably require yet more work (and more risk),
and would probably hurt performance, perhaps measurably.

-David

--
L. David Baron                                <URL: http://dbaron.org/ >
           Technical Lead, Layout & CSS, Mozilla Corporation

_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

beltzner
On 6/2/06, L. David Baron <[hidden email]> wrote:
>  1. Who decided on this policy?

As I recall, this policy came out of the December Firefox Developer's
Summit in a session organized by Brendan on branch management.

>  2. What is the goal of this policy?  (i.e., why do we have it?)

The goal was to minimize the impact of Firefox 2 (a primarily
front-end update to Firefox 1.5) on our extension development
community by not revving the APIs unless absolutely necessary, thus
enabling most existing extensions to "just work" with Firefox 2 (once
they'd been smoketested and had their maxVers bumped, natch).

>  3. What exactly does it mean?  (i.e., what counts as an interface)

Here I can't help you. Connor? Brendan? FWIW, though, I've heard
rumours that SafeBrowsing ended up chaging some existing APIs in
addition to adding some. Not sure if or how that decision was made
properly.

cheers,
mike
--
/ mike beltzner / user experience lead / mozilla corporation /
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
On 6/1/06, beltzner <[hidden email]> wrote:
> On 6/2/06, L. David Baron <[hidden email]> wrote:
> >  1. Who decided on this policy?
>
> As I recall, this policy came out of the December Firefox Developer's
> Summit in a session organized by Brendan on branch management.

Yup, that's what I recall as well.


> >  2. What is the goal of this policy?  (i.e., why do we have it?)
>
> The goal was to minimize the impact of Firefox 2 (a primarily
> front-end update to Firefox 1.5) on our extension development
> community by not revving the APIs unless absolutely necessary, thus
> enabling most existing extensions to "just work" with Firefox 2 (once
> they'd been smoketested and had their maxVers bumped, natch).

Exactly.


> >  3. What exactly does it mean?  (i.e., what counts as an interface)
>
> Here I can't help you. Connor? Brendan? FWIW, though, I've heard
> rumours that SafeBrowsing ended up chaging some existing APIs in
> addition to adding some. Not sure if or how that decision was made
> properly.

In general, we've said that any interface reachable via QueryInterface
is considered something we don't want to change if we can help it.  I
think you can extend this to XBL-defined properties, global functions
in browser.js, etc.

By the way, SafeBrowsing was purely additive, so I don't think it
would have changed any APIs.  It definitely did add an URL
classification API to toolkit, but API addition is not a problem ;-)

Perhaps, in the case of bug 336791, it would help if you could list
out the affected interfaces?  Then, we can review the changes
one-by-one?

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

L. David Baron
On Friday 2006-06-02 08:36 -0700, Darin Fisher wrote:
> Perhaps, in the case of bug 336791, it would help if you could list
> out the affected interfaces?  Then, we can review the changes
> one-by-one?

It's at least the following:
nsIAttribute
nsIContent
nsIDocument
nsPIWindowRoot
imgIDecoderObserver

The first 4 could be worked around with a 1-word penalty on every object
by not adding to the inheritance chain, but I'd really rather not do
that for nsIContent.

The latter could be worked around with a 1-word penalty on a whole bunch
of objects plus a bit of performance loss to extra QueryInterface calls
(which I'd also rather not have).

-David

--
L. David Baron                                <URL: http://dbaron.org/ >
           Technical Lead, Layout & CSS, Mozilla Corporation

_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
I wanted to point out that the other reason for not changing
interfaces is because Firefox 2 will be shipping Gecko 1.8.1, and so
based on that version number, it could be reasonably expected that the
new Gecko should be backwards compatible with Gecko 1.8.0.


On 6/2/06, L. David Baron <[hidden email]> wrote:

> On Friday 2006-06-02 08:36 -0700, Darin Fisher wrote:
> > Perhaps, in the case of bug 336791, it would help if you could list
> > out the affected interfaces?  Then, we can review the changes
> > one-by-one?
>
> It's at least the following:
> nsIAttribute
> nsIContent
> nsIDocument
> nsPIWindowRoot

These are definitely internal APIs, but they are sadly needed by
extensions in order to do many reasonable things.  For example, the
browser metrics extension (mozilla/extensions/metrics) is forced to
use nsIDocument in order to observe document destruction.  Bryner was
telling me about how much of a pain it is to try to make use of that
API across all the various versions of Firefox (mostly because of all
of the things it pulls in).  If we had a better, more stable API that
provided the same information for Firefox 2, then maybe it wouldn't
matter to the browser metrics extension if the interface changed.  In
other cases, nsIContent and nsIDocument end up being the only way to
access the associated nsIChannel or nsIURI objects.  For a variety of
reasons, extensions may be dipping into these interfaces, so we should
be careful.  However, maybe we can tolerate changes to these
interfaces for Gecko 1.8.1 because they are very internal and
non-scriptable.  It's not like extension authors won't be able to
cope, we just want to minimize the cost for them to cope.


> imgIDecoderObserver

This interface is needed if you want to use image lib to load and
decode images.  I know of a couple binary extensions that make use of
imgILoader, but they do not go so far as to implement
imgIDecoderObserver and so they wouldn't suffer from an interface
change here.  Others might.  Hard to say.

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Mike Shaver
In reply to this post by Darin Fisher-2
On 6/2/06, Darin Fisher <[hidden email]> wrote:
> In general, we've said that any interface reachable via QueryInterface
> is considered something we don't want to change if we can help it.  I
> think you can extend this to XBL-defined properties, global functions
> in browser.js, etc.

Does this extend to non-XPCOM interfaces that are available to plugins
and extensions?  One example is a binary-incompatible change in the JS
engine (we changed the size of JSFunctionSpec, which is passed from
client to engine and never the other way)

Do plugins or extensions link against or call into the JS engine in a
way that would cause them grief here?  It seems to me that they might
well, but I can't point at one definitively.

Mike
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
On 6/4/06, Mike Shaver <[hidden email]> wrote:

> On 6/2/06, Darin Fisher <[hidden email]> wrote:
> > In general, we've said that any interface reachable via QueryInterface
> > is considered something we don't want to change if we can help it.  I
> > think you can extend this to XBL-defined properties, global functions
> > in browser.js, etc.
>
> Does this extend to non-XPCOM interfaces that are available to plugins
> and extensions?  One example is a binary-incompatible change in the JS
> engine (we changed the size of JSFunctionSpec, which is passed from
> client to engine and never the other way)
>
> Do plugins or extensions link against or call into the JS engine in a
> way that would cause them grief here?  It seems to me that they might
> well, but I can't point at one definitively.

I don' t know about that particular example, but I do know that
plug-ins are often forced to use the JS API directly if they want to
do anything interesting with Javascript.  When it comes to
implementing APIs invoked by Javascript, there's only so much you can
do with XPCOM.  It isn't expressive enough, so you end up dropping
down to JS API calls to do things like grok optional parameters, etc.
It's hard to say how much more of the JS API an extension might use (I
have no idea).

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
On 6/4/06, Darin Fisher <[hidden email]> wrote:

> On 6/4/06, Mike Shaver <[hidden email]> wrote:
> > On 6/2/06, Darin Fisher <[hidden email]> wrote:
> > > In general, we've said that any interface reachable via QueryInterface
> > > is considered something we don't want to change if we can help it.  I
> > > think you can extend this to XBL-defined properties, global functions
> > > in browser.js, etc.
> >
> > Does this extend to non-XPCOM interfaces that are available to plugins
> > and extensions?  One example is a binary-incompatible change in the JS
> > engine (we changed the size of JSFunctionSpec, which is passed from
> > client to engine and never the other way)
> >
> > Do plugins or extensions link against or call into the JS engine in a
> > way that would cause them grief here?  It seems to me that they might
> > well, but I can't point at one definitively.
>
> I don' t know about that particular example, but I do know that
> plug-ins are often forced to use the JS API directly if they want to
> do anything interesting with Javascript.  When it comes to
> implementing APIs invoked by Javascript, there's only so much you can
> do with XPCOM.  It isn't expressive enough, so you end up dropping
> down to JS API calls to do things like grok optional parameters, etc.
> It's hard to say how much more of the JS API an extension might use (I
> have no idea).

s/plug-ins/extension/ ... or maybe I meant both, in which case... add-ons!

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Brendan Eich-2
In reply to this post by Darin Fisher-2
True Netscape-style plugins use NPRuntime, not the JS API, for  
scriptability.  We've never documented the JS API to plugin  
developers as a supported interface.

XUL extensions could use lots of things, but I don't know of any that  
depend on JSFunctionSpec.  Is there a way to search AMO easily?  How  
many extensions hosted at AMO or extension-mirror.nl (or whatever  
it's called) even contain compiled code?

/be
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Gijs Kruitbosch ("Hannibal")
In reply to this post by Darin Fisher-2
Brendan Eich wrote:
 > XUL extensions could use lots of things, but I don't know of any that
 > depend on JSFunctionSpec.  Is there a way to search AMO easily?  How
 > many extensions hosted at AMO or extension-mirror.nl (or whatever it's
 > called) even contain compiled code?
 >
 > /be

I think there are a fair few, and to make matters worse, no
'open-sourceness' is forced. I don't want to debate the whole
ideological thing** but the fact that you can't peek at the source for
the compiled components will make it harder to search it. I mean, right
now I'm certain that there is no way to do that, and I'm thinking that
making such a way exist for extensions would be a 'hard' problem.

-- Gijs

** so please, no replies from whoever else reads this concerning that,
make a different topic in a more relevant group iff you absolutely must...
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Mike Shaver
In reply to this post by Brendan Eich-2
On 6/5/06, Brendan Eich <[hidden email]> wrote:
> XUL extensions could use lots of things, but I don't know of any that
> depend on JSFunctionSpec.  Is there a way to search AMO easily?  How
> many extensions hosted at AMO or extension-mirror.nl (or whatever
> it's called) even contain compiled code?

A non-zero number, it's not easy to search and find out which or how.
But even if none there do, I'd be pretty surprised to discover that no
plugins or extensions against Fx1.5 used the JSAPI.  IIRC, it was in
part for compatibility with plugins linking against JS directly that
we haven't pulled libmozjs.so into the static part of the Firefox
build, but Benjamin would know more about that.

Mike
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Benjamin Smedberg
In reply to this post by Brendan Eich-2
Mike Shaver wrote:
> On 6/5/06, Brendan Eich <[hidden email]> wrote:
>> XUL extensions could use lots of things, but I don't know of any that
>> depend on JSFunctionSpec.  Is there a way to search AMO easily?  How
>> many extensions hosted at AMO or extension-mirror.nl (or whatever
>> it's called) even contain compiled code?

JSFunctionSpec is not part of the public JSAPI (jsapi.h), correct? Just as
we are still free to change implementation-class layout for gecko181, we
should feel free to change the internal implementation of spidermonkey for 181.

> A non-zero number, it's not easy to search and find out which or how.
> But even if none there do, I'd be pretty surprised to discover that no
> plugins or extensions against Fx1.5 used the JSAPI.  IIRC, it was in
> part for compatibility with plugins linking against JS directly that
> we haven't pulled libmozjs.so into the static part of the Firefox
> build, but Benjamin would know more about that.

There are most-certainly plugins about that claim to use only frozen
interfaces and link against libmozjs.so. I don't know why; but I've grown to
accept that I won't understand everything in the world.

--BDS
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Benjamin Smedberg
In reply to this post by beltzner
Darin Fisher wrote:

> think you can extend this to XBL-defined properties, global functions
> in browser.js, etc.

On the contrary, we are explicitly excluding the global functions/properties
in browser.js (or anywhere in the chrome) from the stability requirement.
The "stable interfaces" rule was put in place primarily for plugins and
binary extensions, not for XUL/JS extensions, which will most likely need to
be revised for various chrome updates anyway.

As for the patch at hand, we have been very careful not to modify
nsIContent/nsIDocument and I don't think we should on the branch.

--BDS
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Mike Shaver
In reply to this post by Benjamin Smedberg
On 6/6/06, Benjamin Smedberg <[hidden email]> wrote:

> Mike Shaver wrote:
> > On 6/5/06, Brendan Eich <[hidden email]> wrote:
> >> XUL extensions could use lots of things, but I don't know of any that
> >> depend on JSFunctionSpec.  Is there a way to search AMO easily?  How
> >> many extensions hosted at AMO or extension-mirror.nl (or whatever
> >> it's called) even contain compiled code?
>
> JSFunctionSpec is not part of the public JSAPI (jsapi.h), correct? Just as
> we are still free to change implementation-class layout for gecko181, we
> should feel free to change the internal implementation of spidermonkey for 181.

JSFunctionSpec is part of the public JSAPI, though it's defined in
jspubtd.h (JS PUBlic Type Definitions; included by jsapi.h).  They're
passed to JS_DefineFunctions and JS_InitClass at least, which are
pretty common in JS embeddings.  Dunno if they're likewise common in
plugins and extensions.

> There are most-certainly plugins about that claim to use only frozen
> interfaces and link against libmozjs.so. I don't know why; but I've grown to
> accept that I won't understand everything in the world.

If we can look at the plugins, we can see what symbols they use, but
that's not really a reliable investigation path for our purposes.  If
symbol versioning actually worked, we could replace the
JSFunctionSpec-taking public functions for the older versions with
ones that mapped between old and new structures.

Also, I would like a pony.

Mike
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Boris Zbarsky
In reply to this post by Benjamin Smedberg
Benjamin Smedberg wrote:
> As for the patch at hand, we have been very careful not to modify
> nsIContent/nsIDocument and I don't think we should on the branch.

I don't think we've really had a case quite like this come up so far:

1)  Very big win from the change (bigger win than any other "platform uplift"
     stuff we've taken or will take on branch, imo).
2)  No sane way to make the change without changing nsIContent/nsIDocument

So basically, our options seem to be:

A)  Forgo the win.
B)  Increase data structure size and reduce perf; introduce a very large
     branch/trunk skew; land less well-tested code on branch.
C)  Modify nsIContent/nsIDocument on the branch.

In retrospect, we should have landed nsIDOMGCParticipant (with no impl or
something) on the branch before we cut it, but that's 20-20 hindsight for us.  :(

Going forward, I'd vote for either A or C above; B really doesn't appeal, and
I'm not even the one who'd have to do the work.

-Boris
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
In reply to this post by Benjamin Smedberg
On 6/6/06, Benjamin Smedberg <[hidden email]> wrote:

> Darin Fisher wrote:
>
> > think you can extend this to XBL-defined properties, global functions
> > in browser.js, etc.
>
> On the contrary, we are explicitly excluding the global functions/properties
> in browser.js (or anywhere in the chrome) from the stability requirement.
> The "stable interfaces" rule was put in place primarily for plugins and
> binary extensions, not for XUL/JS extensions, which will most likely need to
> be revised for various chrome updates anyway.

If the goal is helping ensure that most extensions continue working,
then we are best served by preserving as many "APIs" as we can.  It is
true that our UI has changed, and that will cause grief for some
overlay-based extensions, but that doesn't mean that we should punt
entirely on "XUL" API stability.

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
In reply to this post by Boris Zbarsky
On 6/6/06, Boris Zbarsky <[hidden email]> wrote:

> Benjamin Smedberg wrote:
> > As for the patch at hand, we have been very careful not to modify
> > nsIContent/nsIDocument and I don't think we should on the branch.
>
> I don't think we've really had a case quite like this come up so far:
>
> 1)  Very big win from the change (bigger win than any other "platform uplift"
>      stuff we've taken or will take on branch, imo).
> 2)  No sane way to make the change without changing nsIContent/nsIDocument
>
> So basically, our options seem to be:
>
> A)  Forgo the win.
> B)  Increase data structure size and reduce perf; introduce a very large
>      branch/trunk skew; land less well-tested code on branch.
> C)  Modify nsIContent/nsIDocument on the branch.
>
> In retrospect, we should have landed nsIDOMGCParticipant (with no impl or
> something) on the branch before we cut it, but that's 20-20 hindsight for us.  :(
>
> Going forward, I'd vote for either A or C above; B really doesn't appeal, and
> I'm not even the one who'd have to do the work.
>
> -Boris

We discussed this at today's bon echo meeting.  The general consensus
was that this might be the kind of change that could justify
nsIContent/nsIDocument API changes for the very reasons that boris
enumerates.

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

L. David Baron
In reply to this post by L. David Baron
On Friday 2006-06-02 08:52 -0700, L. David Baron wrote:

> On Friday 2006-06-02 08:36 -0700, Darin Fisher wrote:
> > Perhaps, in the case of bug 336791, it would help if you could list
> > out the affected interfaces?  Then, we can review the changes
> > one-by-one?
>
> It's at least the following:
> nsIAttribute
> nsIContent
> nsIDocument
> nsPIWindowRoot
> imgIDecoderObserver
So I've taken the standard procedures for not modifying interfaces
(creating _MOZILLA_1_8_BRANCH interfaces or adding new interfaces as
additional interfaces on the implementations rather than in the
inheritance chain) so that the only modified interface is nsIContent.
(As I've said before, I'd really rather not pay the 1-word penalty for
every element and every text node.)  See the patch and diff on bug
336791.

I did something a little funky to nsIEventListenerManager.h.  I changed
a method name but just left an argument unused (with a C++ default
argument) to account for it no longer having any parameters.  And I did
not change the IID even though the calling protocol is technically
slightly different.  (Although the only problem with callers doing
things the old way, if the callers were doing it correctly and calling
both RemoveAllListeners and SetListenerTarget(nsnull), which many
weren't, is that it should assert.  The purpose of the change in
protocol was mainly to make callers get it right -- rename
RemoveAllListeners to Disconnect, make it also set the target to null,
and forbid (by assertion) SetListenerTarget from being called with a
null argument.)

Does anybody object to this (nsIContent or nsIEventListenerManager)?

-David

--
L. David Baron                                <URL: http://dbaron.org/ >
           Technical Lead, Layout & CSS, Mozilla Corporation

_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Benjamin Smedberg
In reply to this post by L. David Baron
L. David Baron wrote:

> I did something a little funky to nsIEventListenerManager.h.  I changed
> a method name but just left an argument unused (with a C++ default
> argument) to account for it no longer having any parameters.  And I did
> not change the IID even though the calling protocol is technically
> slightly different.  (Although the only problem with callers doing

So the method has the same signature, but with a C++ default arg? Do you
envision that coders may need to detect whether they're calling the "old"
signature or the "new" signature? If we're going to rev nsIContent, I'm not
sure it's not worth revving nsIEventListenerManager also.

Darin, what do you think?

--BDS
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
Reply | Threaded
Open this post in threaded view
|

Re: interface stability policy on 1.8 branch

Darin Fisher-2
On 6/12/06, Benjamin Smedberg <[hidden email]> wrote:

> L. David Baron wrote:
>
> > I did something a little funky to nsIEventListenerManager.h.  I changed
> > a method name but just left an argument unused (with a C++ default
> > argument) to account for it no longer having any parameters.  And I did
> > not change the IID even though the calling protocol is technically
> > slightly different.  (Although the only problem with callers doing
>
> So the method has the same signature, but with a C++ default arg? Do you
> envision that coders may need to detect whether they're calling the "old"
> signature or the "new" signature? If we're going to rev nsIContent, I'm not
> sure it's not worth revving nsIEventListenerManager also.
>
> Darin, what do you think?

I don't know enough about the APIs in question.  FWIW, I don't have
any objections to David's proposed changes.

-Darin
_______________________________________________
dev-planning mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-planning
12