Closed Bug 682845 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Networking, defect, major)

defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla9
Tracking Status
firefox9 + ---

People

(Reporter: bzbarsky, Assigned: jesup)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Blocks: 665706
Blocks: 682697
OS: Mac OS X → All
Hardware: x86 → All
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+
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
Attachment #557603 - Attachment is obsolete: true
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+
(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.
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/c5ed260e9a49
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
Duplicate of this bug: 682697
If there's any extension compatibility issues, please add the "addon-compat" keyword to this bug.
You need to log in before you can comment on or make changes to this bug.