RESOLVED FIXED179157
Assert that updateStyle and updateLayout are only called when it's safe to dispatch events 
https://bugs.webkit.org/show_bug.cgi?id=179157
Summary Assert that updateStyle and updateLayout are only called when it's safe to di...
Ryosuke Niwa
Reported 2017-11-01 23:34:10 PDT
Since updating the style or layout can currently execute scripts, we should assert that these functions are only called when it's safe to execute scripts.
Attachments
Fixes the bug (1.52 KB, patch)
2017-11-01 23:54 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Adds the assertions (9.15 KB, patch)
2017-11-02 00:27 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews116 for mac-elcapitan (1.99 MB, application/zip)
2017-11-02 01:35 PDT, Build Bot 		no flags
Details
Fixed builds & tests (13.29 KB, patch)
2017-11-02 16:36 PDT, Ryosuke Niwa 		zalan: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2017-11-01 23:34:58 PDT
<rdar://problem/35144778>
Ryosuke Niwa
Comment 2 2017-11-01 23:54:30 PDT
Created attachment 325691 [details] Fixes the bug
Ryosuke Niwa
Comment 3 2017-11-01 23:55:36 PDT
Comment on attachment 325691 [details] Fixes the bug Wrong bug
Ryosuke Niwa
Comment 4 2017-11-02 00:27:11 PDT
Created attachment 325695 [details] Adds the assertions
Build Bot
Comment 5 2017-11-02 01:35:51 PDT
Comment on attachment 325695 [details] Adds the assertions Attachment 325695 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/5073448 New failing tests: fast/forms/textarea-set-defaultvalue-after-value.html svg/custom/check-intersection-basic.svg
Build Bot
Comment 6 2017-11-02 01:35:52 PDT
Created attachment 325699 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Antti Koivisto
Comment 7 2017-11-02 01:48:10 PDT
Comment on attachment 325695 [details] Adds the assertions View in context: https://bugs.webkit.org/attachment.cgi?id=325695&action=review > Source/WebCore/svg/SVGSVGElement.cpp:344 > +static bool checkIntersectionWithoutUpdatingLayout(RefPtr<SVGElement>&& element, SVGRect& rect) > +{ > + return element && RenderSVGModelObject::checkIntersection(element->renderer(), rect.propertyReference()); > +} > + > +static bool checkEnclosureWithoutUpdatingLayout(RefPtr<SVGElement>&& element, SVGRect& rect) > +{ > + return element && RenderSVGModelObject::checkEnclosure(element->renderer(), rect.propertyReference()); > +} Why rvalue references? These functions don't appear to be moving ownership. (I see existing code in SVGSVGElement::checkIntersection uses them too.)
Ryosuke Niwa
Comment 8 2017-11-02 16:36:13 PDT
Created attachment 325794 [details] Fixed builds & tests
Build Bot
Comment 9 2017-11-02 16:39:03 PDT
Attachment 325794 [details] did not pass style-queue: ERROR: Source/WebCore/dom/ContainerNode.cpp:152: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] Total errors found: 1 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Ryosuke Niwa
Comment 10 2017-11-02 20:48:13 PDT
Committed r224378: <https://trac.webkit.org/changeset/224378>
Note You need to log in before you can comment on or make changes to this bug.