Let's flip the switch and test it on the EWS bots.
Created attachment 284955 [details] proposed patch.
Comment on attachment 284955 [details] proposed patch. Attachment 284955 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1781500 Number of test failures exceeded the failure limit.
Created attachment 284956 [details] Archive of layout-test-results from ews112 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 284955 [details] proposed patch. We're definitely still getting assertion failures in many tests. Will need to fix them all before we can re-enable the StringView life-cycle checks.
Looking forward to learning details about tests that trigger assertion failures. If there are too many then I guess we can leave this alone and come back to it later. I’m hoping there are just a few mistakes either in the usage of StringView or in the checking that affect many tests.
All crash logs that I spot checked asserted under ComplexTextController::collectComplexTextRuns(). An example of affected test is accessibility/content-editable-as-textarea.html.
Could you point me at one of those? Scanning the source code of ComplexTextController::collectComplexTextRuns, I can’t spot where StringView is used at all. Maybe I can try testing this myself at some point.
The StringView is used here: #1 0x11d15db82 in WTF::StringView::characters8() const at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/StringView.h:337 #2 0x11d92070c in WebCore::TextRun::characters8() const at OpenSource/Source/WebCore/platform/graphics/TextRun.h:82 #3 0x11d91021d in WebCore::ComplexTextController::collectComplexTextRuns() at OpenSource/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:327 #4 0x11d90f9e7 in WebCore::ComplexTextController::ComplexTextController(WebCore::FontCascade const&, WebCore::TextRun const&, bool, WTF::HashSet<WebCore::Font const*, WTF::PtrHash<WebCore::Font const*>, WTF::HashTraits<WebCore::Font const*> >*, bool) at OpenSource/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:144 #5 0x11d917074 in WebCore::ComplexTextController::ComplexTextController(WebCore::FontCascade const&, WebCore::TextRun const&, bool, WTF::HashSet<WebCore::Font const*, WTF::PtrHash<WebCore::Font const*>, WTF::HashTraits<WebCore::Font const*> >*, bool) at OpenSource/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp:132 #6 0x11ec775b8 in WebCore::FontCascade::adjustSelectionRectForComplexText(WebCore::TextRun const&, WebCore::LayoutRect&, int, int) const at OpenSource/Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm:532 #7 0x11ec443c1 in WebCore::FontCascade::adjustSelectionRectForText(WebCore::TextRun const&, WebCore::LayoutRect&, int, int) const at OpenSource/Source/WebCore/platform/graphics/FontCascade.cpp:506 #8 0x11fa55396 in WebCore::InlineTextBox::positionForOffset(int) const at OpenSource/Source/WebCore/rendering/InlineTextBox.cpp:1015 #9 0x1228c9089 in WebCore::RenderText::localCaretRect(WebCore::InlineBox*, int, WebCore::LayoutUnit*) at OpenSource/Source/WebCore/rendering/RenderText.cpp:451 #10 0x11d577e48 in WebCore::AXObjectCache::localCaretRectForCharacterOffset(WebCore::RenderObject*&, WebCore::CharacterOffset const&) at OpenSource/Source/WebCore/accessibility/AXObjectCache.cpp:2470 #11 0x11d578387 in WebCore::AXObjectCache::absoluteCaretBoundsForCharacterOffset(WebCore::CharacterOffset const&) at OpenSource/Source/WebCore/accessibility/AXObjectCache.cpp:2479 #12 0x11d18f7f5 in WebCore::AccessibilityRenderObject::boundsForRange(WTF::RefPtr<WebCore::Range>) const at OpenSource/Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2008 I think that it is invalidated here: #0 0x1130fa8d2 in WTF::StringView::invalidate(WTF::StringImpl const&) at OpenSource/Source/WTF/wtf/text/StringView.cpp:213 #1 0x1130cb8c1 in WTF::StringImpl::~StringImpl() at OpenSource/Source/WTF/wtf/text/StringImpl.cpp:110 #2 0x1130cbba5 in WTF::StringImpl::~StringImpl() at OpenSource/Source/WTF/wtf/text/StringImpl.cpp:107 #3 0x1130cbbc5 in WTF::StringImpl::destroy(WTF::StringImpl*) at OpenSource/Source/WTF/wtf/text/StringImpl.cpp:138 #4 0x11d0c3a61 in WTF::StringImpl::deref() at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/StringImpl.h:599 #5 0x11d0c39d1 in void WTF::derefIfNotNull<WTF::StringImpl>(WTF::StringImpl*) at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/PassRefPtr.h:40 #6 0x11d0c392a in WTF::RefPtr<WTF::StringImpl>::~RefPtr() at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/RefPtr.h:62 #7 0x11d0c3725 in WTF::RefPtr<WTF::StringImpl>::~RefPtr() at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/RefPtr.h:62 #8 0x11d0c3705 in WTF::String::~String() at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/WTFString.h:137 #9 0x11d0c36e5 in WTF::String::~String() at OpenSource/WebKitBuild/Debug/usr/local/include/wtf/text/WTFString.h:137 #10 0x11fa4270e in WebCore::InlineTextBox::constructTextRun(WebCore::RenderStyle const&, WTF::String*) const at OpenSource/Source/WebCore/rendering/InlineTextBox.cpp:1031 #11 0x11fa5536f in WebCore::InlineTextBox::positionForOffset(int) const at OpenSource/Source/WebCore/rendering/InlineTextBox.cpp:1014 #12 0x1228c9089 in WebCore::RenderText::localCaretRect(WebCore::InlineBox*, int, WebCore::LayoutUnit*) at OpenSource/Source/WebCore/rendering/RenderText.cpp:451 #13 0x11d577e48 in WebCore::AXObjectCache::localCaretRectForCharacterOffset(WebCore::RenderObject*&, WebCore::CharacterOffset const&) at OpenSource/Source/WebCore/accessibility/AXObjectCache.cpp:2470 #14 0x11d578387 in WebCore::AXObjectCache::absoluteCaretBoundsForCharacterOffset(WebCore::CharacterOffset const&) at OpenSource/Source/WebCore/accessibility/AXObjectCache.cpp:2479 #15 0x11d18f7f5 in WebCore::AccessibilityRenderObject::boundsForRange(WTF::RefPtr<WebCore::Range>) const at OpenSource/Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2008 #16 0x123d2c21b in ::-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:](NSString *, id) at OpenSource/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4123
OK, that indicates a bug in InlineTextBox::constructTextRun that can easily be fixed. It should be a small performance boost too.
Myles and I talked about how to fix that InlineTextBox::constructTextRun issue and he’s planning to make a patch that does it in the next couple of days.
(In reply to comment #10) > Myles and I talked about how to fix that InlineTextBox::constructTextRun > issue and he’s planning to make a patch that does it in the next couple of > days. Starting on this now.
(In reply to comment #11) > (In reply to comment #10) > > Myles and I talked about how to fix that InlineTextBox::constructTextRun > > issue and he’s planning to make a patch that does it in the next couple of > > days. > > Starting on this now. I've been doing the work for this at https://bugs.webkit.org/show_bug.cgi?id=160584.
Created attachment 286736 [details] Let's try the EWS again.
Created attachment 286774 [details] rebased.
Comment on attachment 286774 [details] rebased. Attachment 286774 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1930248 New failing tests: js/regress/put-by-id-transition-with-indexing-header.html
Created attachment 286796 [details] Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Adding CHECK_STRINGVIEW_LIFETIME to the bug title and in a comment so anyone searching bugs.webkit.org for that string will be able to find this easily.
<rdar://problem/28479434>
Created attachment 290122 [details] proposed patch. I think I've resolved the remaining issues that prevents us from re-enabling this. Let's have the EWS confirm my test results.
Comment on attachment 290122 [details] proposed patch. EWS bots confirm that we're good to go.
Thanks for the review. Landed in r206563: <http://trac.webkit.org/r206563>.
(In reply to comment #21) > Thanks for the review. Landed in r206563: <http://trac.webkit.org/r206563>. Nice work, Mark!
(In reply to comment #21) > Thanks for the review. Landed in r206563: <http://trac.webkit.org/r206563>. It made some tests timeout on debug bots, see bug162722 for details.
Re-opened since this is blocked by bug 162732
*** Bug 162722 has been marked as a duplicate of this bug. ***
I've sped up the slow tests in r206600: <http://trac.webkit.org/r206600>. This patch is re-landed in r206601: <http://trac.webkit.org/r206601>.