828296 - Bugfix #790374 breaks binary compatibility of nsIPrefBranch.

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[Gersner]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.97 Safari/537.11

Steps to reproduce:

Bugfix #790374 introduced a new GetFloat method to nsIPrefBranch interface. Code compiled with Gecko 17 SDK or earlier will be binary incompatible with the new interface.

Comment 1

[Gersner]

Relevant Changeset
https://hg.mozilla.org/mozilla-central/rev/ac9fdc7330d7

Comment 2

[(not currently active) Ted Mielczarek]

Note that we explicitly do not claim binary compatibility between releases nowadays, so this is likely to be INVALID, unless we neglected to change the UUID of the interface, or broke this in a security dot-release or something like that.

Comment 3

[Benjamin Smedberg]

Yuck. Obviously we should have changed the IID, but now that FF18 is released I don't think we can change it for the release.

We should definitely change it in 21 (nightly) and 20 (aurora). Scott can you do that?

Release drivers, I'm not sure what to do about this for 19 (beta). Probably nothing.

Comment 4

[Scott Johnson (:jwir3)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)

> We should definitely change it in 21 (nightly) and 20 (aurora). Scott can
> you do that?
Yes.

Comment 5

[Scott Johnson (:jwir3)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> Release drivers, I'm not sure what to do about this for 19 (beta). Probably
> nothing.
Well, it's still the first week of the beta cycle. We could probably still change it.

Comment 6

[Scott Johnson (:jwir3)]

Attachment: b828296-fx20

Comment 7

[Benjamin Smedberg]

We shouldn't change it if we've already done beta1, since that's what everyone uses as the SDK for that release.

Comment 8

[Scott Johnson (:jwir3)]

Just to be clear, is the IID going to be the same for aurora and nightly? (I think so, since there aren't any other changes that I see).

Comment 9

[Scott Johnson (:jwir3)]

Comment on attachment 699899 [details] [diff] [review]
b828296-fx20

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 790374
User impact if declined: Binary compatibility will break.
Testing completed (on m-c, etc.): none yet
Risk to taking this patch (and alternatives if risky): It changes an IID, but nothing else, so very low risk.
String or UUID changes made by this patch: Changed IID for nsIPrefBranch.

Comment 10

[Scott Johnson (:jwir3)]

Comment on attachment 699899 [details] [diff] [review]
b828296-fx20

Accidentally r-'ed myself. :|

Comment 11

[Scott Johnson (:jwir3)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> We shouldn't change it if we've already done beta1, since that's what
> everyone uses as the SDK for that release.

I don't think we've done beta1 yet, since there was android test bustage on mozilla-beta.

Comment 12

[Scott Johnson (:jwir3)]

Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d5f9078e2bc6

Comment 13

[Alex Keybl [:akeybl]]

(In reply to Scott Johnson (:jwir3) from comment #11)
> (In reply to Benjamin Smedberg  [:bsmedberg] from comment #7)
> > We shouldn't change it if we've already done beta1, since that's what
> > everyone uses as the SDK for that release.
> 
> I don't think we've done beta1 yet, since there was android test bustage on
> mozilla-beta.

We've already gone to build with beta 1 (desktop/mobile), since that was only mobile talos bustage. We can't have yet another re-spin and still get a release out tomorrow.

That being said, getting this into beta 2 may not be too late. We just have to make that call with some more context.

Would somebody mind explaining why we'd ever choose to break binary compatibility in FF19, and then break it ("fix it") for FF20/21 again?

My understanding is that our options would be:
1) Land on 21/20/19 (all affected branches)
2) Don't land anywhere, given comment 2

To make that decision, I think we need more context as to why comment 2 doesn't hold true here, and it'd be good to check in with Jorge about the timing of getting this into beta 2 and communicating to add-on developers.

Comment 14

[Scott Johnson (:jwir3)]

> 2) Don't land anywhere, given comment 2
If we decide to go this route, then I'll have to backout of inbound, since I already pushed it.

Comment 15

[Benjamin Smedberg]

There's no reason *not* to land this on 20/21. It's probably not worth landing this on 19 because authors who care (people who are trying to share DLLs across releases) are already going to have to use some other magic to switch between Firefox 17 and 18 other than the IID.

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d5f9078e2bc6

Comment 17

[Alex Keybl [:akeybl]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #15)
> There's no reason *not* to land this on 20/21. It's probably not worth
> landing this on 19 because authors who care (people who are trying to share
> DLLs across releases) are already going to have to use some other magic to
> switch between Firefox 17 and 18 other than the IID.

Are you suggesting that this won't have abnormal impact to binary add-on developers in FF19? That they already need to make compatibility changes for the add-on to work with FF19 regardless?

Comment 18

[Benjamin Smedberg]

They had to make the changes for it to work in 18. And in normal cases you'd have to recompile every release. Missing IID changes like this really only affect the few addons (Skype and a few others) which try to share binaries across versions by checking IIDs.

Comment 19

[Alex Keybl [:akeybl]]

Sounds good. No need to track for any release in that case, we'll just uplift to FF20.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c20c04b0dbed