Bug 25617 - Fix memory/performance regression because of too much form control related abstraction just for WMLs sake
Summary: Fix memory/performance regression because of too much form control related ab...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
: 23810 (view as bug list)
Depends on:
Blocks: 20393
  Show dependency treegraph
 
Reported: 2009-05-07 09:10 PDT by Nikolas Zimmermann
Modified: 2009-05-08 12:13 PDT (History)
2 users (show)

See Also:


Attachments
Initial patch (76.94 KB, patch)
2009-05-07 09:26 PDT, Nikolas Zimmermann
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2009-05-07 09:10:08 PDT
As the summary says, the new abstraction layer(s): FormControlElement / FormControlElementWithState / InputElement etc. has caused ambiguity for several HTML*Element classes.

For instance HTMLInputElement now inherits from HTMLFormControlElementWithState and InputElement. HTMLFormControlElementWithState inherits from HTMLFormControlElement and FormControlElementWithState. In turn HTMLFormControlElement inherits from HTMLElement and FormControlElement. As you can see this is rather ambigious.

All methods now living in those abstract classes (FormControlElement etc.) previously mostly lived in Node and related classes. The idea to resolve the memory regressions while keeping a rather sane API is to group these methods as virtual functions in Element.

While one can argue (like I did, when I introduced this abstraction layer) this is not the nicest way to design it, it's for sure less memory-intensive to add some extra virtual functions then a whole set of new base classes (4 byte per extra inherited class!).

The first thing to fix is to remove the FormControlElement/FormControlElementWithState base classes completely and group their functionality in Element in a seperated section "// FormControlElement API". The methods should be renamed to avoid confusion, unlike the previous version before my refactorization where functions like isEnabled() lived in Node - which is completely weird.
For instance isEnabled() should be renamed to isEnabledFormControl(), or isReadOnly() -> isReadOnlyFormControl(). etc.

About to attach a patch.
NOTE: This bug is a blocker for all further WML work. Memory regressions should all be resolved first. WML doesn't want to hurt HTML in any way :-)
Comment 1 Nikolas Zimmermann 2009-05-07 09:26:07 PDT
Created attachment 30101 [details]
Initial patch

No regressions, ran all tests.
Comment 2 Dave Hyatt 2009-05-07 14:36:41 PDT
Comment on attachment 30101 [details]
Initial patch

Looks good. r=me.
Comment 3 Nikolas Zimmermann 2009-05-07 14:44:16 PDT
Landed in r43367.
Comment 4 Nikolas Zimmermann 2009-05-08 12:13:47 PDT
*** Bug 23810 has been marked as a duplicate of this bug. ***