Bug 261305

Summary: New test added in 265747@main hits assertion failure: ScriptDisallowedScope::InMainThread::isScriptAllowed()
Product: WebKit Reporter: Karl Rackler <rackler>
Component: New BugsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: ahmad.saleem792, rniwa, webkit-bot-watchers-bugzilla, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=261339

Description Karl Rackler 2023-09-07 17:24:08 PDT
Description:
editing/inserting/break-out-of-nested-lists.html

The test was added at 265747@main and has been a consistent failure since then.

This issue can be bisected to 265747@main using command:
run-webkit-tests --debug --iterations=1 -1  editing/inserting/break-out-of-nested-lists.html

History:
https://results.webkit.org/?suite=layout-tests&test=editing%2Finserting%2Fbreak-out-of-nested-lists.html&platform=mac&flavor=wk1&style=debug&limit=6000&recent=false

Crash Log:
No crash log found.

stdout:

stderr:
ASSERTION FAILED: ScriptDisallowedScope::InMainThread::isScriptAllowed()
/Volumes/Data/worker/trunk-romee-debug-archive/build/OpenSource/Source/WebCore/dom/Document.cpp(5389) : void WebCore::Document::dispatchWindowEvent(WebCore::Event &, WebCore::EventTarget *)
1   0x11d993ab9 WTFCrash
2   0x11d993ad9 WTFCrashWithSecurityImplication
3   0x12a03d5bd WebCore::Document::dispatchWindowEvent(WebCore::Event&, WebCore::EventTarget*)
4   0x12adf2671 WebCore::dispatchEventsOnWindowAndFocusedElement(WebCore::Document*, bool)
5   0x12adf2567 WebCore::FocusController::setFocusedInternal(bool)
6   0x12adf466e WebCore::FocusController::setActivityState(WTF::OptionSet<WebCore::ActivityState>)
7   0x12aeaf60c WebCore::Page::setActivityState(WTF::OptionSet<WebCore::ActivityState>)
8   0x12adf23bc WebCore::FocusController::setFocused(bool)
9   0x10b9d0667 -[WebHTMLView resignFirstResponder]
10  0x7ff812417494 -[NSWindow _realMakeFirstResponder:]
11  0x128f2f250 WebCore::safeRemoveFromSuperview(NSView*)
12  0x128f2f157 WebCore::Widget::removeFromSuperview()
13  0x128e52f09 WebCore::ScrollView::platformRemoveChild(WebCore::Widget*)
14  0x12b0b8b1b WebCore::ScrollView::removeChild(WebCore::Widget&)
15  0x12ae5b9e4 WebCore::LocalFrameView::removeChild(WebCore::Widget&)
16  0x12ba78874 WebCore::WidgetHierarchyUpdatesSuspensionScope::moveWidgets()
17  0x12828aad8 WebCore::WidgetHierarchyUpdatesSuspensionScope::~WidgetHierarchyUpdatesSuspensionScope()
18  0x12827eec5 WebCore::WidgetHierarchyUpdatesSuspensionScope::~WidgetHierarchyUpdatesSuspensionScope()
19  0x12a02d52b WebCore::Document::resolveStyle(WebCore::Document::ResolveStyleType)
20  0x12a02df8d WebCore::Document::updateStyleIfNeeded()
21  0x12a04c38a WebCore::Document::finishedParsing()
22  0x12a7559e8 WebCore::HTMLConstructionSite::finishedParsing()
23  0x12a7abe80 WebCore::HTMLTreeBuilder::finished()
24  0x12a75ce93 WebCore::HTMLDocumentParser::end()
25  0x12a75ae11 WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd()
26  0x12a75ab39 WebCore::HTMLDocumentParser::prepareToStopParsing()
27  0x12a75b947 WebCore::HTMLDocumentParser::endIfDelayed()
28  0x12a75d260 WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution()
29  0x12a75d5ec WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&)
30  0x12a1f3f5a WebCore::PendingScript::notifyClientFinished()
31  0x12a1f3fb9 WebCore::PendingScript::notifyFinished(WebCore::LoadableScript&)
Comment 1 Radar WebKit Bug Importer 2023-09-07 17:24:52 PDT
<rdar://problem/115145062>
Comment 2 EWS 2023-09-07 17:41:31 PDT
Test gardening commit 267760@main (66f969fb7c23): <https://commits.webkit.org/267760@main>

Reviewed commits have been landed. Closing PR #17562 and removing active labels.
Comment 3 Ryosuke Niwa 2023-09-10 11:15:31 PDT
This is an issue with the assertion. We just need to disable it in WebKit1 like this in ScriptDisallowedScope.h:

        static bool isScriptAllowed()
        {
            ASSERT(isMainThread());
            if ()
#if PLATFORM(IOS_FAMILY)
            return isInWebProcess() || !s_count || webThreadDelegateMessageScopeCount;
#else
            return isInWebProcess() || !s_count;
#endif
        }

We can then isEventDispatchAllowedInSubtree like this:
        static bool isEventDispatchAllowedInSubtree(Node& node)
        {
#if ASSERT_ENABLED || ENABLE(SECURITY_ASSERTIONS)
            return isScriptAllowed() || EventAllowedScope::isAllowedNode(node);
#else
            UNUSED_PARAM(node);
            return true;
#endif
        }
Comment 4 Ahmad Saleem 2023-09-10 11:19:35 PDT
@Rniwa - should I do PR with proposed changes?
Comment 5 Ryosuke Niwa 2023-09-10 11:20:16 PDT
(In reply to Ahmad Saleem from comment #4)
> @Rniwa - should I do PR with proposed changes?

Yes, please. I can do it too but I have other test failures to investigate at the moment.
Comment 6 Ahmad Saleem 2023-09-10 11:30:26 PDT
(In reply to Ryosuke Niwa from comment #5)
> (In reply to Ahmad Saleem from comment #4)
> > @Rniwa - should I do PR with proposed changes?
> 
> Yes, please. I can do it too but I have other test failures to investigate
> at the moment.

No worries, will do in next few hours. 👍
Comment 7 Ahmad Saleem 2023-09-10 16:22:05 PDT
PR - https://github.com/WebKit/WebKit/pull/17639

Locally confirmed that it does indeed not crash while running:

run-webkit-tests --debug --iterations=1 -1  editing/inserting/break-out-of-nested-lists.html

and yes, I removed test expectation before running the above. :-)
Comment 8 Ryosuke Niwa 2023-09-12 03:26:38 PDT
Pull request: https://github.com/WebKit/WebKit/pull/17691
Comment 9 Ryosuke Niwa 2023-09-12 06:33:11 PDT
*** Bug 261339 has been marked as a duplicate of this bug. ***
Comment 10 EWS 2023-09-13 00:40:04 PDT
Committed 267935@main (f33e99829e4f): <https://commits.webkit.org/267935@main>

Reviewed commits have been landed. Closing PR #17691 and removing active labels.