ssia.
<rdar://problem/35110321>
<rdar://problem/35110347>
Created attachment 324501 [details] Patch
Comment on attachment 324501 [details] Patch Attachment 324501 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4946487 Number of test failures exceeded the failure limit.
Created attachment 324505 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 324507 [details] Patch
Comment on attachment 324507 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324507&action=review > Source/WebCore/ChangeLog:9 > + This patch turn m_subtreeLayoutRoot into a optional weak pointer. Optional, because It seems weird that we need an optional weak ptr. Is it necessary to differentiate between having a null pointer and not having a pointer at all (std::nullopt)?
Comment on attachment 324507 [details] Patch Attachment 324507 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4947417 New failing tests: svg/custom/delete-text-innerText-crash.html fast/forms/textarea-paste-newline.html svg/custom/bug79798.html editing/pasteboard/smart-paste-in-text-control.html svg/custom/delete-text-crash.html editing/execCommand/insert-list-xml.xhtml editing/pasteboard/copy-paste-pre-line-content.html editing/pasteboard/copy-paste-first-line-in-textarea.html
Created attachment 324514 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
(In reply to Daniel Bates from comment #7) > Comment on attachment 324507 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=324507&action=review > > > Source/WebCore/ChangeLog:9 > > + This patch turn m_subtreeLayoutRoot into a optional weak pointer. Optional, because > > It seems weird that we need an optional weak ptr. Is it necessary to > differentiate between having a null pointer and not having a pointer at all > (std::nullopt)? Yes. Not having a pointer at all means we are not planning to run a subtree layout, while having a null pointer means that we are planning to run a subtree layout but due to some tree mutation we have destroyed the renderer that initiated this layout.
(In reply to zalan from comment #10) > (In reply to Daniel Bates from comment #7) > > Comment on attachment 324507 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=324507&action=review > > > > > Source/WebCore/ChangeLog:9 > > > + This patch turn m_subtreeLayoutRoot into a optional weak pointer. Optional, because > > > > It seems weird that we need an optional weak ptr. Is it necessary to > > differentiate between having a null pointer and not having a pointer at all > > (std::nullopt)? > Yes. Not having a pointer at all means we are not planning to run a subtree > layout, while having a null pointer means that we are planning to run a > subtree layout but due to some tree mutation we have destroyed the renderer > that initiated this layout. Do we do something different for these cases when FrameView::layout() is called? From briefly looking at the now obsolete attachment #324507 [details] it seems that we always perform a full layout (unless we do not have a RenderView).
(In reply to Build Bot from comment #9) > Created attachment 324514 [details] > Archive of layout-test-results from ews114 for mac-elcapitan > > The attached test failures were seen while running run-webkit-tests on the > mac-debug-ews. > Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6 These failures actually are not regressions. The patch reveals a bug where we mark the RenderView dirty soon after we schedule a subtree layout.
(In reply to Daniel Bates from comment #11) > (In reply to zalan from comment #10) > > (In reply to Daniel Bates from comment #7) > > > Comment on attachment 324507 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=324507&action=review > > > > > > > Source/WebCore/ChangeLog:9 > > > > + This patch turn m_subtreeLayoutRoot into a optional weak pointer. Optional, because > > > > > > It seems weird that we need an optional weak ptr. Is it necessary to > > > differentiate between having a null pointer and not having a pointer at all > > > (std::nullopt)? > > Yes. Not having a pointer at all means we are not planning to run a subtree > > layout, while having a null pointer means that we are planning to run a > > subtree layout but due to some tree mutation we have destroyed the renderer > > that initiated this layout. > > Do we do something different for these cases when FrameView::layout() is > called? From briefly looking at the now obsolete attachment #324507 [details] > [details] it seems that we always perform a full layout (unless we do not > have a RenderView). ASSERT on the second case and skip the layout. (clearly missing from this patch)
Created attachment 324518 [details] Patch
Attachment 324518 [details] did not pass style-queue: ERROR: Source/WebCore/page/FrameView.cpp:1399: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to zalan from comment #14) > Created attachment 324518 [details] > Patch This is just for EWSing -needs to be split up into 2 separate patches.
Comment on attachment 324518 [details] Patch Attachment 324518 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4949265 New failing tests: fast/dynamic/remove-invisible-node-inside-selection.html
Created attachment 324520 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 324518 [details] Patch Attachment 324518 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4949270 New failing tests: fast/dynamic/remove-invisible-node-inside-selection.html
Created attachment 324521 [details] Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 324533 [details] Patch
(In reply to zalan from comment #12) > (In reply to Build Bot from comment #9) > > Created attachment 324514 [details] > > Archive of layout-test-results from ews114 for mac-elcapitan > > > > The attached test failures were seen while running run-webkit-tests on the > > mac-debug-ews. > > Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6 > > These failures actually are not regressions. The patch reveals a bug where > we mark the RenderView dirty soon after we schedule a subtree layout. See bug 178651
Created attachment 324540 [details] Patch
Created attachment 324541 [details] Patch
Created attachment 324579 [details] Patch
Comment on attachment 324579 [details] Patch Clearing flags on attachment: 324579 Committed r223853: <https://trac.webkit.org/changeset/223853>
All reviewed patches have been landed. Closing bug.