Bug 74991 – Automate elements' registration as document namedItem/extraNamedItem.

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Andreas Kling]

Some of our HTML elements (applet, embed, form, iframe, image and object) track their "name" and "id" attributes in order to keep the "named item" and "extra named item" maps in HTMLDocument in sync.

We could do this automatically by hooking the relevant attribute changes and get rid of some duplicated code (plus those elements will shrink by two AtomicStrings each.)

Rough patch idea coming..

[Andreas Kling]

Created attachment 120139 [details]
Proposed patch

A first stab at this.

[Andreas Kling]

Created attachment 120140 [details]
Proposed patch

[Antti Koivisto]

Comment on attachment 120140 [details]
Proposed patch

Looks good.

[Antti Koivisto]

Comment on attachment 120140 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120140&action=review

r- for now for perf concern.

> Source/WebCore/html/HTMLElement.cpp:1005
> +void HTMLElement::insertedIntoDocument()
> +{
> +    StyledElement::insertedIntoDocument();
> +
> +    const AtomicString& name = fastGetAttribute(HTMLNames::nameAttr);
> +    if (!name.isNull())
> +        updateName(nullAtom, name);
> +}

Hmm, adding attribute lookup here might be slow. Is there a way to avoid this?

[Andreas Kling]

Created attachment 120214 [details]
Proposed patch v2

Perf concern addressed by using a Node bit to cache whether we have a name attribute or not (just like we already did for id attributes.)

[WebKit Review Bot]

Attachment 120214 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'CMakeLists.txt', u'ChangeLog', u'LayoutTes..." exit_code: 1

ERROR: FAILURES FOR <lucid, x86_64, release, cpu> in LayoutTests/platform/chromium/test_expectations.txt
ERROR: Line:192 Path does not exist. http/tests/workers/interrupt-database-sync-open-crash.html
LayoutTests/platform/chromium/test_expectations.txt:192:  Path does not exist. http/tests/workers/interrupt-database-sync-open-crash.html  [test/expectations] [2]
Total errors found: 1 in 62 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antti Koivisto]

Comment on attachment 120214 [details]
Proposed patch v2

r=me

[Andreas Kling]

Comment on attachment 120214 [details]
Proposed patch v2

Clearing flags on attachment: 120214

Committed r103473: <http://trac.webkit.org/changeset/103473>

[Andreas Kling]

All reviewed patches have been landed.  Closing bug.

[Andreas Kling]

Reverted r103473 for reason:

Overestimated my superpowers.

Committed r103478: <http://trac.webkit.org/changeset/103478>

[Andreas Kling]

Created attachment 120265 [details]
Proposed patch v3

Same patch with the setHasName() moved to StyledElement::attributeChanged() to ensure that the hasID flag always gets set.

[WebKit Review Bot]

Attachment 120265 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:30:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 120265 [details]
Proposed patch v3

Rejecting attachment 120265 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
file Source/WebCore/html/HTMLFormElement.cpp
patching file Source/WebCore/html/HTMLFormElement.h
patching file Source/WebCore/html/HTMLImageElement.cpp
patching file Source/WebCore/html/HTMLImageElement.h
patching file Source/WebCore/html/HTMLObjectElement.cpp
patching file Source/WebCore/html/HTMLObjectElement.h
patching file Source/WebCore/html/HTMLPlugInElement.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Antti Koivisto', u'--f..." exit_code: 1

Full output: http://queues.webkit.org/results/10955394

[Andreas Kling]

Committed r103638: <http://trac.webkit.org/changeset/103638>