Bug 665706 changes IDL without changing IIDs, changes nsStandardURL serialization format

RESOLVED FIXED in mozilla9

Status

()

Core
Networking
--
major
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: bz, Assigned: jesup)

Tracking

unspecified
mozilla9
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox9+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Bug 665706 changed some interfaces without changing their IIDs.  Furthermore, it changed the nsIObjectOutputStream serialization format of nsStandardURL, which will cause fastload and session restore failures.

The IIDs need to be revved.  For serialization, probably best to switch to serializing empty string and when deserializing appending whatever is read to the filepath or something...
This should ideally be resolved before the aurora migration for 9, due to the extension compat issues.
tracking-firefox9: --- → ?
Blocks: 665706
(Assignee)

Updated

6 years ago
Blocks: 682697
(Assignee)

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 2

6 years ago
Created attachment 557603 [details] [diff] [review]
Fix serializations of nsStandardURL broken by bug 665706
(Assignee)

Comment 3

6 years ago
Comment on attachment 557603 [details] [diff] [review]
Fix serializations of nsStandardURL broken by bug 665706

Patch for the serialization issues.  IID patch to come.

Tested with the tet profile here; loads and works.  Still need to test backwards and forwards compatibility with URLs with params.

bz: reasonable approach?  The backwards-compatibility aspect is the riskiest, in that an old browser would have the param in the filepath/etc where it wouldn't have before.  Otherwise we'll have to parse it out the old way here to write it out (and read it always that way); basically locking in the old serialization forever.

Need to figure out if this is backwards-compatibility issue is a real problem or if it's safe (or just a very unlikely annoyance).
Attachment #557603 - Flags: feedback?(bzbarsky)
Comment on attachment 557603 [details] [diff] [review]
Fix serializations of nsStandardURL broken by bug 665706

This looks fine, though some spaces after comma in the Merge() callsites would be good.
Attachment #557603 - Flags: feedback?(bzbarsky) → feedback+
(Assignee)

Comment 5

6 years ago
So, I've been busy.  I believe the patch here plus IID changes, and another addition to the hack to recognize different IIDs for nsIURI, will solve this problem.

But this changes the serialization yet again, even though it's effectively going back to an old one.  The question is what's the impact of that on people running this patch in FF9 (perhaps little to one).  Admittedly, limited breakage like this may be an "allowable" sort of breakage for nightly users, and if I can ascertain that it's that case I can fix it once, and pull out the "fix-it" code later after virtually all profiles have gotten updated.  Also, we must take a fix for bug 682762 also caused by the bug 665706 change.

While it definitely breaks this testcase (in bug 682697), I'd like to really understand the impact of this on users and how likely is this serialization change (if not fixed) likely to break things.  And can we ever get rid of this compatibility code?

IID+nit patch coming
Status: NEW → ASSIGNED
(Assignee)

Comment 6

6 years ago
Created attachment 559600 [details] [diff] [review]
Update IIDs, fix serializations of nsStandardURL broken by bug 665706
(Assignee)

Updated

6 years ago
Attachment #557603 - Attachment is obsolete: true
(Assignee)

Comment 7

6 years ago
Comment on attachment 559600 [details] [diff] [review]
Update IIDs, fix serializations of nsStandardURL broken by bug 665706

And we'll need SR here, unless you'd like to SR it bz and have someone else r= it (I assume we still(?) disapprove of r= and sr= from the same person if we can avoid it)
Attachment #559600 - Flags: review?(bzbarsky)
I would way rather have the breakage of this sort (affects only some tabs, etc) for nightly users than for everyone updating stable Firefox!
Comment on attachment 559600 [details] [diff] [review]
Update IIDs, fix serializations of nsStandardURL broken by bug 665706

I think that given that this is just addressing sr comments on another bug, my reviewing it is good enough...
Attachment #559600 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 10

6 years ago
(In reply to Randell Jesup [:jesup] from comment #5)
> So, I've been busy.  I believe the patch here plus IID changes, and another
> addition to the hack to recognize different IIDs for nsIURI, will solve this
> problem.

No need to update the hack as this changes nsIURL and associated IIDs, not nsIURI

> Also, we must take a fix for bug 682762 also caused by the
> bug 665706 change.

We still need that.
(Assignee)

Updated

6 years ago
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/c5ed260e9a49
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
(Assignee)

Updated

6 years ago
Duplicate of this bug: 682697
If there's any extension compatibility issues, please add the "addon-compat" keyword to this bug.
tracking-firefox9: ? → -

Updated

5 years ago
tracking-firefox9: - → +
You need to log in before you can comment on or make changes to this bug.