Johnny Stenback (:jst) Assignee
	Description • 23 years ago

There's quite a few HTML element classes in the mozilla code that's not used at
all when loading jrgm's pageload tests, this involves loading 40 top 100 pages,
and should give us a reasonable (but of course not perfect) picture of what code
is commonly used and what's not. When running these tests we don't use any of
these elements:

nsHTMLAppletElement
nsHTMLButtonElement
nsHTMLDelElement
nsHTMLDirectoryElement
nsHTMLEmbedElement
nsHTMLFieldsetElement
nsHTMLInsElement
nsHTMLLabelElement
nsHTMLLegendElement
nsHTMLMenuElement
nsHTMLModElement
nsHTMLObjectElement
nsHTMLOlistElement
nsHTMLOptGroupElement
nsHTMLParamElement
nsHTMLQuoteElement
nsHTMLTableCaptionElement
nsHTMLTableColGroupElement
nsHTMLTableRowElement
nsHTMLTablecolElement
nsHTMLTextareaElement
nsHTMLWbrElement

I've started working on collapsing these unused elements into a few shared
classes that when used will work exactly like the current code at the expence of
a few more vtables on the rarely used element objects.

Patch coming up that collapses nsHTMLTableColGroupElement.cpp and
nsHTMLTableColElement.cpp into nsHTMLTableColElement.cpp, and also
nsTHMLBaseElement.cpp, nsHTMLEmbedElement.cpp, nsHTMLIsIndexElement.cpp,
nsHTMLParamElement.cpp, nsHTMLSpacerElement.cpp and nsHTMLWBRElement.cpp into
nsHTMLSharedLeafElement.cpp. The result of this first step is that we can remove
6 files from the project.

	Chris Waterson
	Comment 4 • 23 years ago

sr=waterson! Go Johnny go!

	Johnny Stenback (:jst) Assignee
	Comment 5 • 23 years ago

Dbaron, would you review this (second and last patch)?

	David Baron :dbaron:
	Comment 6 • 23 years ago

Comment on attachment 55649 [details] [diff] [review]
Same as above, excluding deleted files.

>Index: content/html/content/src/nsHTMLInputElement.cpp
>+  nsresult rv = ((nsGenericHTMLElement *)it)->Init(aNodeInfo);
>+  nsresult rv = ((nsGenericHTMLElement *)it)->Init(mNodeInfo);

Could you use NS_STATIC_CAST, like you did in nsHTMLSelectElement.cpp?

r=dbaron (on the last 2 attachments)

	Johnny Stenback (:jst) Assignee
	Comment 7 • 23 years ago

Yup, will do. Thanks for the review.

	Johnny Stenback (:jst) Assignee
	Comment 8 • 23 years ago

Above patches checked in, more to come...

	Peter Lubczynski
	Comment 9 • 23 years ago

Is it possible the check-in to this bug is causing the blocker bug 107627?

	Johnny Stenback (:jst) Assignee
	Comment 10 • 23 years ago

Yes, this fix caused bug 107627 (which is now fixed).

	Fabian Guisset
	Comment 11 • 23 years ago

Fixed, or do we want to keep it open for future similar improvements?

	Johnny Stenback (:jst) Assignee
	Comment 12 • 23 years ago

There's more to do here...

	Johnny Stenback (:jst) Assignee
	Comment 13 • 23 years ago

Pushing to mozilla1.0

	Christopher Aillon (sabbatical, not receiving bugmail)
	Comment 15 • 22 years ago

Comment on attachment 89166 [details] [diff] [review]
Combine rarely used container classes...

This patch looks fine, and comparing the methods in the old classes to the new
combined class looks correct as well.

r=caillon with the additional caveat of removing (or actually using) the XXX
comment here:
http://lxr.mozilla.org/mozilla/source/content/html/content/src/nsHTMLSharedCont
ainerElem.cpp#344

	Heikki Toivonen (remove -bugzilla when emailing directly)
	Comment 16 • 22 years ago

Comment on attachment 89166 [details] [diff] [review]
Combine rarely used container classes...

>Index: content/html/content/src/Makefile.in
>===================================================================
>-		nsHTMLDelElement.cpp \
>-		nsHTMLDirectoryElement.cpp \
>-		nsHTMLInsElement.cpp \
>-		nsHTMLMenuElement.cpp \
>-		nsHTMLModElement.cpp \
>-		nsHTMLQuoteElement.cpp \
>+		nsHTMLSharedContainerElem.cpp \
>-		nsHTMLTableCaptionElement.cpp \
>Index: content/html/content/src/makefile.win
>===================================================================
>-	.\$(OBJDIR)\nsHTMLDelElement.obj		\
>-	.\$(OBJDIR)\nsHTMLDirectoryElement.obj		\
>-	.\$(OBJDIR)\nsHTMLInsElement.obj		\
>-	.\$(OBJDIR)\nsHTMLMenuElement.obj		\
>-	.\$(OBJDIR)\nsHTMLModElement.obj		\
>-	.\$(OBJDIR)\nsHTMLOListElement.obj		\
>-	.\$(OBJDIR)\nsHTMLQuoteElement.obj		\
>+	.\$(OBJDIR)\nsHTMLSharedContainerElem.obj	\
>-	.\$(OBJDIR)\nsHTMLTableCaptionElement.obj	\

These two lists above don't match (also check Mac project)???

>Index: content/html/content/src/nsGenericHTMLElement.h
>===================================================================
>+inline nsresult
>+NS_NewHTMLOListElement(nsIHTMLContent** aResult, nsINodeInfo *aNodeInfo)
>+{
>+  return NS_NewHTMLSharedContainerElement(aResult, aNodeInfo);
>+}

Is OL really a rarely used element? I wouldn't consider it rare, but...

>Index: content/html/content/src/nsHTMLDelElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLDirectoryElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLInsElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLMenuElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLModElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLOListElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLQuoteElement.cpp
>===================================================================
>Index: content/html/content/src/nsHTMLTableCaptionElement.cpp
>===================================================================
>Index: content/macbuild/content.xml
>===================================================================
>Index: content/shared/public/nsHTMLAtomList.h
>===================================================================

	Johnny Stenback (:jst) Assignee
	Comment 17 • 22 years ago

Mass-reassigning bugs.

	David Baron :dbaron:
	Comment 19 • 20 years ago

Comment on attachment 167388 [details] [diff] [review]
fix regression from col/colgroup combining (checked in to trunk, 2004-11-29 21:52 -0800).

This fixes a regression (perhaps asymptomatic) from attachment 55647 [details] [diff] [review].

	Boris Zbarsky [:bzbarsky]
	Comment 20 • 20 years ago

Comment on attachment 167388 [details] [diff] [review]
fix regression from col/colgroup combining (checked in to trunk, 2004-11-29 21:52 -0800).

r+sr=bzbarsky

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 21 • 14 years ago

File followups if we ever want to do more of this.