RESOLVED FIXED 128797
setSelection should not synchronously trigger layout 
https://bugs.webkit.org/show_bug.cgi?id=128797
Summary setSelection should not synchronously trigger layout
Ryosuke Niwa
Reported 2014-02-13 22:17:23 PST
When there is a pending layout or style recalc, simply delay the work until the layout happens.
Attachments
Work in progress (15.04 KB, patch)
2014-02-13 22:21 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Removed Xcode change (still WIP) (11.19 KB, patch)
2014-02-13 22:26 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Removes the sync layout (10.04 KB, patch)
2014-02-14 00:06 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Removed the stale declaration of selectionUpdateTimerFired (9.61 KB, patch)
2014-02-14 00:10 PST, Ryosuke Niwa 		koivisto: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2014-02-13 22:21:25 PST
Created attachment 224160 [details] Work in progress
WebKit Commit Bot
Comment 2 2014-02-13 22:23:27 PST
Attachment 224160 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Ryosuke Niwa
Comment 3 2014-02-13 22:26:21 PST
Created attachment 224161 [details] Removed Xcode change (still WIP)
WebKit Commit Bot
Comment 4 2014-02-13 22:28:13 PST
Attachment 224161 [details] did not pass style-queue: ERROR: Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Ryosuke Niwa
Comment 5 2014-02-14 00:06:03 PST
Created attachment 224170 [details] Removes the sync layout
Ryosuke Niwa
Comment 6 2014-02-14 00:06:31 PST
I'm quite happy about the fact I've successfully avoid adding yet-another timer.
Ryosuke Niwa
Comment 7 2014-02-14 00:10:27 PST
Created attachment 224171 [details] Removed the stale declaration of selectionUpdateTimerFired
Antti Koivisto
Comment 8 2014-02-14 12:29:21 PST
Comment on attachment 224171 [details] Removed the stale declaration of selectionUpdateTimerFired View in context: https://bugs.webkit.org/attachment.cgi?id=224171&action=review > Source/WebCore/dom/Document.cpp:1786 > +bool Document::needsLayout() const The name is confusing, this checks ancestor documents too. Something like selfOrParentDocumentNeedsLayout() perhaps? > Source/WebCore/dom/Document.h:558 > + bool needsStyleRecalc() const { return m_optimizedStyleSheetUpdateTimer.isActive() || m_pendingStyleRecalcShouldForce || childNeedsStyleRecalc(); } Does this really need to be inlined? Why isn't hasPendingStyleRecalc() sufficient? These are confusing enough already, I wouldn't like to add more slightly different variants. Why doesn't this need to recurse to ancestors? It is used in the same places as the newly added needsLayout(). > Source/WebCore/editing/FrameSelection.cpp:1338 > + if (!m_frame) Frameless FrameSelection sounds weird.
Ryosuke Niwa
Comment 9 2014-02-14 14:22:26 PST
Committed r164133: <http://trac.webkit.org/changeset/164133>
Note You need to log in before you can comment on or make changes to this bug.