Bug 182695

Summary: Use child text content when determining whether to bail early in running a script
Product: WebKit Reporter: Domenic Denicola <d>
Component: WebCore JavaScriptAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, esprehn+autocc, ews-watchlist, kangil.han, rbuis, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Domenic Denicola 2018-02-12 08:51:34 PST
Test: http://w3c-test.org/html/semantics/scripting-1/the-script-element/emptyish-script-elements.html

Spec: https://html.spec.whatwg.org/#prepare-a-script step 5

Recent spec change: https://github.com/whatwg/html/pull/3451 (to match Firefox and Edge and be simpler)

The observable difference here is due to whether or not you mark the script as "already-started", preventing future executions when its child text content changes.
Comment 1 Rob Buis 2020-05-22 08:47:12 PDT
Created attachment 400052 [details]
Patch
Comment 2 youenn fablet 2020-05-25 01:00:22 PDT
Comment on attachment 400052 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400052&action=review

> Source/WebCore/dom/ScriptElement.cpp:184
> +    const String& sourceText = scriptContent();

s/const String&/auto, scriptContent() is returning a String.
If we need to optimise this a bit, maybe we should add a TextNodeTraversal::isEmpty routine so that we create the string only when executing the script.
Comment 3 Rob Buis 2020-05-25 04:28:18 PDT
Created attachment 400193 [details]
Patch
Comment 4 Rob Buis 2020-05-25 07:42:06 PDT
Comment on attachment 400052 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400052&action=review

>> Source/WebCore/dom/ScriptElement.cpp:184
>> +    const String& sourceText = scriptContent();
> 
> s/const String&/auto, scriptContent() is returning a String.
> If we need to optimise this a bit, maybe we should add a TextNodeTraversal::isEmpty routine so that we create the string only when executing the script.

Done. I think it is okay since the majority of the time the script should have non empty content. Also I checked chromium code and they also just get the contents and then test for empty.
Comment 5 EWS 2020-05-25 07:47:56 PDT
Committed r262125: <https://trac.webkit.org/changeset/262125>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400193 [details].
Comment 6 Radar WebKit Bug Importer 2020-05-25 07:48:19 PDT
<rdar://problem/63597671>