234636 - Freeze nsIGlobalHistory2

Status: RESOLVED WONTFIX

(Core Graveyard :: History: Global, defect, P3)

Description

[Benjamin Smedberg]

Since we have replaced nsIGlobalHistory, we need to freeze the new interface
before 1.7b/ff0.9.  I landed the interface with @status UNDER_REVIEW, but we
should probably make a NG announcement, etc.

Comment 1

[Alec Flett]

Hmm. So looking back over nsIGlobalHistory2 for the first time, I wish I had
been involved...

mostly I like that extra flags have been added to addURI(), but I'm disappointed
that its a fixed set of boolean parameters. I think we've run into a number of
cases where we want to add flags to the old AddPage() API. I anticipate that
future implementations of nsIGlobalHistory2 may want to make other policy
decisions about what pages get added to the database, and so forth, and so I
propose that we instead make a bitfield parameter so that we can add future
flags later without writing nsIGlobalHistory3

Also, I'm slightly concerned that the seamonkey-based nsGlobalHistory no longer
implements nsIGlobalHistory - what was the motivation for keeping the adapter in
docshell? Why not let the old heavyweight implementation keep this so that
consumers can call the old nsIGlobalHistory::IsVisited(), and let docshell
forget that nsIGlobalHistory even existed - it moves that bloat from the
embeddor-required docshell to the application-specific nsGlobalHistory...

Comment 2

[Benjamin Smedberg]

alecf: many apologies, I thought you were in the loop on the original bug. I
don't mind making secondary changes.

I'm concerned about putting an open-ended bitflag into an interface that's going
to be frozen. How do we define what embedders should do with unknown bitfields?
Are some bitfields going to be reserved for embedder use, and never used by
gecko/docshell?  How would embedders know what "version" of the bitfields
they're getting from docshell?

I put the nsIGlobalHistory->nsIGlobalHistory2 adapter in docshell so that the
onus of keeping backwards compatibility is on gecko, not the embedder.  I'm open
to doing it the other way, but I don't really think it would help that much.

--BDS

Comment 3

[Alec Flett]

Well, the open-ended bitfield is pretty common across gecko - we've used it in
docshell, necko, and more.. the rule is pretty straight forward about the
"unknown bits" - a "1" indiciates some special new situation that the
implementor needs to be aware of - so the assumption is that they are zero
unless otherwise set.. 

so an implementor should ignore bits that it does not know about (i.e. when
testing for known bits, should filter out exactly the bit it is looking for, so
doing 'if (aFlags & HISTORY_BIT_FOO) {' rather than 'if (aFlags >>
HISTORY_BIT_FOO) {' and assume 0 in a bit means to do the 'default' action for
that bit.

this allows docshell to set the specific bits it cares about without confusing
slightly older implementations. 

Its really more straightforward than it sounds, I promise :)

as for backwards compatibility, I guess it depends on which embeddor we're
talking about - if we're talking about a controlled environment like a device or
something, the embeddor is going to know exactly which version of mozilla is
installed and implement exactly the right nsIGlobalHistory*. If we're talking
about a GRE consumer then the backwards compatibility thing is kind of an issue.

I guess what I'm mostly concerned with is that nsGlobalHistory is backwards
compatilble with nsIGlobalHistory so that extensions and plugins written for
mozilla 1.4 and the like continue to work. I don't know the size of the adaptors
in docshell, maybe they aren't an issue?

Comment 4

[Benjamin Smedberg]

(In reply to comment #3)
> Well, the open-ended bitfield is pretty common across gecko - we've used it in
> docshell, necko, and more.. the rule is pretty straight forward about the

I get the idea, I just wasn't aware of it in frozen interfaces. In any case, I
can write a patch pretty quickly.

> as for backwards compatibility, I guess it depends on which embeddor we're
> talking about - if we're talking about a controlled environment like a device or
> something, the embeddor is going to know exactly which version of mozilla is
> installed and implement exactly the right nsIGlobalHistory*. If we're talking

Correct... I plan to update the nsEmbedGlobalHistory impl to use the new
interface, and stop building both adapters for minimo, where sideways
compatibility isn't an issue.

> I guess what I'm mostly concerned with is that nsGlobalHistory is backwards
> compatilble with nsIGlobalHistory so that extensions and plugins written for
> mozilla 1.4 and the like continue to work.

It the extension only uses nsIGlobalHistory (e.g. DOMI), the adapter works
perfectly. The only nasty case is if the extension tries to QI nsIGlobalHistory
to nsIBrowserHistory, which is not possible with the adapter. But I haven't seen
any extensions that do this, and it is a non-frozen interface anyway.

> I don't know the size of the adaptors
> in docshell, maybe they aren't an issue?

Each adapter costs about 1k. I don't see it costing much less as a part of
globalhistory instead of a separate entity. Having it separate also means that
when/if epiphany/others decide to use nsIGlobalHistory2, they get the adapter
for free.

Comment 5

[Benjamin Smedberg]

Ack! This got away. I would *really* like to freeze this for 1.8, what do we
need to do with the flags param to make this happen?

Comment 6

[Benjamin Smedberg]

Bah, this missed the 1.8 train :-(

Comment 7

[Wayne Mery (:wsmwk)]

1.9?  

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We don't freeze interfaces anymore.