273319 - [FIXr]Eliminate nsIWebShell and nsIWebShellContainer

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

Once bug 272471 is fixed, it should be pretty simple to eliminate nsIWebShell
and nsIWebShellContainer.  The comments that bug's patch adds to nsIWebShell
describe what needs to be done.

Comment 1

[Boris Zbarsky [:bzbarsky]]

We'll want to kill nsIWebShellServices too, I think.  Probably a separate bug...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Another note to self -- nsIXULCommand seems unused.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I field bug 273812 on fixing the mac widget code to not use nsIDocShell (I'm
switching it to that from nsIWebShell).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: With nsIWebShell.h actually removed

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 168319 [details] [diff] [review]
With nsIWebShell.h actually removed

What do you guys think?  Basic changes are:

1)  Switch things like viewer and mac menu code to nsIDocShell instead.

2)  Remove some dead code in viewer.

3)  Switch mailnews off of nsIWebShellContainer.

I did verify that cached compose windows work fine in the build with this
patch.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 168319 [details] [diff] [review]
With nsIWebShell.h actually removed

Very nice cleanup! sr=jst

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 168319 [details] [diff] [review]
With nsIWebShell.h actually removed

xpcom/tests/SizeTest06.cpp

a kinda pointless change, this won't compile anyway :-) but sure.

webshell/tests/viewer/nsBrowserWindow.cpp	9
-//	 nsAutoString theString(theURL);
-//	 mApp->ViewSource(theString);
       //XXX Find out how the string is allocated, and perhaps delete it...

unsure what the comment refers to, but maybe to theString? in which case, it
should be deleted together with the commented-out code...


stopping before xpfe/appshell/src/nsWebShellWindow.cpp, will finish the review
tomorrow...

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 168319 [details] [diff] [review]
With nsIWebShell.h actually removed

(I only skimmed the mac menu changes, assuming they are basically
s/DocShell/WebShell/g)


+nsWebShellWindow::GetDocShell(nsIDocShell *& aDocShell)

hmm, maybe that signature should be changed at some point to use the more
common nsIDocShell** parameter type

Comment 10

[Boris Zbarsky [:bzbarsky]]

webshellwindow may need extra smacking too....  I'd rather move us to
nsWebBrowser and kill off webshellwindow, really.

I'll land this when I get back.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to tip

Comment 12

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

note that this broke beos; should be fixed now with
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/widget/src/beos&command=DIFF_FRAMESET&file=Makefile.in&rev1=1.46&rev2=1.47&root=/cvsroot
(a REQUIRES change)

Comment 14

[timeless]

fwiw this also broke os/2
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla%2Fwidget%2Fsrc%2Fos2%2FMakefile.in&rev=1.60