Bug 227745

Summary:

Use setBooleanAttribute instead of setAttributeWithoutSynchronization(X, Y ? emptyAtom() : nullAtom());

Product:

WebKit

Reporter:

Tim Nguyen (:ntim) <ntim>

Component:

DOM

Assignee:

Tim Nguyen (:ntim) <ntim>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, darin, esprehn+autocc, ews-watchlist, gyuyoung.kim, koivisto, mifenton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description Tim Nguyen (:ntim) 2021-07-07 07:54:16 PDT

https://webkit-search.igalia.com/webkit/search?q=emptyAtom%28%29+%3A+nullAtom%28%29%29&path=&case=false&regexp=false

In these 2 cases, attribute sync doesn't really add value here. SetBooleanAttribute() is easier to read.

Comment 1 Radar WebKit Bug Importer 2021-07-07 07:54:28 PDT

<rdar://problem/80269282>

Comment 2 Radar WebKit Bug Importer 2021-07-07 07:56:30 PDT

<rdar://problem/80269369>

Comment 3 Radar WebKit Bug Importer 2021-07-07 07:56:49 PDT

<rdar://problem/80269378>

Comment 4 Tim Nguyen (:ntim) 2021-07-07 08:19:33 PDT

Created attachment 433030 [details]
Patch

Comment 5 EWS 2021-07-07 09:14:24 PDT

Committed r279643 (239456@main): <https://commits.webkit.org/239456@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433030 [details].

Comment 6 Darin Adler 2021-07-07 12:42:44 PDT

Comment on attachment 433030 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        It's easier to read and attribute sync doesn't make a difference here (it only does for style & SVG attributes).

This comment shows a slight misunderstanding of "without synchronization". The function does not exist so that we can bypass synchronization. It’s an optimized case that saves work when we know the attribute we are setting is not something that can have SVG animation applied to it, nor is the attribute "style". By calling the plain old setAttribute, we are just forgoing a micro-optimization.

So a more accurate comment would be more like, "we can do without the slight optimization that we get by calling setAttributeWithoutSynchronization".