455736 - Getting an app cache should not EnsureContentViewer()

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

We seem to EnsureContentViewer() when we get an app cache.  Since we try to get one in nsHttpChannel::OpenCacheEntry, this means that as soon as we create a docshell and start an HTTP load in it we immediately create an about:blank document in the docshell.  In many cases, that document is not needed, and will be thrown away immediately.  It certainly can't provide an app cache!

It would make a lot more sense to me to:

1)  Go ahead and get the root docshell no matter whether |this| has a content
    viewer.
2)  Don't use GetInterface (or any other method that does EnsureContentViewer)
    to get the document.  Probably the right way is to QI to nsIDocShell,
    GetContentViewer, and then GetDocument() from that if the viewer is
    non-null.

Looks like a regression from bug 442806, and I'm really sorry I didn't catch this earlier....

Comment 1

[Dave Camp (:dcamp)]

Attachment: fix

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 339106 [details] [diff] [review]
fix

]>+++ b/docshell/base/nsDocShell.cpp
>@@ -456,26 +456,35 @@ NS_IMETHODIMP nsDocShell::GetInterface(c
>         nsCOMPtr<nsIDOMDocument> domDoc = do_GetInterface(rootItem);

You want to drop the "= ..." part.

r+sr=bzbarsky with that.

Comment 3

[Dave Camp (:dcamp)]

Attachment: patch as committed

Comment 4

[Dave Camp (:dcamp)]

Landed as http://hg.mozilla.org/mozilla-central/rev/1a971f9406f8

Comment 5

[Dave Camp (:dcamp)]

Sorry, I meant http://hg.mozilla.org/mozilla-central/rev/ad3074e58009

Comment 6

[Benjamin Smedberg]

Landed before 1.9.1 branching.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

adding fixed 1.9.1 per comment 6