Last Comment Bug 682031 - Cannot access App Tab iframe.document in tab from session restore after FF upgrade
: Cannot access App Tab iframe.document in tab from session restore after FF up...
Status: VERIFIED FIXED
[qa!]
:
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: Other Other
: -- normal (vote)
: mozilla9
Assigned To: Randell Jesup [:jesup]
:
Mentors:
Depends on:
Blocks: 677260
  Show dependency treegraph
 
Reported: 2011-08-25 11:09 PDT by Stefan
Modified: 2013-12-27 14:22 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed


Attachments
Add another old nsIURI IID to the hack in nsBinaryStream to fix sessionrestore (1.24 KB, patch)
2011-08-25 15:43 PDT, Randell Jesup [:jesup]
dholbert: review+
bzbarsky: review+
bugzilla: approval‑mozilla‑aurora+
Details | Diff | Review

Description Stefan 2011-08-25 11:09:00 PDT
User Agent:  

Steps to reproduce:

1. Start Fx 2011-08-14-03-07-49-mozilla-central with new profile.
2. Open Testcase from Bug 662242 
   https://bugzilla.mozilla.org/attachment.cgi?id=537650
   Pin tab as app tab. 
3. Close Fx.
4. Start Fx 2011-08-21-03-07-58-mozilla-central.
4. Click at the 'A'-button.


Actual results:

- no "success" string
- Error: Permission denied to access property 'document'
Source File: https://bug662242.bugzilla.mozilla.org/attachment.cgi?id=537517 Line: 10


Expected results:

"success" message
Comment 1 Boris Zbarsky [:bz] 2011-08-25 11:54:27 PDT
Stefan, thanks for filing this.  I would guess that this broke when bug 677260 change the nsIURI iid, because nsPrincipal writes out the IID as part of the data it writes.

There are existing hacks in nsBinaryInputStream::ReadObject to deal with that; we need to add another one.

We should consider changing nsPrincipal to write the URIs out as nsISupports.....

