Friday, 12 February 2016

Review: Codethink sponsored work on NetSurf, LibCSS and LibDOM

Review: Codethink sponsored work on NetSurf, LibCSS and LibDOM
==============================================================

Over the last couple of weeks I have been lucky enough to be able
to do work on the NetSurf project at work.
At [Codethink](http://www.codethink.co.uk/) we have an arrangement
that allows engineers to fill time between projects by making
contributions to certain open source projects.

My request to work on [NetSurf](http://www.netsurf-browser.org/),
and more particularly, its [LibDOM](http://ns-b.org/projects/libdom/)
and [LibCSS](http://ns-b.org/projects/libcss/) sub-projects was
approved and I was able to get a good amount done.


LibDOM
------

Most of my time has been spent improving LibDOM; the Document Object
Model library. Since the recent moves towards adding JavaScript
support in NetSurf, various parts of LibDOM that had never been used
are beginning to get exercised. This has revealed various issues.

The first series of changes affected the specialisations of HTMLElement:

* Added a new, more efficient means of identifying an HTML element's
tag name.
* Fixed some missing HTML element specialisations.
* Made some optimisations relating to how the appropriate specialisation
for an element is determined.
* Made the internal workings of HTMLElement specialisation more consistent.
* Exposed a function to enabled client applications to use the new
efficient means of identifying HTML element tag types.

6d60021 Split out tag names from general string table and enum.
57 files changed, 502 insertions(+), 324 deletions(-)
3894e33 Split detection of HTML TAG type out into helper.
1 file changed, 325 insertions(+), 138 deletions(-)
1f07c35 Enable table row HTML element specialisation.
1 file changed, 4 insertions(+)
dd2e075 Add missing specialisation of BLOCKQUOTE.
1 file changed, 4 insertions(+)
8d09ec0 Add missing detection of HTMLDivElement specialisation.
1 file changed, 8 insertions(+)
81c012f Add missing detection of HTMLMetaElement specialisation.
1 file changed, 7 insertions(+)
e346fef Fix up HTMLBaseElement specialisation.
2 files changed, 8 insertions(+), 12 deletions(-)
13df5af Add missing detection of HTMLBaseElement specialisation.
1 file changed, 8 insertions(+)
0174d8b Remove default from HTML element specialisation switch.
1 file changed, 1 deletion(-)
12385d1 Optimise element specialisation: use non-caseless
comparison in ladder.
1 file changed, 63 insertions(+), 63 deletions(-)
13f0636 Optimise HTMLElement specialisation slightly.
1 file changed, 222 insertions(+), 189 deletions(-)
5b6c191 Fix longstanding failure to handle HTMLDirectoryElement
specialisation.
3 files changed, 6 insertions(+), 9 deletions(-)
9def0a4 Merge branch 'tlsa/html-element-type'
4bd00fd Fix: Pass tag_name through to HTMLQuoteElement initialiser.
3 files changed, 9 insertions(+), 8 deletions(-)
809a7dc Merge branch 'tlsa/html-element-type'
bab3f35 Simplified consistant interface to HTMLElement creation.
107 files changed, 894 insertions(+), 972 deletions(-)
5007c80 Store HTMLElement tag types as enum on the html elements.
6 files changed, 15 insertions(+), 7 deletions(-)
fa21110 Style: Fix indentation and tidy up.
1 file changed, 78 insertions(+), 66 deletions(-)
cedfbbc Add function to get html element's tag type.
4 files changed, 41 insertions(+), 1 deletion(-)
e137aa0 Merge branch 'tlsa/html-element-type'

Various profiling sessions indicated that the lower-casing of HTML
attributes was taking too long, so that was optimised.

a37713d Optimise dom_string_(toupper|tolower) functions.
1 file changed, 9 insertions(+), 31 deletions(-)

Another area we were spending too much time was in event dispatch.
Previously we would gather a list of nodes from the event target node
all the way back to the root node. Now we generate a list containing
nodes with registered listeners. In most cases, when there is no
scripting involved we can now avoid generating the list at all.

b4a245f Don't add target event to list of capture/bubbling event
targets.
1 file changed, 2 insertions(+), 4 deletions(-)
7e9681b Remove parameter documentation for non-existant parameter.
1 file changed, 1 deletion(-)
2196dd1 Optimise event dispatch.
1 file changed, 57 insertions(+), 20 deletions(-)
737cb99 Further optimise event dispatch.
1 file changed, 20 insertions(+), 5 deletions(-)

More changes to HTMLElement handling, mostly to extend the list of
known tags to include those that are not specialised in the DOM spec.

acedecb Sort HTML elements to make it easier to see what's there.
1 file changed, 44 insertions(+), 44 deletions(-)
c12b1ea Add full list of HTML5 elements to enum.
2 files changed, 363 insertions(+), 202 deletions(-)
d9c4dec Merge branch 'tlsa/html-element-type'
9ef22b6 Add CENTER to list of HTML elements.
2 files changed, 4 insertions(+)
e9257f8 Merge branch 'tlsa/html-element-type'

Various issues were identified with the HTMLElement class and
its specialisations' copy constructors. These were fixed in
the following series.

557678e Style: Fix sporadic use of spaces for indentation in
element copy constructor.
1 file changed, 5 insertions(+), 5 deletions(-)
b38b017 Split out element content copying from allocation in copy
constructor.
2 files changed, 41 insertions(+), 25 deletions(-)
89f73a0 Fix: Implement HTMLElement copy constructor.
2 files changed, 37 insertions(+), 1 deletion(-)
1ad8e7c Fix: Handle class list allocation failure in HTMLElement
copy constructor.
1 file changed, 13 insertions(+), 5 deletions(-)
e3dd4b2 Fix: Proper copy constructor for HTMLAnchorElement.
2 files changed, 41 insertions(+), 3 deletions(-)
49c0d92 Fix: Proper copy constructor for HTMLAppletElement.
2 files changed, 41 insertions(+), 3 deletions(-)
28f754a Fix: Proper copy constructor for HTMLAreaElement.
2 files changed, 41 insertions(+), 3 deletions(-)
d6017f1 Fix: Proper copy constructor for HTMLBaseElement.
2 files changed, 41 insertions(+), 3 deletions(-)
3542d8d Fix: Proper copy constructor for HTMLBasefontElement.
2 files changed, 41 insertions(+), 3 deletions(-)
1eb6cc5 Fix: Proper copy constructor for HTMLBodyElement.
2 files changed, 41 insertions(+), 3 deletions(-)
6c8d440 Fix: Proper copy constructor for HTMLBRElement.
2 files changed, 41 insertions(+), 3 deletions(-)
42bcd5d Provide generic copy constructor for HTMLButtonElement.
2 files changed, 41 insertions(+), 3 deletions(-)
bcc4b3e Fix: HTMLButtonElement's copy constructor copies its
specialised members.
1 file changed, 4 insertions(+)
43930a9 Fix: Proper copy constructor for HTMLDirectoryElement.
2 files changed, 41 insertions(+), 3 deletions(-)
9942e99 Fix: Proper copy constructor for HTMLDivElement.
2 files changed, 41 insertions(+), 3 deletions(-)
80873f9 Fix: Proper copy constructor for HTMLDListElement.
2 files changed, 41 insertions(+), 3 deletions(-)
b5c1e7c Fix: Proper copy constructor for HTMLFieldsetElement.
2 files changed, 41 insertions(+), 3 deletions(-)
663650a Fix: Proper copy constructor for HTMLFontElement.
2 files changed, 41 insertions(+), 3 deletions(-)
eef199e Fix: Proper copy constructor for HTMLFormElement.
2 files changed, 41 insertions(+), 3 deletions(-)
bd8ff18 Fix: Proper copy constructor for HTMLFrameElement.
2 files changed, 41 insertions(+), 3 deletions(-)
f4bec4a Fix: Proper copy constructor for HTMLFramesetElement.
2 files changed, 41 insertions(+), 3 deletions(-)
098842b Fix: Proper copy constructor for HTMLHeadElement.
2 files changed, 41 insertions(+), 3 deletions(-)
7eb3726 Fix: Proper copy constructor for HTMLHeadingElement.
2 files changed, 41 insertions(+), 3 deletions(-)
bdff6f9 Fix: Proper copy constructor for HTMLHRElement.
2 files changed, 41 insertions(+), 3 deletions(-)
840478f Fix: Proper copy constructor for HTMLHTMLElement.
2 files changed, 41 insertions(+), 3 deletions(-)
66a5830 Fix: Proper copy constructor for HTMLIframeElement.
2 files changed, 41 insertions(+), 3 deletions(-)
d3dd126 Fix: Proper copy constructor for HTMLImageElement.
2 files changed, 41 insertions(+), 3 deletions(-)
9f795a6 Provide generic copy constructor for HTMLInputElement.
2 files changed, 41 insertions(+), 3 deletions(-)
ec5f036 Fix: HTMLInputElement's copy constructor copies its
specialised members.
1 file changed, 12 insertions(+)
df4df17 Fix: Proper copy constructor for HTMLIsindexElement.
2 files changed, 41 insertions(+), 3 deletions(-)
07b7dd8 Fix: Proper copy constructor for HTMLLabelElement.
2 files changed, 41 insertions(+), 3 deletions(-)
87d9b3f Fix: Proper copy constructor for HTMLLegendElement.
2 files changed, 41 insertions(+), 3 deletions(-)
139250e Fix: Proper copy constructor for HTMLLiElement.
2 files changed, 41 insertions(+), 3 deletions(-)
9863b47 Fix: Proper copy constructor for HTMLLinkElement.
2 files changed, 41 insertions(+), 3 deletions(-)
c027180 Fix: Proper copy constructor for HTMLMapElement.
2 files changed, 41 insertions(+), 3 deletions(-)
13c7b9e Cleanup: Remove odd declaration of callback outside header
guard.
2 files changed, 1 insertion(+), 3 deletions(-)
0238cec Fix: Proper copy constructor for HTMLMenuElement.
2 files changed, 41 insertions(+), 3 deletions(-)
a730440 Fix: Proper copy constructor for HTMLMetaElement.
2 files changed, 41 insertions(+), 3 deletions(-)
5ab7524 Fix: Proper copy constructor for HTMLModElement.
2 files changed, 41 insertions(+), 3 deletions(-)
f97e745 Fix: Proper copy constructor for HTMLObjectElement.
2 files changed, 41 insertions(+), 3 deletions(-)
84f9b4e Fix: Proper copy constructor for HTMLOListElement.
2 files changed, 41 insertions(+), 3 deletions(-)
64833e7 Fix: Proper copy constructor for HTMLOptGroupElement.
2 files changed, 41 insertions(+), 3 deletions(-)
89171f6 Provide generic copy constructor for HTMLOptionElement.
2 files changed, 41 insertions(+), 3 deletions(-)
37eb480 Fix: HTMLOptionElement's copy constructor copies its
specialised members.
1 file changed, 3 insertions(+)
842deef Fix: Proper copy constructor for HTMLParagraphElement.
2 files changed, 41 insertions(+), 3 deletions(-)
37692b4 Fix: Proper copy constructor for HTMLParamElement.
2 files changed, 41 insertions(+), 3 deletions(-)
c4b0e2a Fix: Proper copy constructor for HTMLPreElement.
2 files changed, 41 insertions(+), 3 deletions(-)
c4159ca Fix: Proper copy constructor for HTMLQuoteElement.
2 files changed, 41 insertions(+), 3 deletions(-)
d82f237 Fix: Proper copy constructor for HTMLScriptElement.
2 files changed, 41 insertions(+), 3 deletions(-)
cff37d7 Provide generic copy constructor for HTMLSelectElement.
2 files changed, 41 insertions(+), 3 deletions(-)
3c056b4 Fix: HTMLSelectElement's copy constructor copies its
specialised members.
1 file changed, 6 insertions(+)
2dda1dd Fix: Proper copy constructor for HTMLStyleElement.
2 files changed, 41 insertions(+), 3 deletions(-)
609e19e Fix: Proper copy constructor for HTMLTableElement.
2 files changed, 41 insertions(+), 3 deletions(-)
7782715 Cleanup: Remove odd declaration of callbacks outside header
guard.
2 files changed, 2 insertions(+), 4 deletions(-)
103c582 Cleanup: Don't put static function in header.
2 files changed, 2 insertions(+), 5 deletions(-)
349992c Fix: Proper copy constructor for HTMLTableCaptionElement.
2 files changed, 41 insertions(+), 3 deletions(-)
7d0c256 Fix: Proper copy constructor for HTMLTableCellElement.
2 files changed, 41 insertions(+), 3 deletions(-)
41fd1fe Fix: Proper copy constructor for HTMLTableColElement.
2 files changed, 41 insertions(+), 3 deletions(-)
e13599e Fix: Proper copy constructor for HTMLTableRowElement.
2 files changed, 41 insertions(+), 3 deletions(-)
f566b7c Cleanup: Remove odd declaration of callback outside header
guard.
2 files changed, 1 insertion(+), 2 deletions(-)
afec576 Fix: Proper copy constructor for HTMLTableSectionElement.
2 files changed, 41 insertions(+), 3 deletions(-)
848c41d Cleanup: Remove odd declaration of callback outside header
guard.
2 files changed, 1 insertion(+), 2 deletions(-)
b26b3cf Provide generic copy constructor for HTMLTextAreaElement.
2 files changed, 41 insertions(+), 3 deletions(-)
0480b0a Fix: HTMLTextAreaElement's copy constructor copies its
specialised members.
1 file changed, 9 insertions(+)
0f69427 Fix: Proper copy constructor for HTMLTitleElement.
2 files changed, 41 insertions(+), 3 deletions(-)
d19eda0 Fix: Proper copy constructor for HTMLUListElement.
2 files changed, 41 insertions(+), 3 deletions(-)
68c4463 Merge branch 'tlsa/fix-html-element-copy-constructors'

Finally a Coverity static analysis issue was fixed.

9f2c89e Fix Coverity #1350096.
1 file changed, 1 insertion(+), 1 deletion(-)

Overall, the there were 99 commits to LibDOM over the course of this stint.


LibCSS
------

Most of the work done on LibCSS was cherry-picking a commit to change
presentational hint handling to be more efficient from another of my
branches, and getting it merged. This was dependent on getting NetSurf
ready to handle the change.

This change to presentational hint handling has made the selection
process much faster for old-school sites that use HTML attributes
for styling content, rather than CSS. It also speeds up the handling
of modern CSS-using sites too, as there is now no overhead when there
are no hints present in the document.

d4b2c5c Merge branch 'tlsa/upfront-hints'
9024333 Documentation: Presentational hint API change.
1 file changed, 19 insertions(+)
df3d8e8 Documentation: Fix typos.
1 file changed, 3 insertions(+), 3 deletions(-)


NetSurf
-------

Much of the work done on NetSurf was in response to the changes made
in LibCSS and LibDOM, but I also spent time on bug fixes, cleanups,
and pre-release testing.

The first set of changes were mostly related to JavaScript creation
of HTMLElements, and making use of the new LibDOM API for determining
an element's tag type.

b58176c Spaces to tabs.
1 file changed, 1 insertion(+), 1 deletion(-)
b002ba7 Simplify special element handling in node inserted callback.
1 file changed, 31 insertions(+), 31 deletions(-)
6904622 JavaScript: Fix & optimise HTMLElement specialisation proto
str generation.
1 file changed, 199 insertions(+), 9 deletions(-)
2443cc4 Attempt to squash warning that happens on CI.
1 file changed, 3 insertions(+), 3 deletions(-)
b1dbc04 Add paragraph proto test.
1 file changed, 23 insertions(+)
9fb755b Handle known HTML elements without specialisations.
1 file changed, 7 insertions(+), 1 deletion(-)
3cc80b6 Add test for document.write() adding a STYLE element.
1 file changed, 15 insertions(+)

Next was a complete rewrite of CSS presentational hints handling.
Presentational hints are mostly legacy HTML element attributes which
affect the presentation of the elements. They affect the CSS computed
style when no other styles have affected it. Using the new LibCSS API
coupled with the new LibDOM element tag type feature has made support
for presentational hints much faster.

2784514 Rewrite hints handling to be more efficient.
2 files changed, 788 insertions(+), 1123 deletions(-)
eb67607 Merge branch 'tlsa/upfront-hints'

I noticed and investigated an apparent memory leak. Some changes were
made to improve the cache logging, which ruled the cache out as the
cause of the problem. The leak was tracked to a particular issue in
the GTK front end's text rendering.

63fb2cb Add URL to llcache object destruction logging.
1 file changed, 2 insertions(+), 1 deletion(-)
fc2d766 Style: Wrap some long lines.
1 file changed, 31 insertions(+), 18 deletions(-)
255361a Log the cache limit along side size, after clean.
1 file changed, 1 insertion(+), 1 deletion(-)
9cb522c Don't haemorrhage Pango layouts out of nsfont_paint.
1 file changed, 1 insertion(+)

A few tidy-ups to GTK font rendering and core CSS hint handling:

df2d62a Use readonly get_line, since it is supposed to be faster.
1 file changed, 1 insertion(+), 1 deletion(-)
f5be1eb Add switch case fallthrough comments.
1 file changed, 5 insertions(+)
5108424 Add switch case fallthrough comment.
1 file changed, 1 insertion(+)
3af77ea Can free pango font desc as soon as it's set.
1 file changed, 12 insertions(+), 10 deletions(-)

Finally I fixed several bugs on the bug tracker.

The first was an assertion (crash) that could occur during HTML
layout. The issue stemmed from NetSurf's allowing :before and
:after pseudo elements to be generated for replaced elements.
This behaviour was unspecified in CSS 2.1, but in CSS 3 it is
not permitted.

Second was to remove some code which was complicating the status
bar updating for HTML contents.

Third was to fix the object accounting that is done to keep track
of fetches used for additional resources such as images when
rendering an HTML content. This accounting went wrong when a
fetch for an HTML content was stopped, preventing the content
ever reaching the DONE state, and causing misleading status
bar messages.

3bee7b7 Layout: Don't generate :before and :after boxes for
replaced elements.
2 files changed, 17 insertions(+), 6 deletions(-)
d4a01d5 HTML: Remove some status bar updating code.
4 files changed, 43 deletions(-)
04e61b7 Fix object accounting for aborted HTML contents.
1 file changed, 4 insertions(+), 3 deletions(-)

I also did quite a bit of testing, particularly of the JavaScript
related code, to ensure that there is nothing major blocking the
forthcoming NetSurf 3.4 release.

Overall, there were 20 commits to NetSurf through the course of this stint.


Overview and thanks
-------------------

I was able to get many hours of work done on the NetSurf project over
this period. It has been a very nice experience, working on this
project at work. Thanks to the work environment, I found myself more
focused than I usually am when I'm working on it in my spare time.

Many thanks to [Codethink](http://www.codethink.co.uk/) for providing
this opportunity. I'm very grateful and I've really enjoyed it.

The stats and logs presented were gathered using the following commands
in the NetSurf, LibDOM and LibCSS repositories.

$ git shortlog -n -s -e | grep "michael.drake@codethink.co.uk"
$ git log --author="Michael Drake <michael.drake@codethink.co.uk>"
--oneline --shortstat --reverse


--

Michael Drake http://www.codethink.co.uk/

No comments:

Post a Comment