Bug 129671

Summary: inSynchronizationOfLazyAttribute is making setAttributeInternal incomprehensible
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED WONTFIX    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, esprehn+autocc, kangil.han, kling, koivisto
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 129550    
Bug Blocks:    
Attachments:
Description Flags
Cleanup none

Ryosuke Niwa
Reported 2014-03-04 01:37:23 PST
inSynchronizationOfLazyAttribute is evaluated 4 times in various conditions even though all can just call ensureUniqueElementData().attributeAt(index).setValue(newValue) and exit early when it's true. Just do that so that the rest of code is more readable.
Attachments
Cleanup (2.50 KB, patch)
2014-03-04 02:15 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2014-03-04 02:15:19 PST
Created attachment 225764 [details] Cleanup
Andreas Kling
Comment 2 2014-03-04 02:21:08 PST
Comment on attachment 225764 [details] Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=225764&action=review > Source/WebCore/dom/Element.cpp:1032 > + if (UNLIKELY(inSynchronizationOfLazyAttribute)) { > + ensureUniqueElementData().attributeAt(index).setValue(newValue); > + return; > + } I think you have a bug here. If there is an Attr node, you will no longer reset its text children.
Ryosuke Niwa
Comment 3 2014-03-04 02:32:07 PST
Comment on attachment 225764 [details] Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=225764&action=review >> Source/WebCore/dom/Element.cpp:1032 >> + } > > I think you have a bug here. If there is an Attr node, you will no longer reset its text children. It comes from http://trac.webkit.org/changeset/139100. I can add a comment if you'd like.
Ryosuke Niwa
Comment 4 2016-04-29 20:25:00 PDT
Comment on attachment 225764 [details] Cleanup Too old.
Note You need to log in before you can comment on or make changes to this bug.