113970 - Clean up mozilla/webshell

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Stephen Walker]

Is this file still needed? It only contains:

#include "nsIWebShell.h"

void
XXXNeverCalled()
	{
	}

Comment 1

[Adam Lock]

Large swathes of mozilla/webshell could be removed I suspect.

1. mozilla/webshell/embed
   I have no idea wtf ngprefs is but it looks like it's never been touched since
   it was added 3 years ago
2. mozilla/webshell/src
   All webshell source got moved to to mozilla/docshell
3. mozilla/webshell/tests
   I *really* wish I could kill viewer. I suspect the layout people may still
   use it, though they should really be testing in mfcEmbed or Mozilla
   these days.

So all we'd be left with is mozilla/webshell/public. This contains the
nsIWebShell* interfaces and could either stay where it is or move over to
docshell (which could use a mozilla/docshell/public directory)

Comment 2

[Adam Lock]

Also, the following methods on nsIWebShell appear to be killable:

FindNext
GetRootWebShell
GetReferrer
GetTopLevelWindow
GetHistoryLength
GetHistoryIndex (with some work to viewer.exe)
GetURL (ditto)
SetURL

And their impls in nsWebShell plus these methods too

GetParent
SetParent
GetRootWebShellEvenIfChrome

That should leave some nsIWebShell gristle to deal with nsIWebShellContainer
that's about all

Comment 3

[Adam Lock]

Attachment: Patch

This patch brutally chops out 200+ lines of dead webshell crap, fixes a few
oddities in docshell caused by removal of nsWebShell overides (e.g.
Get/SetDimensions) and makes nsDocShell::SetCurrentURI a method on nsIDocShell
to replace nsIWebShell::SetURL which has been removed.

Patch also modifies editor slightly to call nsIDocShell::SetCurrentURI instead
of SetURL and ensures viewer.exe still work.

Now nsIWebShell is reduced to 3 methods - SetContainer, GetContainer and
GetDocumentLoader.

Reviews please?

Comment 4

[Judson Valeski]

sweet! all those methods just go away (no callsites in mozilla or commercial?)?
If so, r=valeski.

Comment 5

[Adam Lock]

I'll build commercial to make sure but there doesn't appear to be anywhere it
calls webshell methods.

Rick do you want to sr this one?

Comment 6

[Adam Lock]

Verifying that commercial build is happy with these changes.

Comment 7

[rpotts (gone)]

Comment on attachment 62388 [details] [diff] [review]
Patch 

ya!!  Adam, thanks for cleaning up all of this nasty cruft!!

Comment 8

[Adam Lock]

Fix is checked in

Comment 9

[Adam Lock]

Actually, I'll reopen this bug until I've had time to delete all the unused
files still festering in mozilla/webshell.

Comment 10

[Håkan Waara]

I'm just wondering about the Qt code you removed... what's 'dead' about it? Just
curious.

Comment 11

[Adam Lock]

mozilla/webshell/Makefile.in didn't even reference mozilla/webshell/embed so the
QTstuff has not been built on Unix for some time. Besides which the code is
prehistoric and wouldn't even work without being rewritten.

So webshell is reduced to a gnarly stump of public headers can be transplanted
to docshell at some point and the viewer test app. I wish viewer would die. I
wonder if anyone still needs it.

Marking this bug FIXED.

Comment 12

[David Baron :dbaron: 🇪🇸 ⌚UTC+1]

Layout people still need viewer.  To get rid of it we'd need some other app that
had the debugging capabilities that are in viewer's menus, and we'd need to
rewrite the layout regression test harness to use some other app.  That's
certainly doable, but it's a bit of work and nobody's signed up to do it.

Comment 13

[Adam Lock]

Raised bug 122829 to move viewer into layout.