1294638 - Remove appId and isInIsolatedMozBrowserElement from nsIDocShell because they're in nsILoadContext

Status: RESOLVED WONTFIX

(Core :: DOM: Navigation, defect)

Description

[Nicholas Nethercote [inactive]]

Bug 775860 duplicated two attributes from nsIDocShell into nsILoadContext.
Given that nsDocShell is the only class that implements nsIDocShell, *and* it
also implements nsILoadContext, we can remove the attributes from nsIDocShell.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Remove appId and isInIsolatedMozBrowserElement from nsIDocShell because they're in nsILoadContext

I discovered this while modifying the code generation of infallible attributes;
having two identical attributes (one fallible, one infallible) used by
nsDocShell was causing some difficulties. Avoiding the duplication seems like a
good thing to do.

Comment 2

[Nicholas Nethercote [inactive]]

Try looks reasonable: https://treeherder.mozilla.org/#/jobs?repo=try&revision=65a6c8123e48

I'm not sure if the combination of casting and QI'ing in the patch is optimal. Suggestions welcome.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

so infallible doesn't work in nsILoadContext because it isn't builtinclass or what?

Comment 4

[Nicholas Nethercote [inactive]]

(In reply to Olli Pettay [:smaug] from comment #3)
> so infallible doesn't work in nsILoadContext because it isn't builtinclass

That's correct.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8780407 [details] [diff] [review]
Remove appId and isInIsolatedMozBrowserElement from nsIDocShell because they're in nsILoadContext

Conceptually, the attrs on loadcontext are information about a load, while the ones on docshell are information about a spec navigation context...

And in general, I think having docshell directly implement nsILoadContext was a mistake; we should have used a shim object that forwarded some stuff to the docshell.  I would really much rather we tried to fix that mistake instead of enshrining it even more.

Comment 6

[Nicholas Nethercote [inactive]]

Fair enough.