Last Comment Bug 293175 - Inconsistent document container handling
: Inconsistent document container handling
Status: NEW
[ETA ?]
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: x86 Linux
-- normal with 4 votes (vote)
: mozilla1.8beta4
Assigned To: Nobody; OK to take it and work on it
: Andrew Overholt [:overholt]
Depends on:
Blocks: blazinglyfastback 292960 292961 293171
  Show dependency treegraph
Reported: 2005-05-06 12:53 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2008-09-03 08:35 PDT (History)
17 users (show)
benjamin: blocking1.8b5-
pavlov: blocking1.9-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description User image Boris Zbarsky [:bz] (still a bit busy) 2005-05-06 12:53:55 PDT
Before bfcache we had the invariant that a document's container was immutable
after document creation and was the docshell the document was in.  With bfcache,
this is still true _except_ for bfcached documents, which have a null container.

Should there really be a difference here between bfcache documents and other
documents that have been navigated away from?  That seems wrong to me.  We
should probably unset the container consistently, and make sure document
teardown still works right...

Also, should documents created via DOMImplementation really have a container
set?  That seems wrong to me.
Comment 1 User image Brian Ryner (not reading) 2005-07-19 23:35:44 PDT
I agree, but this is a separate issue from bfcache.  Unmarking dependency to get
it off the radar.
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2005-07-20 08:32:35 PDT
Er...  We have an invariant that code may be depending on and that bfcache
violates.  Until we check that all code that uses this container deals with it
being null and that for a bfcached document this leads to the right behavior,
this is in fact a bfcache issue.  If such checking has already been done, then
my apologies; let me know and I'll remove the dependency in that case.
Comment 3 User image Brian Ryner (not reading) 2005-07-21 15:07:53 PDT
pulling into beta for investigation
Comment 4 User image Asa Dotzler [:asa] 2005-09-21 12:07:26 PDT
bryner, have you had a chance to look into this? 
Comment 5 User image Asa Dotzler [:asa] 2005-09-21 14:41:16 PDT
bryner's busy with other bug fixing. Who else could help here?
Comment 6 User image Brendan Eich [:brendan] 2005-09-21 14:54:13 PDT
peterv, maybe?

Comment 7 User image Asa Dotzler [:asa] 2005-09-29 12:29:41 PDT
We're not getting anywhere here. Who else can help? Johnny, Bryner, any ideas
here? Time is running out.
Comment 8 User image Benjamin Smedberg [:bsmedberg] 2005-09-30 11:43:24 PDT
Without an actual example of somebody relying on this former invariant, we're
not going to block on this.
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2005-09-30 13:11:48 PDT
Anyone calling GetScriptGlobalObject() is relying on it.
Comment 10 User image Boris Zbarsky [:bz] (still a bit busy) 2005-09-30 13:14:01 PDT
Calling it after nsDocument::Destroy, that is.
Comment 11 User image Benjamin Smedberg [:bsmedberg] 2005-09-30 13:16:01 PDT
But do we have an case of that happening in-tree or in common extensionland?
Comment 12 User image Boris Zbarsky [:bz] (still a bit busy) 2005-09-30 14:09:54 PDT
We have at least some known in-tree cases; they're already covered by separate
bugs (eg XTF has this issue at document teardown).  I have no idea about
extensions.  The whole point of this bug was to either fix the issue or to check
that our in-tree consumers are OK with this.  Since the latter hasn't happened,
I can't tell you whether they're OK or not, clearly.
Comment 13 User image Brian Ryner (not reading) 2006-08-27 13:55:33 PDT
I think all of the issues we know about here have been addressed.
Comment 14 User image Boris Zbarsky [:bz] (still a bit busy) 2006-08-27 14:17:30 PDT
Quick check turns up XBL code that definitely calls GetScriptGlobalObject() in cases when bfcache has nulled out the container (so whatever that code is trying to do doesn't work).  We do have a separate bug on that, but it sure isn't fixed...

Given that, I doubt that the code-reading that needs to happen here has been done.
Comment 15 User image Brian Ryner (not reading) 2008-03-25 20:54:18 PDT
Reassigning my bugs, since I'm not actually working on them.

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