Randell, this is a fx9 issue only, right, or did bug 677260 make fx8 aurora?
Comment 2 Daniel Holbert [:dholbert] 2011-08-25 11:58:22 PDT
(In reply to Boris Zbarsky (:bz) from comment #1)
> Stefan, thanks for filing this.  I would guess that this broke when bug
> 677260 change the nsIURI iid, because nsPrincipal writes out the IID as part
> of the data it writes.

Ah yes -- sadly, this breaks (read: needs an addition to the existing hackaround) whenever the nsIURI iid changes.  We really should change nsPrincipal as bz suggests. (That's bug 662693)

See comment about this in the nsIURI idl file:
 http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsIURI.idl#98
Comment 3 Randell Jesup [:jesup] 2011-08-25 14:45:33 PDT
Affects FF8 as well (change made the 8 cutoff)
Comment 4 Randell Jesup [:jesup] 2011-08-25 15:43:43 PDT
Created attachment 555872 [details] [diff] [review]
Add another old nsIURI IID to the hack in nsBinaryStream to fix sessionrestore
Comment 5 Randell Jesup [:jesup] 2011-08-25 15:45:44 PDT
Comment on attachment 555872 [details] [diff] [review]
Add another old nsIURI IID to the hack in nsBinaryStream to fix sessionrestore

full disclosure: I've built and run it, but I haven't tested the sequence given yet -- will do so tonight.
Comment 6 Daniel Holbert [:dholbert] 2011-08-25 15:56:38 PDT
Comment on attachment 555872 [details] [diff] [review]
Add another old nsIURI IID to the hack in nsBinaryStream to fix sessionrestore

Looks good to me (though I hope this is the last extension of this hack that we'll need... you or I or someone should really fix bug 662693 before the next time we tweak nsIURI. :))

I think you'll need sign-off from someone with more xpcom know-how, too. (I've written very few patches for /xpcom, and I don't believe I've ever reviewed any patches there).  Flagging additional-r?bz.

We should get this into Aurora as soon as we can.
Comment 7 Boris Zbarsky [:bz] 2011-08-25 19:04:16 PDT
Comment on attachment 555872 [details] [diff] [review]
Add another old nsIURI IID to the hack in nsBinaryStream to fix sessionrestore

r=me
Comment 8 Boris Zbarsky [:bz] 2011-08-25 19:04:59 PDT
I'm actually tempted to say that we should remove the function signatures that serialize any IID other than nsISupports....  and then not save the iid at all.
Comment 9 Boris Zbarsky [:bz] 2011-08-25 19:05:13 PDT
But that's definitely a separate bug.
Comment 10 Randell Jesup [:jesup] 2011-08-25 23:26:33 PDT
Verified with testcase
Comment 11 Matt Brubeck (:mbrubeck) 2011-08-26 09:27:25 PDT
http://hg.mozilla.org/mozilla-central/rev/e5adec25155d
Comment 12 Stefan 2011-08-27 23:34:45 PDT
(In reply to Randell Jesup [:jesup] from comment #10)
> Verified with testcase

Sure that you verified with the correct versions? I just encountered the bug when starting a Fx built from current revision 6c8a909977d3 on a profile which was written with a revision 3d615a56ad46 (of 2011-08-17).
Comment 13 Stefan 2011-08-28 03:26:31 PDT
Bug 682697 Submitted
Comment 14 Asa Dotzler [:asa] 2011-09-01 14:58:14 PDT
tracking this and what appears to be a regression from this fix in bug 682269
Comment 15 Randell Jesup [:jesup] 2011-09-01 15:20:07 PDT
Probably not Bug 682269 - Order a new laptop for laura

Probably bug 682845 (bug 682647 is almost certainly a dup of that)
Comment 16 Boris Zbarsky [:bz] 2011-09-01 15:35:47 PDT
There are no known regressions from this fix.  And this fix does need to land on aurora.  Randell, please request approval accordingly.
Comment 17 Randell Jesup [:jesup] 2011-09-01 19:42:17 PDT
Request made.
Comment 18 Boris Zbarsky [:bz] 2011-09-01 19:58:51 PDT
But you also nuked the tracking flags... :(
Comment 19 Boris Zbarsky [:bz] 2011-09-01 19:59:24 PDT
And I meant request approval on the _patch_, not request tracking on the bug!
Comment 20 Randell Jesup [:jesup] 2011-09-22 21:56:58 PDT
In Aurora as https://hg.mozilla.org/releases/mozilla-aurora/rev/6f21b34b00c6
Comment 21 Virgil Dicu [:virgil] [QA] 2011-10-06 08:10:02 PDT
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:9.0a2) Gecko/20111004 Firefox/9.0a2
Mozilla/5.0 (Windows NT 6.1; rv:10.0a1) Gecko/20111004 Firefox/10.0a1
Mozilla/5.0 (Windows NT 5.1; rv:10.0a1) Gecko/20111004 Firefox/10.0a1
Mozilla/5.0 (Windows NT 5.1; rv:9.0a2) Gecko/20111004 Firefox/9.0a2
Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111005 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111004 Firefox/10.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:8.0a2) Gecko/20110921 Firefox/8.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0a1) Gecko/20110919 Firefox/9.0a1

Verified on Mac OS 10.6, Ubuntu 11.04, Windows Xp and 7 using the steps in comment 0. I have upgraded from Firefox 7.0.1 to Firefox 8, from Firefox 8 to 9 and from 9 to 10. The success string was displayed as expected.
Comment 22 Virgil Dicu [:virgil] [QA] 2011-10-06 08:12:41 PDT
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a2) Gecko/20111006 Firefox/9.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a1) Gecko/20111006 Firefox/10.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0

These are the correct build ids on Mac OS 10.6.

Note You need to log in before you can comment on or make changes to this bug.