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..
Created attachment 120139 [details] Proposed patch A first stab at this.
Created attachment 120140 [details] Proposed patch
Comment on attachment 120140 [details] Proposed patch Looks good.
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?
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.)
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.
Comment on attachment 120214 [details] Proposed patch v2 r=me
Comment on attachment 120214 [details] Proposed patch v2 Clearing flags on attachment: 120214 Committed r103473: <http://trac.webkit.org/changeset/103473>
All reviewed patches have been landed. Closing bug.
Reverted r103473 for reason: Overestimated my superpowers. Committed r103478: <http://trac.webkit.org/changeset/103478>
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.
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.
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
Committed r103638: <http://trac.webkit.org/changeset/103638>